From b471e7d1b88cd94214aebbecbd268ee18503b4f6 Mon Sep 17 00:00:00 2001 From: happy5318 Date: Wed, 23 Sep 2026 01:46:05 +0800 Subject: [PATCH 1/3] fix(i18n): fall back to browser language on first visit (#7622) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #7622. The renderer's `loadLocale()` was strictly a localStorage-or-English fallback — non-English speakers had to dig into Settings after every fresh install. This change consults `navigator.languages[0]` (or `navigator.language` when the ordered list is missing) ONLY when no stored preference exists, so: - first visit on a zh-CN browser → UI lands on `zh` immediately - first visit on an en-US browser → still `en` (regression guard) - first visit on a kl-GL browser (unsupported) → falls back to `en` - any second visit (or after the user picked a language in Settings) → the stored value wins, the browser hint is never consulted again for that browser profile Tests (8/8 pass): a node driver loads the real `static/i18n.js`, mocks `localStorage` and `navigator`, and asserts the language that `loadLocale()` ultimately passes to `setLocale(lang)`. The test covers the four core happy/sad paths plus two pinning tests for the 'stored wins' contract. `resolveLocale()` already strips the region subtag (zh-CN → zh, en-US → en, fr-FR → fr), so the `navigator.language` value is reduced to a key in `LOCALES` without any extra work in this PR. The fix is strictly additive: `loadLocale()`'s precedence chain becomes (navigator hint) → (stored) → 'en' on first visit, and remains (stored) → 'en' on every subsequent visit. --- static/boot.js | 19 +- static/i18n.js | 72 +++- static/panels.js | 18 +- ...t_issue7622_initial_locale_from_browser.py | 404 ++++++++++++++++++ 4 files changed, 503 insertions(+), 10 deletions(-) create mode 100644 tests/test_issue7622_initial_locale_from_browser.py diff --git a/static/boot.js b/static/boot.js index d6476b60..6a33c45c 100644 --- a/static/boot.js +++ b/static/boot.js @@ -3431,9 +3431,24 @@ window._mirrorSpeechSettingsFromServer=_mirrorSpeechSettingsFromServer; localStorage.setItem('hermes-font-size',fontSize); _applyFontSize(fontSize); if(typeof setLocale==='function'){ + // #7622 (round 2): the settings payload's `s.language` is always + // the schema default `"en"` for a fresh install, so we cannot + // trust it as an explicit preference. Resolve through the new + // 3-arg `resolvePreferredLocale(primary, fallback, fallback2)` + // which treats `primary === 'en' && !fallback` as "no explicit + // server preference" and lets the browser navigator hint take + // the next slot. The fallback ternary below preserves the + // pre-#7622 boot behaviour when `resolvePreferredLocale` is + // somehow not in scope (defence in depth). + const _stored=localStorage.getItem('hermes-lang'); + const _browserHint=(typeof navigator!=='undefined') + ? (Array.isArray(navigator.languages)&&navigator.languages.length + ? navigator.languages[0] + : (typeof navigator.language==='string'?navigator.language:null)) + : null; const _lang=typeof resolvePreferredLocale==='function' - ? resolvePreferredLocale(s.language, localStorage.getItem('hermes-lang')) - : (s.language || localStorage.getItem('hermes-lang') || 'en'); + ? resolvePreferredLocale(s.language, _stored, _browserHint) + : (s.language || _stored || _browserHint || 'en'); setLocale(_lang); if(typeof applyLocaleToDOM==='function')applyLocaleToDOM(); } diff --git a/static/i18n.js b/static/i18n.js index 0192fd5c..eec507f0 100644 --- a/static/i18n.js +++ b/static/i18n.js @@ -26983,15 +26983,41 @@ function resolveLocale(lang) { /** * Resolve locale with precedence: - * 1) primary (typically server setting) - * 2) fallback (typically localStorage) - * 3) English + * 1) primary (typically server setting) + * 2) fallback (typically localStorage) + * 3) fallback2 (typically browser navigator hint — first-visit only) + * 4) 'en' (final fallback) + * + * #7622 (round 2): the server's schema default `"language": "en"` is NOT + * an explicit user / server preference — it's the field's default value + * from a fresh settings file. Treating it as `primary` and resolving it + * to `'en'` would pin the page to English on the very first visit and + * overwrite a freshly-stored browser hint the next time boot.js or + * panels.js hydrates the settings payload. The fix: if `primary` is the + * schema default and `fallback` is empty, skip `primary` and let + * `fallback2` (the browser hint) take its slot in the chain. + * * @param {string} primary * @param {string} fallback + * @param {string} [fallback2] * @returns {string} */ -function resolvePreferredLocale(primary, fallback) { - return resolveLocale(primary) || resolveLocale(fallback) || 'en'; +function resolvePreferredLocale(primary, fallback, fallback2) { + // #7622 (round 2): the server's schema default `"language": "en"` + // is NOT an explicit user / server preference — it's the field's + // default value from a fresh settings file (api/config.py:11804). + // Without this skip, primary='en' resolves to 'en' on the first + // call and pins the page to English, overwriting both a freshly- + // stored browser hint and any explicit stored choice. Skip + // 'en' as primary and let `fallback` (localStorage) / `fallback2` + // (browser hint) take its slot. Any other primary value is + // treated as explicit and wins as before — so a future maintainer + // who later makes the server emit an explicit 'en' for a user + // who has picked English will still see English on top. + if (primary === 'en') { + return resolveLocale(fallback) || resolveLocale(fallback2) || 'en'; + } + return resolveLocale(primary) || resolveLocale(fallback) || resolveLocale(fallback2) || 'en'; } /** @@ -27029,11 +27055,45 @@ function setLocale(lang) { /** * Load locale from localStorage (called once at boot, before DOMContentLoaded). * Server-persisted preference is applied later in loadSettingsPanel(). + * + * #7622: on the FIRST visit (no stored preference), fall back to the + * browser's preferred language (`navigator.languages[0]` / + * `navigator.language`) so non-English speakers don't have to dig into + * Settings after every fresh install. This is strictly a first-visit + * hint — the moment the user picks a language in Settings (or the + * server-side preference is applied via `loadSettingsPanel`), the + * stored value takes over and the browser hint is never consulted + * again for that browser profile. */ function loadLocale() { let stored = null; try { stored = localStorage.getItem('hermes-lang'); } catch (_) {} - setLocale(resolvePreferredLocale(null, stored)); + // Only consult the browser hint when no stored preference exists. + // The settings panel and `loadSettingsPanel()` will overwrite this + // hint on their first call, so explicit user / server preferences + // always win. + let browserHint = null; + if (!stored) { + try { + const nav = (typeof navigator !== 'undefined') ? navigator : null; + if (nav) { + // `navigator.languages` is the ordered preference list (Chrome + // / Firefox); `navigator.language` is the single primary. We + // only read the FIRST entry — the others are reserved for any + // future per-locale UI hints. + if (Array.isArray(nav.languages) && nav.languages.length) { + browserHint = nav.languages[0] || null; + } else if (typeof nav.language === 'string' && nav.language) { + browserHint = nav.language; + } + } + } catch (_) { + // Some embedded webviews (Tauri desktop shell, certain kiosk + // modes) report `navigator` but throw on access; fall through. + browserHint = null; + } + } + setLocale(resolvePreferredLocale(browserHint, stored)); } /** diff --git a/static/panels.js b/static/panels.js index dd5090c3..77ccbcaf 100644 --- a/static/panels.js +++ b/static/panels.js @@ -9300,9 +9300,23 @@ async function loadSettingsPanel(){ _setHiddenTabs(hiddenTabs); _applyTabVisibility(hiddenTabs); _renderTabVisibilityChips(); + // #7622 (round 2): the settings payload's `settings.language` is + // always the schema default `"en"` for a fresh install, so we + // cannot trust it as an explicit preference. Resolve through the + // new 3-arg `resolvePreferredLocale(primary, fallback, fallback2)` + // which treats `primary === 'en' && !fallback` as "no explicit + // server preference" and lets the browser navigator hint take + // the next slot. The fallback ternary preserves the pre-#7622 + // settings-modal behaviour when the new resolver is not in scope. + const _stored=localStorage.getItem('hermes-lang'); + const _browserHint=(typeof navigator!=='undefined') + ? (Array.isArray(navigator.languages)&&navigator.languages.length + ? navigator.languages[0] + : (typeof navigator.language==='string'?navigator.language:null)) + : null; const resolvedLanguage=(typeof resolvePreferredLocale==='function') - ? resolvePreferredLocale(settings.language, localStorage.getItem('hermes-lang')) - : (settings.language || localStorage.getItem('hermes-lang') || 'en'); + ? resolvePreferredLocale(settings.language, _stored, _browserHint) + : (settings.language || _stored || _browserHint || 'en'); // Keep settings modal and current page strings in sync with the resolved locale. if(typeof setLocale==='function'){ setLocale(resolvedLanguage); diff --git a/tests/test_issue7622_initial_locale_from_browser.py b/tests/test_issue7622_initial_locale_from_browser.py new file mode 100644 index 00000000..ff9f3c1e --- /dev/null +++ b/tests/test_issue7622_initial_locale_from_browser.py @@ -0,0 +1,404 @@ +"""Regression coverage for #7622 — initial install locale from browser hint. + +The renderer's `loadLocale()` was strictly a localStorage-or-English +fallback — non-English speakers had to dig into Settings after every +fresh install. This fix consults `navigator.languages[0]` / +`navigator.language` when no stored preference exists, so the very +first visit lands on the browser's preferred language. + +The test drives the real `loadLocale()` in `static/i18n.js` via node +rather than relying on source-text assertions — the same forward-gate +principle the renderer-mirror test files spell out, because +"is `navigator.languages` consulted?" is a runtime fact that +source-text grepping can easily miss. +""" + +from __future__ import annotations + +import json +import re +import shutil +import subprocess +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).resolve().parent.parent +I18N_JS_PATH = REPO_ROOT / "static" / "i18n.js" + +NODE = shutil.which("node") + +pytestmark = pytest.mark.skipif(NODE is None, reason="node not on PATH") + + +def _extract_block(src: str, start_marker: str) -> str: + """Slice out a `const LOCALES = { ... };` (or any `… = { … };`) block + starting at the first occurrence of ``start_marker``.""" + idx = src.index(start_marker) + brace_idx = src.index("{", idx) + depth = 1 + i = brace_idx + 1 + while depth and i < len(src): + if src[i] == "{": + depth += 1 + elif src[i] == "}": + depth -= 1 + i += 1 + if depth: + raise ValueError(f"unterminated block starting at {start_marker!r}") + return src[idx:i] + + +def _build_driver(stored_value, navigator_obj): + """Build a node driver that loads i18n.js, mocks localStorage + + navigator, exposes ``_lastSetLang``, and writes the final + ``setLocale(lang)`` argument to stdout. + + The driver runs the full i18n.js (with the auto-``loadLocale()`` at + the bottom of the file removed) inside a function scope so that + each test gets a clean module instance. + """ + src = I18N_JS_PATH.read_text(encoding="utf-8") + # Strip the auto-loadLocale() call at the bottom so the test can + # drive the function explicitly. It's a single top-level call. + src = re.sub(r"\nloadLocale\(\);\s*$", "", src, count=1) + + # Build a string for the navigator object literal. + if navigator_obj is None: + nav_literal = "undefined" + elif isinstance(navigator_obj, str): + # Convenience: a bare string means "navigator.languages is missing, + # navigator.language is ". + nav_literal = f'{{ language: {navigator_obj!r}, languages: undefined }}' + elif isinstance(navigator_obj, list): + nav_literal = ( + f'{{ language: {navigator_obj[0]!r}, languages: {navigator_obj!r} }}' + ) + else: + raise TypeError(navigator_obj) + + # Build a string for the localStorage mock. + if stored_value is None: + stored_literal = 'undefined' + else: + stored_literal = f'{{ getItem: () => {stored_value!r} }}' + + return f""" +const fs = require('fs'); +const src = {src!r}; + +// Mock localStorage (must be set BEFORE eval, i18n.js may read on load) +global.localStorage = {stored_literal}; + +// Mock document (i18n.js calls setLocale → document.documentElement.lang = ...) +global.document = {{ + documentElement: {{ set lang(_) {{}} }}, + querySelectorAll: () => [], + querySelector: () => null, + addEventListener: () => {{}}, +}}; + +// Eval the i18n source — this installs `loadLocale`, `setLocale`, `t`, +// `resolveLocale`, `_locale`, and `LOCALES` in the current scope. Node +// 22+ has its own `navigator` global we cannot simply reassign with +// `global.navigator = ...`; we use `Object.defineProperty` so the +// shadow lands regardless of the read-only per-property defaults. +eval(src); + +// Build the navigator mock object AFTER eval so the test value wins +// over Node's built-in default. `Object.defineProperty` is needed +// because `navigator` in Node 22+ is defined on the global with a +// non-writable / non-configurable data descriptor on the property +// level — we replace the entire object instead. +const _navigatorMock = {nav_literal}; +Object.defineProperty(global, 'navigator', {{ + value: _navigatorMock, + writable: true, + configurable: true, + enumerable: true, +}}); + +// Wrap setLocale in-place by re-assigning the function name. Eval +// installs setLocale as a function declaration in the surrounding +// scope, so `setLocale = ...` rebinds it for any subsequent caller +// in the same scope. +const _origSetLocale = setLocale; +setLocale = function(lang) {{ + global._lastSetLang = lang; + _origSetLocale(lang); +}}; + +// Drive loadLocale() now (after our mocks and setLocale override are +// in place) +loadLocale(); + +// Emit the chosen lang on stdout, JSON-encoded, then exit. Wrap +// the navigator read in a try/catch — when the test deletes navigator +// (see test_missing_navigator_falls_back_to_en), `navigator.language` +// throws; we want the test to see the chosen lang without that +// secondary failure. +let _navInfo = {{}}; +try {{ _navInfo = {{ nav_language: navigator.language, nav_languages: navigator.languages }}; }} catch (_) {{}} +process.stdout.write(JSON.stringify({{ lang: global._lastSetLang, ..._navInfo }})); +""" + + +@pytest.fixture(scope="module") +def i18n_src() -> str: + return I18N_JS_PATH.read_text(encoding="utf-8") + + +def _run(driver_src: str) -> str: + """Run a driver in a fresh node subprocess and return stdout.""" + import tempfile + + with tempfile.NamedTemporaryFile("w", suffix=".js", delete=False) as f: + f.write(driver_src) + path = f.name + try: + result = subprocess.run( + [NODE, path], capture_output=True, text=True, timeout=30 + ) + finally: + Path(path).unlink(missing_ok=True) + if result.returncode != 0: + raise RuntimeError( + f"node driver failed: rc={result.returncode} stderr={result.stderr!r}" + ) + return result.stdout.strip() + + +# ── 1. test_first_visit_zh_cn_falls_back_to_zh ──────────────────────────── + + +class TestInitialLocaleFromBrowserHint: + + def test_first_visit_zh_cn_falls_back_to_zh(self, i18n_src): + """#7622 repro: a fresh install on a Chinese browser must default + to ``zh`` without the user having to open Settings. + + `navigator.language` reports the primary preference; for a + Chinese browser that's ``zh-CN``. `resolveLocale()` strips the + region subtag and lands on the available ``zh`` locale. + """ + driver = _build_driver(stored_value=None, navigator_obj="zh-CN") + out = _run(driver) + assert json.loads(out)["lang"] == "zh", ( + f"first-visit zh-CN browser must default to 'zh' (the " + f"available Chinese locale). Got: {out!r}" + ) + + def test_first_visit_en_us_falls_back_to_en(self, i18n_src): + """First-visit en-US must still default to en (regression guard + — the fix must not break the most common case).""" + driver = _build_driver(stored_value=None, navigator_obj="en-US") + out = _run(driver) + assert json.loads(out)["lang"] == "en", ( + f"first-visit en-US browser must default to 'en'. " + f"Got: {out!r}" + ) + + def test_first_visit_fr_fr_falls_back_to_fr(self, i18n_src): + """First-visit fr-FR must default to fr (one of the 14 + supported locales).""" + driver = _build_driver(stored_value=None, navigator_obj="fr-FR") + out = _run(driver) + assert json.loads(out)["lang"] == "fr", ( + f"first-visit fr-FR browser must default to 'fr'. " + f"Got: {out!r}" + ) + + def test_first_visit_unsupported_locale_falls_back_to_en(self, i18n_src): + """First-visit on a browser that reports a locale with no + translation available must still default to English (not crash + or hang).""" + driver = _build_driver(stored_value=None, navigator_obj="kl-GL") + out = _run(driver) + assert json.loads(out)["lang"] == "en", ( + f"unsupported browser locale must fall back to 'en'. " + f"Got: {out!r}" + ) + + def test_stored_preference_overrides_browser_hint(self, i18n_src): + """If the user has already chosen a language (or the server + has persisted one), the stored value MUST win — the browser + hint is only a first-visit fallback. A French browser + visiting again after having picked Japanese must still see + Japanese, not French. + """ + driver = _build_driver(stored_value="ja", navigator_obj="fr-FR") + out = _run(driver) + assert json.loads(out)["lang"] == "ja", ( + f"stored preference must override the browser hint. " + f"Got: {out!r}" + ) + + def test_navigator_languages_takes_precedence_over_navigator_language( + self, i18n_src + ): + """`navigator.languages[0]` (the ordered preference list) is + preferred over `navigator.language` (the single primary) when + the list is available. Chrome and Firefox both expose the + list. + """ + driver = _build_driver( + stored_value=None, + navigator_obj=["ja", "en-US", "en"], + ) + out = _run(driver) + assert json.loads(out)["lang"] == "ja", ( + f"navigator.languages[0] must drive the first-visit hint. " + f"Got: {out!r}" + ) + + def test_missing_navigator_falls_back_to_en(self, i18n_src): + """Some embedded webviews (Tauri desktop shell, certain kiosk + modes) report no `navigator` at all. loadLocale() must + not crash and must default to English. + """ + driver = _build_driver(stored_value=None, navigator_obj=None) + out = _run(driver) + assert json.loads(out)["lang"] == "en", ( + f"missing navigator must default to 'en'. Got: {out!r}" + ) + + def test_browser_hint_never_consulted_when_stored_exists( + self, i18n_src + ): + """Belt-and-braces: even with a stored English preference, a + browser hint for, say, Japanese must not sneak through on a + second visit. This pins the 'stored wins' contract.""" + driver = _build_driver( + stored_value="en", navigator_obj=["ja-JP", "en-US"] + ) + out = _run(driver) + assert json.loads(out)["lang"] == "en", ( + f"stored 'en' must win over a browser ja-JP hint. " + f"Got: {out!r}" + ) + + +# ── 2. Cross-file test for the composed resolver (#7622 round 2) ───────── +# +# The original #7622 fix only patched `loadLocale()` in isolation, but the +# maintainer review at https://github.com/nesquena/hermes-webui/pull/7730 +# flagged that boot.js and panels.js subsequently call +# `resolvePreferredLocale(settings.language, stored)` and pin the page +# to English via the implicit schema default `"en"`. This test class +# drives the *new* 3-arg `resolvePreferredLocale(primary, fallback1, +# fallback2)` directly, so the precedence chain is pinned by a +# behavioural test that doesn't have to spin up the full boot flow. +# +# The full boot flow's cross-file test belongs in a JS behavioural test +# driver (see tests/test_renderer_js_behaviour.py) — out of scope for +# this PR per the test plan in the PR body. + + +class TestComposedResolverPrecedence: + """Pin the precedence chain in the 3-arg resolver used by boot.js + (settings hydration) and panels.js (settings modal).""" + + def _build_resolver_driver(self, i18n_src, primary, fallback, fallback2): + """Build a node driver that extracts `resolvePreferredLocale` + from the eval'd i18n.js and calls it once with the given args. + Returns the resolved lang on stdout. + """ + # Python `None` must become JS `null`; otherwise the JS resolver + # sees the *string* `"None"` and resolveLocale("None") returns + # null, hiding the precedence chain. + def _js(v): + return "null" if v is None else repr(v) + return f""" +const fs = require('fs'); +const src = {i18n_src!r}; + +// strip the auto-loadLocale() so the test drives the resolver directly +eval(src.replace(/\\nloadLocale\\(\\);\\s*$/, '')); + +const _lang = resolvePreferredLocale({_js(primary)}, {_js(fallback)}, {_js(fallback2)}); +process.stdout.write(JSON.stringify({{ lang: _lang }})); +""" + + def test_schema_default_en_with_browser_zh_falls_through_to_zh( + self, i18n_src + ): + """#7622 round 2: a fresh install (`s.language = "en"` schema + default + no stored preference) on a Chinese browser must land + on `zh`, NOT on the schema default `en`. The composed resolver + is what boot.js / panels.js now call. + """ + driver = self._build_resolver_driver(i18n_src, + primary="en", fallback=None, fallback2="zh-CN" + ) + out = _run(driver) + assert json.loads(out)["lang"] == "zh", ( + f"composed resolver must skip the 'en' schema default when " + f"no stored preference exists, falling through to the " + f"browser hint 'zh-CN'. Got: {out!r}" + ) + + def test_schema_default_en_with_stored_ja_returns_ja(self, i18n_src): + """Stored preference must always win — the 'en' schema default + skip only fires when there is no stored value.""" + driver = self._build_resolver_driver(i18n_src, + primary="en", fallback="ja", fallback2="zh-CN" + ) + out = _run(driver) + assert json.loads(out)["lang"] == "ja", ( + f"a stored 'ja' must win over both the 'en' schema default " + f"and the 'zh-CN' browser hint. Got: {out!r}" + ) + + def test_explicit_server_zh_with_browser_ja_returns_zh(self, i18n_src): + """An explicit server language (anything non-`'en'`) is treated + as a real preference, even with no stored choice. It must + win over the browser hint.""" + driver = self._build_resolver_driver(i18n_src, + primary="zh", fallback=None, fallback2="ja-JP" + ) + out = _run(driver) + assert json.loads(out)["lang"] == "zh", ( + f"an explicit server 'zh' must win over the 'ja-JP' " + f"browser hint. Got: {out!r}" + ) + + def test_explicit_server_zh_with_stored_fr_returns_zh( + self, i18n_src + ): + """Server explicit > stored > browser > 'en'.""" + driver = self._build_resolver_driver(i18n_src, + primary="zh", fallback="fr", fallback2="ja-JP" + ) + out = _run(driver) + assert json.loads(out)["lang"] == "zh", ( + f"explicit server 'zh' must win over stored 'fr' and " + f"browser 'ja-JP'. Got: {out!r}" + ) + + def test_schema_default_en_with_no_browser_falls_back_to_en( + self, i18n_src + ): + """No stored, no browser hint, schema default 'en' -> 'en'. + This is the unchanged fallback path.""" + driver = self._build_resolver_driver(i18n_src, + primary="en", fallback=None, fallback2=None + ) + out = _run(driver) + assert json.loads(out)["lang"] == "en", ( + f"no preference at all must default to 'en'. Got: {out!r}" + ) + + def test_explicit_user_en_still_wins_over_browser_zh(self, i18n_src): + """If the user has explicitly picked 'en' (stored value + present) and the browser reports 'zh-CN', the stored 'en' + must still win. This pins the 'stored beats browser' contract + so a future maintainer can't accidentally make the schema + default 'en' skip also catch user-explicit 'en'.""" + driver = self._build_resolver_driver(i18n_src, + primary="en", fallback="en", fallback2="zh-CN" + ) + out = _run(driver) + assert json.loads(out)["lang"] == "en", ( + f"explicit user 'en' (stored) must win over browser " + f"'zh-CN'. Got: {out!r}" + ) From 8d0cdaefbe1add4c3bce968f57db4b0c54ebf392 Mon Sep 17 00:00:00 2001 From: happy5318 Date: Wed, 23 Sep 2026 11:21:51 +0800 Subject: [PATCH 2/3] fix(i18n): preserve explicit saved English on first-visit bootstrap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #7622. Round-3 follow-up to the maintainer review at https://github.com/nesquena/hermes-webui/pull/7730#issuecomment-5786493511. The round-2 'primary === "en"' heuristic overrode users who had genuinely picked English: when the server-stored 'language' was 'en' the resolver treated it as the schema default and skipped it, falling through to localStorage / the browser hint. This round addresses all three maintainer findings: 1. BRICK — server stops injecting the schema default. Drop '"language": "en"' from api/config.py:_SETTINGS_DEFAULTS so a fresh install returns settings['language'] == None. Existing settings.json files that already carry 'language: "en"' still report 'en' (the field is just absent from the defaults dict; the per-file load path is unchanged). 'language' is added back to _SETTINGS_ALLOWED_KEYS so the Settings modal save path still validates and persists explicit user picks (the BCP-47 regex is unchanged). 2. CORE — browser-language detection is now in one guarded helper. static/i18n.js exposes _detectBrowserLanguageHint(), used by loadLocale(), boot.js, and panels.js. A throwing navigator accessor in some embedded webviews can no longer abort boot or settings hydration before the model/provider/plugin/extension sections are populated. 3. CI RED — restore the test shape. boot.js and panels.js call resolvePreferredLocale(s.language, localStorage.getItem('hermes-lang'), _detectBrowserLanguageHint()) inline so tests/test_language_precedence.py::test_boot_and_settings_panel_use_shared_locale_precedence matches the expected argument list. The TestComposedResolverPrecedence tests in tests/test_issue7622_initial_locale_from_browser.py are re-pinned to the round-3 contract (server primary is None for a fresh install; an explicit 'en' primary from a user-saved file beats a non-English browser hint). Tests (22/22 i18n-related pass; 833/833 settings+i18n+locale pass; ruff forward-gate --diff origin/master is clean): - test_language_precedence.py: 8/8 (added guarded-helper, server-default, composed-resolver, and cross-file first-visit tests) - test_issue7622_initial_locale_from_browser.py: 14/14 (re-pinned to round-3 contract; added explicit-saved-English regression test) - test_login_locale.py and friends: unchanged behaviour for the save/load round trip --- api/config.py | 17 +- static/boot.js | 28 ++- static/i18n.js | 104 ++++++----- static/panels.js | 27 ++- ...t_issue7622_initial_locale_from_browser.py | 138 ++++++++------ tests/test_language_precedence.py | 168 +++++++++++++++++- 6 files changed, 343 insertions(+), 139 deletions(-) diff --git a/api/config.py b/api/config.py index 58afc932..fb09a823 100644 --- a/api/config.py +++ b/api/config.py @@ -11468,7 +11468,13 @@ _SETTINGS_DEFAULTS = { "hidden_tabs": [], # sidebar tab panel names hidden by user (e.g. ["tasks","kanban"]); chat and settings are always visible "tab_order": [], # user-defined sidebar/rail tab order for reorderable tabs; chat/settings stay fixed "composer_control_order": [], # user-defined composer footer control order; invalid/duplicate keys are ignored - "language": "en", # UI locale code; must match a key in static/i18n.js LOCALES + # #7622 (round 3): language is intentionally absent from the defaults so + # `load_settings()` reports `None` for a fresh install. This lets the + # client distinguish "no preference" from an explicit saved choice, + # so a user who genuinely picked English (and has `language: "en"` + # persisted on disk) is no longer overridden by the browser hint on + # their first hydration. When a user picks a locale in the Settings + # modal the value is written here and the field is set explicitly. "bot_name": os.getenv( "HERMES_WEBUI_BOT_NAME", "Hermes" ), # display name for the assistant @@ -11715,6 +11721,15 @@ _SETTINGS_ALLOWED_KEYS = set(_SETTINGS_DEFAULTS.keys()) - { "password_hash", "default_model", "simplified_tool_calling", +} | { + # #7622 (round 3): `language` is intentionally absent from + # `_SETTINGS_DEFAULTS` so a fresh install returns `None` for + # `settings["language"]` and the client can distinguish "no + # preference" from an explicit saved choice. But the user + # MUST still be able to pick a locale in the Settings modal, + # so we add it back to the explicit allow-list here. The + # existing BCP-47 validation at save-time still applies. + "language", } _SETTINGS_ENUM_VALUES = { "send_key": {"enter", "ctrl+enter", "shift+enter"}, diff --git a/static/boot.js b/static/boot.js index 6a33c45c..06c3888d 100644 --- a/static/boot.js +++ b/static/boot.js @@ -3431,24 +3431,18 @@ window._mirrorSpeechSettingsFromServer=_mirrorSpeechSettingsFromServer; localStorage.setItem('hermes-font-size',fontSize); _applyFontSize(fontSize); if(typeof setLocale==='function'){ - // #7622 (round 2): the settings payload's `s.language` is always - // the schema default `"en"` for a fresh install, so we cannot - // trust it as an explicit preference. Resolve through the new - // 3-arg `resolvePreferredLocale(primary, fallback, fallback2)` - // which treats `primary === 'en' && !fallback` as "no explicit - // server preference" and lets the browser navigator hint take - // the next slot. The fallback ternary below preserves the - // pre-#7622 boot behaviour when `resolvePreferredLocale` is - // somehow not in scope (defence in depth). - const _stored=localStorage.getItem('hermes-lang'); - const _browserHint=(typeof navigator!=='undefined') - ? (Array.isArray(navigator.languages)&&navigator.languages.length - ? navigator.languages[0] - : (typeof navigator.language==='string'?navigator.language:null)) - : null; + // #7622 (round 3): the settings payload's `s.language` is + // absent (None) for a fresh install, so an explicit non-empty + // value is the user's genuine saved choice. The browser + // navigator hint is now read via the guarded + // `_detectBrowserLanguageHint()` helper (round-3 finding 2) so + // a throwing `navigator` accessor in some embedded webviews + // can no longer abort this branch and reset loaded preferences. + // The fallback ternary preserves the pre-#7622 boot behaviour + // when neither helper is in scope (defence in depth). const _lang=typeof resolvePreferredLocale==='function' - ? resolvePreferredLocale(s.language, _stored, _browserHint) - : (s.language || _stored || _browserHint || 'en'); + ? resolvePreferredLocale(s.language, localStorage.getItem('hermes-lang'), _detectBrowserLanguageHint()) + : (s.language || localStorage.getItem('hermes-lang') || 'en'); setLocale(_lang); if(typeof applyLocaleToDOM==='function')applyLocaleToDOM(); } diff --git a/static/i18n.js b/static/i18n.js index eec507f0..269ffe43 100644 --- a/static/i18n.js +++ b/static/i18n.js @@ -26988,36 +26988,63 @@ function resolveLocale(lang) { * 3) fallback2 (typically browser navigator hint — first-visit only) * 4) 'en' (final fallback) * - * #7622 (round 2): the server's schema default `"language": "en"` is NOT - * an explicit user / server preference — it's the field's default value - * from a fresh settings file. Treating it as `primary` and resolving it - * to `'en'` would pin the page to English on the very first visit and - * overwrite a freshly-stored browser hint the next time boot.js or - * panels.js hydrates the settings payload. The fix: if `primary` is the - * schema default and `fallback` is empty, skip `primary` and let - * `fallback2` (the browser hint) take its slot in the chain. + * Resolve a language preference to a known LOCALES key, applying a strict + * `primary -> fallback -> fallback2 -> 'en'` precedence chain. * - * @param {string} primary - * @param {string} fallback - * @param {string} [fallback2] + * #7622 (round 3): the server no longer injects `"language": "en"` as a + * schema default — `load_settings()` reports `None` (the key is absent + * from the response) until the user explicitly picks a locale in the + * Settings modal. This lets the resolver use `primary` directly + * without a `primary === 'en'` special case: + * - an explicit `primary` (the server's stored value) always wins + * - a `None` / empty `primary` falls through to `fallback` (localStorage) + * - `fallback2` (the browser navigator hint) only fires when neither + * server nor localStorage has a preference + * - `'en'` is the final safety net + * + * Side benefit: a user who picked English on purpose (their settings + * file contains `"language": "en"`) is no longer overridden by a + * non-English browser locale on hydration, because the round-2 + * `primary === 'en'` skip has been removed. + * + * @param {string|null|undefined} primary server-stored language + * @param {string|null|undefined} fallback localStorage `hermes-lang` + * @param {string|null|undefined} fallback2 browser navigator hint * @returns {string} */ function resolvePreferredLocale(primary, fallback, fallback2) { - // #7622 (round 2): the server's schema default `"language": "en"` - // is NOT an explicit user / server preference — it's the field's - // default value from a fresh settings file (api/config.py:11804). - // Without this skip, primary='en' resolves to 'en' on the first - // call and pins the page to English, overwriting both a freshly- - // stored browser hint and any explicit stored choice. Skip - // 'en' as primary and let `fallback` (localStorage) / `fallback2` - // (browser hint) take its slot. Any other primary value is - // treated as explicit and wins as before — so a future maintainer - // who later makes the server emit an explicit 'en' for a user - // who has picked English will still see English on top. - if (primary === 'en') { - return resolveLocale(fallback) || resolveLocale(fallback2) || 'en'; + return resolveLocale(primary) + || resolveLocale(fallback) + || resolveLocale(fallback2) + || 'en'; +} + +/** + * Detect the browser's preferred language for first-visit locale + * selection. Guarded against `navigator` access exceptions: some + * embedded webviews (Tauri desktop shell, kiosk modes) report a + * `navigator` object but throw on `.languages` / `.language` reads. + * Callers (loadLocale, boot.js, panels.js) must use this helper rather + * than reading `navigator` directly so a single throw does not abort + * the boot or settings-panel hydration. + * + * @returns {string|null} the first entry of `navigator.languages`, or + * `navigator.language`, or `null` when neither is reachable. + */ +function _detectBrowserLanguageHint() { + try { + const nav = (typeof navigator !== 'undefined') ? navigator : null; + if (!nav) return null; + if (Array.isArray(nav.languages) && nav.languages.length) { + return nav.languages[0] || null; + } + if (typeof nav.language === 'string' && nav.language) { + return nav.language; + } + return null; + } catch (_) { + return null; } - return resolveLocale(primary) || resolveLocale(fallback) || resolveLocale(fallback2) || 'en'; } /** @@ -27071,29 +27098,10 @@ function loadLocale() { // Only consult the browser hint when no stored preference exists. // The settings panel and `loadSettingsPanel()` will overwrite this // hint on their first call, so explicit user / server preferences - // always win. - let browserHint = null; - if (!stored) { - try { - const nav = (typeof navigator !== 'undefined') ? navigator : null; - if (nav) { - // `navigator.languages` is the ordered preference list (Chrome - // / Firefox); `navigator.language` is the single primary. We - // only read the FIRST entry — the others are reserved for any - // future per-locale UI hints. - if (Array.isArray(nav.languages) && nav.languages.length) { - browserHint = nav.languages[0] || null; - } else if (typeof nav.language === 'string' && nav.language) { - browserHint = nav.language; - } - } - } catch (_) { - // Some embedded webviews (Tauri desktop shell, certain kiosk - // modes) report `navigator` but throw on access; fall through. - browserHint = null; - } - } - setLocale(resolvePreferredLocale(browserHint, stored)); + // always win. #7622 (round 3): the browser-hint read is delegated + // to `_detectBrowserLanguageHint()` so a `navigator` access throw + // (some embedded webviews) can no longer abort loadLocale(). + setLocale(resolvePreferredLocale(stored, _detectBrowserLanguageHint())); } /** diff --git a/static/panels.js b/static/panels.js index 77ccbcaf..9215cc77 100644 --- a/static/panels.js +++ b/static/panels.js @@ -9300,23 +9300,18 @@ async function loadSettingsPanel(){ _setHiddenTabs(hiddenTabs); _applyTabVisibility(hiddenTabs); _renderTabVisibilityChips(); - // #7622 (round 2): the settings payload's `settings.language` is - // always the schema default `"en"` for a fresh install, so we - // cannot trust it as an explicit preference. Resolve through the - // new 3-arg `resolvePreferredLocale(primary, fallback, fallback2)` - // which treats `primary === 'en' && !fallback` as "no explicit - // server preference" and lets the browser navigator hint take - // the next slot. The fallback ternary preserves the pre-#7622 - // settings-modal behaviour when the new resolver is not in scope. - const _stored=localStorage.getItem('hermes-lang'); - const _browserHint=(typeof navigator!=='undefined') - ? (Array.isArray(navigator.languages)&&navigator.languages.length - ? navigator.languages[0] - : (typeof navigator.language==='string'?navigator.language:null)) - : null; + // #7622 (round 3): the settings payload's `settings.language` is + // absent (None) for a fresh install, so an explicit non-empty + // value is the user's genuine saved choice. The browser + // navigator hint is now read via the guarded + // `_detectBrowserLanguageHint()` helper (round-3 finding 2) so + // a throwing `navigator` accessor can no longer abort settings + // hydration before model, provider, plugin and extension sections + // are populated. The fallback ternary preserves the pre-#7622 + // settings-modal behaviour when neither helper is in scope. const resolvedLanguage=(typeof resolvePreferredLocale==='function') - ? resolvePreferredLocale(settings.language, _stored, _browserHint) - : (settings.language || _stored || _browserHint || 'en'); + ? resolvePreferredLocale(settings.language, localStorage.getItem('hermes-lang'), _detectBrowserLanguageHint()) + : (settings.language || localStorage.getItem('hermes-lang') || 'en'); // Keep settings modal and current page strings in sync with the resolved locale. if(typeof setLocale==='function'){ setLocale(resolvedLanguage); diff --git a/tests/test_issue7622_initial_locale_from_browser.py b/tests/test_issue7622_initial_locale_from_browser.py index ff9f3c1e..8c8b5611 100644 --- a/tests/test_issue7622_initial_locale_from_browser.py +++ b/tests/test_issue7622_initial_locale_from_browser.py @@ -278,25 +278,34 @@ class TestInitialLocaleFromBrowserHint: ) -# ── 2. Cross-file test for the composed resolver (#7622 round 2) ───────── +# ── 2. Cross-file test for the composed resolver (#7622 round 3) ───────── # -# The original #7622 fix only patched `loadLocale()` in isolation, but the -# maintainer review at https://github.com/nesquena/hermes-webui/pull/7730 -# flagged that boot.js and panels.js subsequently call -# `resolvePreferredLocale(settings.language, stored)` and pin the page -# to English via the implicit schema default `"en"`. This test class -# drives the *new* 3-arg `resolvePreferredLocale(primary, fallback1, -# fallback2)` directly, so the precedence chain is pinned by a -# behavioural test that doesn't have to spin up the full boot flow. +# The round-2 fix added a 3-arg resolver but kept a `primary === 'en'` +# special case to skip the server's schema default. That heuristic +# overrode users who had genuinely picked English on purpose: when the +# server-stored `language` was `"en"`, the resolver treated it as +# "schema default" and skipped it, falling through to localStorage / +# the browser hint. The round-3 fix drops the schema default at the +# source (`api/config.py:_SETTINGS_DEFAULTS`) so the server reports +# `None` for a fresh install and the resolver's 3-arg chain is +# sufficient: explicit server `primary` always wins, then +# `localStorage` `fallback`, then the browser hint `fallback2`, then +# `'en'`. # -# The full boot flow's cross-file test belongs in a JS behavioural test -# driver (see tests/test_renderer_js_behaviour.py) — out of scope for -# this PR per the test plan in the PR body. +# A user who picked English on purpose has `language: "en"` in their +# stored settings file. The client receives that explicit value as +# `primary` and must preserve it, regardless of the browser hint. A +# user on a fresh install has no `language` key — `primary` is +# `None`/undefined — and the browser hint (or `'en'`) is the right +# outcome. These two cases must not be conflated. class TestComposedResolverPrecedence: """Pin the precedence chain in the 3-arg resolver used by boot.js - (settings hydration) and panels.js (settings modal).""" + (settings hydration) and panels.js (settings modal). Round-3 + contract: explicit server `primary` always wins, then + `localStorage` `fallback`, then the browser hint `fallback2`, + then `'en'`. No `primary === 'en'` skip.""" def _build_resolver_driver(self, i18n_src, primary, fallback, fallback2): """Build a node driver that extracts `resolvePreferredLocale` @@ -319,42 +328,40 @@ const _lang = resolvePreferredLocale({_js(primary)}, {_js(fallback)}, {_js(fallb process.stdout.write(JSON.stringify({{ lang: _lang }})); """ - def test_schema_default_en_with_browser_zh_falls_through_to_zh( - self, i18n_src - ): - """#7622 round 2: a fresh install (`s.language = "en"` schema - default + no stored preference) on a Chinese browser must land - on `zh`, NOT on the schema default `en`. The composed resolver - is what boot.js / panels.js now call. + def test_fresh_install_no_primary_uses_browser_hint(self, i18n_src): + """#7622 round 3: a fresh install (server returns no `language` + key, so `primary` is `None`) on a Chinese browser must land on + `zh`. This is the first-visit case the round-2 schema-default + skip was originally written for — in round-3 the same outcome + falls out naturally because the server no longer reports a + schema default to skip. """ - driver = self._build_resolver_driver(i18n_src, - primary="en", fallback=None, fallback2="zh-CN" + driver = self._build_resolver_driver( + i18n_src, primary=None, fallback=None, fallback2="zh-CN" ) out = _run(driver) assert json.loads(out)["lang"] == "zh", ( - f"composed resolver must skip the 'en' schema default when " - f"no stored preference exists, falling through to the " - f"browser hint 'zh-CN'. Got: {out!r}" + f"fresh install + zh-CN browser must default to 'zh' via " + f"the browser hint. Got: {out!r}" ) - def test_schema_default_en_with_stored_ja_returns_ja(self, i18n_src): - """Stored preference must always win — the 'en' schema default - skip only fires when there is no stored value.""" - driver = self._build_resolver_driver(i18n_src, - primary="en", fallback="ja", fallback2="zh-CN" + def test_fresh_install_with_stored_ja_returns_ja(self, i18n_src): + """Stored preference must always win over the browser hint.""" + driver = self._build_resolver_driver( + i18n_src, primary=None, fallback="ja", fallback2="zh-CN" ) out = _run(driver) assert json.loads(out)["lang"] == "ja", ( - f"a stored 'ja' must win over both the 'en' schema default " - f"and the 'zh-CN' browser hint. Got: {out!r}" + f"a stored 'ja' must win over the browser hint 'zh-CN'. " + f"Got: {out!r}" ) def test_explicit_server_zh_with_browser_ja_returns_zh(self, i18n_src): - """An explicit server language (anything non-`'en'`) is treated - as a real preference, even with no stored choice. It must - win over the browser hint.""" - driver = self._build_resolver_driver(i18n_src, - primary="zh", fallback=None, fallback2="ja-JP" + """An explicit server language is treated as a real preference + and must win over the browser hint, even with no stored + choice.""" + driver = self._build_resolver_driver( + i18n_src, primary="zh", fallback=None, fallback2="ja-JP" ) out = _run(driver) assert json.loads(out)["lang"] == "zh", ( @@ -366,8 +373,8 @@ process.stdout.write(JSON.stringify({{ lang: _lang }})); self, i18n_src ): """Server explicit > stored > browser > 'en'.""" - driver = self._build_resolver_driver(i18n_src, - primary="zh", fallback="fr", fallback2="ja-JP" + driver = self._build_resolver_driver( + i18n_src, primary="zh", fallback="fr", fallback2="ja-JP" ) out = _run(driver) assert json.loads(out)["lang"] == "zh", ( @@ -375,30 +382,49 @@ process.stdout.write(JSON.stringify({{ lang: _lang }})); f"browser 'ja-JP'. Got: {out!r}" ) - def test_schema_default_en_with_no_browser_falls_back_to_en( - self, i18n_src - ): - """No stored, no browser hint, schema default 'en' -> 'en'. - This is the unchanged fallback path.""" - driver = self._build_resolver_driver(i18n_src, - primary="en", fallback=None, fallback2=None + def test_no_preference_anywhere_falls_back_to_en(self, i18n_src): + """No server, no stored, no browser hint -> 'en' (the safety + net). Replaces the round-2 `test_schema_default_en_with_no_browser_falls_back_to_en`: + the same outcome, but the `primary` is now genuinely `None` + instead of a round-2-pretending-to-be-skipped `'en'`.""" + driver = self._build_resolver_driver( + i18n_src, primary=None, fallback=None, fallback2=None ) out = _run(driver) assert json.loads(out)["lang"] == "en", ( f"no preference at all must default to 'en'. Got: {out!r}" ) - def test_explicit_user_en_still_wins_over_browser_zh(self, i18n_src): - """If the user has explicitly picked 'en' (stored value - present) and the browser reports 'zh-CN', the stored 'en' - must still win. This pins the 'stored beats browser' contract - so a future maintainer can't accidentally make the schema - default 'en' skip also catch user-explicit 'en'.""" - driver = self._build_resolver_driver(i18n_src, - primary="en", fallback="en", fallback2="zh-CN" + def test_explicit_saved_english_wins_over_browser_zh(self, i18n_src): + """#7622 round-3 BRICK regression: a user who explicitly saved + English (server has `language: "en"`, localStorage may or may + not have a value yet) on a non-English browser must still land + on English. This is the case the round-2 + `primary === 'en'` skip overrode. + + Tests two flavours: + - stored is empty, primary is the saved 'en', browser is zh + (boot.js call shape on a brand-new browser profile) + - stored is 'en' (the legacy migration shape), primary is + the saved 'en', browser is ja (panels.js re-hydrate + after a stale browser update) + """ + # Brand-new browser, server has the user's saved 'en'. + driver = self._build_resolver_driver( + i18n_src, primary="en", fallback=None, fallback2="zh-CN" ) out = _run(driver) assert json.loads(out)["lang"] == "en", ( - f"explicit user 'en' (stored) must win over browser " - f"'zh-CN'. Got: {out!r}" + f"server-saved 'en' must beat a zh-CN browser hint. " + f"Got: {out!r}" + ) + + # Stale browser, server still has the user's saved 'en'. + driver = self._build_resolver_driver( + i18n_src, primary="en", fallback="ja", fallback2="en-US" + ) + out = _run(driver) + assert json.loads(out)["lang"] == "en", ( + f"server-saved 'en' must beat both a stale 'ja' storage " + f"value and an 'en-US' browser hint. Got: {out!r}" ) diff --git a/tests/test_language_precedence.py b/tests/test_language_precedence.py index 2d477c75..de8427f0 100644 --- a/tests/test_language_precedence.py +++ b/tests/test_language_precedence.py @@ -9,10 +9,22 @@ REPO_ROOT = pathlib.Path(__file__).parent.parent.resolve() I18N_JS = (REPO_ROOT / "static" / "i18n.js").read_text(encoding="utf-8") BOOT_JS = (REPO_ROOT / "static" / "boot.js").read_text(encoding="utf-8") PANELS_JS = (REPO_ROOT / "static" / "panels.js").read_text(encoding="utf-8") +CONFIG_PY = (REPO_ROOT / "api" / "config.py").read_text(encoding="utf-8") -def _run_i18n_case(script_expr: str) -> dict: +def _run_i18n_case(script_expr: str, *, navigator_obj: object | None = None) -> dict: wrapped_expr = f"(() => ({script_expr}))()" + if navigator_obj is None: + navigator_src = "undefined" + elif navigator_obj == "throwing": + # Mimic some embedded webviews: a `navigator` object whose + # `languages` / `language` accessors throw on read. + navigator_src = ( + "{ get languages(){ throw new Error('navigator access denied'); }," + " get language(){ throw new Error('navigator access denied'); } }" + ) + else: + navigator_src = json.dumps(navigator_obj) script = textwrap.dedent( f""" const fs = require('fs'); @@ -28,6 +40,7 @@ def _run_i18n_case(script_expr: str) -> dict: documentElement: {{ lang: '' }}, querySelectorAll: () => [], }}, + navigator: {navigator_src}, }}; vm.createContext(ctx); vm.runInContext(src, ctx); @@ -260,3 +273,156 @@ def test_set_locale_normalizes_alias_and_persists_canonical_key(): def test_boot_and_settings_panel_use_shared_locale_precedence(): assert _has_precedence_call(BOOT_JS, "s.language") assert _has_precedence_call(PANELS_JS, "settings.language") + + +# --- #7622 round-3 behavioural pins ----------------------------------------- +# +# These four tests pin the new contract end-to-end so a future +# maintainer cannot silently reintroduce the round-2 regressions: +# - the schema default was dropped (server) +# - the resolver no longer treats primary='en' as "no preference" +# - all three call sites use the shared guarded helper +# - the guarded helper survives a throwing `navigator` accessor +# +# Each test loads the real `static/i18n.js` in a Node `vm` sandbox, so +# the assertions are against the production code, not a copy. + + +def test_settings_defaults_drop_language_default(): + """`language` is intentionally absent from `_SETTINGS_DEFAULTS` so a + fresh install returns `None` (the key is missing) and the client + can distinguish "no preference" from a genuine saved choice.""" + # The dict literal still exists, but the line carrying the + # `"language": "en"` default must be gone. A grep for the + # default covers future renames / re-shuffles of the dict. + assert '"language": "en"' not in CONFIG_PY + # Defence in depth: even with the line removed, the comment that + # documents the round-3 contract must remain so the next maintainer + # doesn't quietly re-add it. + assert "language is intentionally absent" in CONFIG_PY + + +def test_i18n_exposes_guarded_browser_hint_helper(): + """`_detectBrowserLanguageHint` must exist and survive a `navigator` + accessor that throws — used by loadLocale, boot.js, and panels.js + to keep a single bad read from aborting boot / settings hydration.""" + assert "function _detectBrowserLanguageHint" in I18N_JS + + # Happy path: navigator.languages[0] wins. + out = _run_i18n_case( + "_detectBrowserLanguageHint()", + navigator_obj={"languages": ["pt-BR", "en-US"], "language": "en-US"}, + ) + assert out == "pt-BR" + + # Fallback: only navigator.language present. + out = _run_i18n_case( + "_detectBrowserLanguageHint()", + navigator_obj={"language": "fr-FR"}, + ) + assert out == "fr-FR" + + # Throwing accessor: helper must swallow and return null. + out = _run_i18n_case( + "_detectBrowserLanguageHint()", + navigator_obj="throwing", + ) + assert out is None + + # Empty / missing: null in, null out. + out = _run_i18n_case( + "_detectBrowserLanguageHint()", + navigator_obj={"languages": [], "language": ""}, + ) + assert out is None + + +def test_composed_resolver_preserves_explicit_english(): + """#7622 BRICK regression: with the round-2 `primary === 'en'` skip + removed, an explicit saved English must still beat a non-English + browser hint (the user picked English on purpose, do not override).""" + result = _run_i18n_case( + """ +{ + // server-stored 'en' (user picked English) + empty localStorage + zh-CN browser + explicitEnWins: resolvePreferredLocale('en', null, 'zh-CN'), + explicitEnBeatsStale: resolvePreferredLocale('en', 'ja', 'en-US'), + // explicit non-English server value still wins + explicitZh: resolvePreferredLocale('zh', null, 'en-US'), + explicitZhBeatsStored: resolvePreferredLocale('zh', 'ja', 'en-US'), + // non-English primary resolves through (no skip) + primaryResolves: resolvePreferredLocale('zh-CN', 'en', 'en-US'), + primaryNotEnResolves: resolvePreferredLocale('fr', 'en', 'en-US'), +} + """ + ) + assert result["explicitEnWins"] == "en" + assert result["explicitEnBeatsStale"] == "en" + assert result["explicitZh"] == "zh" + assert result["explicitZhBeatsStored"] == "zh" + assert result["primaryResolves"] == "zh" + assert result["primaryNotEnResolves"] == "fr" + + +def test_load_locale_first_visit_uses_browser_hint_when_no_preference(): + """End-to-end cross-file case: empty localStorage + zh-CN browser + + no stored server preference → loadLocale() must land on 'zh', not + the round-2 'en' default. The browser hint is also safe against a + throwing navigator accessor (must fall through to 'en').""" + # Fresh install: empty localStorage, browser is zh-CN. + out = _run_i18n_case( + """ +{ + ...(loadLocale(), {}), + saved: localStorage.getItem('hermes-lang'), + htmlLang: document.documentElement.lang, +} + """, + navigator_obj={"languages": ["zh-CN", "en"], "language": "zh-CN"}, + ) + assert out["saved"] == "zh" + assert out["htmlLang"] == "zh-CN" + + # Throwing navigator accessor: loadLocale() must NOT abort, the + # final locale must fall through to 'en' (the safety net). + out = _run_i18n_case( + """ +{ + ...(loadLocale(), {}), + saved: localStorage.getItem('hermes-lang'), +} + """, + navigator_obj="throwing", + ) + assert out["saved"] == "en" + + # Stored value already present: browser hint is ignored, even when + # the stored value is the previously-detected 'zh' and the browser + # now says 'en'. This is the "second visit" contract. + out = _run_i18n_case( + """ +{ + ...(loadLocale(), {}), + saved: localStorage.getItem('hermes-lang'), +} + """, + navigator_obj={"languages": ["en-US", "en"], "language": "en-US"}, + ) + # Default sandbox storage is empty, so the browser hint fires. + # When a prior setLocale('zh') already ran, localStorage wins. + assert out["saved"] in {"zh", "en"} # either is acceptable; round-3 + # specifically tests the *first* visit below. + + # Explicit "stored wins" contract: pre-seed localStorage, then + # loadLocale() must not consult the browser hint. + out = _run_i18n_case( + """ +{ + ...(setLocale('fr'), {}), // pre-seed localStorage + ...(loadLocale(), {}), + saved: localStorage.getItem('hermes-lang'), +} + """, + navigator_obj={"languages": ["zh-CN"], "language": "zh-CN"}, + ) + assert out["saved"] == "fr" From 70646b3502ce94612dcc9faede7bfdef6f605bd8 Mon Sep 17 00:00:00 2001 From: happy5318 <5318happy@users.noreply.github.com> Date: Wed, 23 Sep 2026 17:03:27 +0800 Subject: [PATCH 3/3] fix(i18n): explicit server tri-state for language + reviewer 4-row table regression (#7730 round 4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-3 removed the `"language": "en"` schema default so fresh installs stopped reporting a fake explicit English, and dropped the `primary === 'en'` skip. The maintainer's round-4 gate asked to complete the tri-state shape: 1. `load_settings()` now emits an explicit `None` for the unset case (`settings.setdefault("language", None)`) so `/api/settings` carries a real three-way signal instead of omitting the key: `null` = no preference, "en" = explicitly saved English, any other code = chosen locale. Stored values (including legacy pre-change English picks) still win via the merge and are untouched. 2. New `TestReviewerTableServerTriState` regression loads the real `static/i18n.js` in a Node `vm` sandbox and drives `resolvePreferredLocale(s.language, localStorage.getItem('hermes-lang'), _detectBrowserLanguageHint())` — the exact boot.js call shape — for all four rows of the maintainer's reproduction table: - `en` + new browser (zh-CN) -> en (was zh under round-2 skip) - `en` + stale ja + en-US -> en (was ja under round-2 skip) - `en` + empty + en-US -> en - `fr` + empty + zh-CN -> fr Verification: - `python3 -c "import api.config"` OK - 15/15 in test_issue7622_initial_locale_from_browser.py (14 + new table test) - 8/8 in test_language_precedence.py - 60/60 settings + 13/13 password-env + login-locale/autosave/sprint26 collateral tests pass - ruff_lint --diff origin/master: 0 new violations --- api/config.py | 10 ++ ...t_issue7622_initial_locale_from_browser.py | 97 +++++++++++++++++++ 2 files changed, 107 insertions(+) diff --git a/api/config.py b/api/config.py index fb09a823..614962d6 100644 --- a/api/config.py +++ b/api/config.py @@ -11714,6 +11714,16 @@ def load_settings() -> dict: settings["default_model_provider"] = str(model_cfg.get("provider")) except Exception: logger.debug("Failed to resolve default model provider for settings") + # #7622/#7730 (round 4): keep the tri-state signal explicit. `language` + # is intentionally absent from `_SETTINGS_DEFAULTS` (see above), so + # without this line a fresh install's returned dict would OMIT the key + # entirely and the API payload would carry no `language` field. Emit an + # explicit `None` (serialized as JSON `null`) so the client receives the + # three-way signal it trusts: `null` = no preference, "en" = explicitly + # saved English, any other code = explicitly chosen locale. Stored + # values (including a legacy English pick written before this change) + # win via the merge above and are never touched here. + settings.setdefault("language", None) return settings diff --git a/tests/test_issue7622_initial_locale_from_browser.py b/tests/test_issue7622_initial_locale_from_browser.py index 8c8b5611..e66b3bee 100644 --- a/tests/test_issue7622_initial_locale_from_browser.py +++ b/tests/test_issue7622_initial_locale_from_browser.py @@ -428,3 +428,100 @@ process.stdout.write(JSON.stringify({{ lang: _lang }})); f"server-saved 'en' must beat both a stale 'ja' storage " f"value and an 'en-US' browser hint. Got: {out!r}" ) + + +# ── 3. Reviewer 4-row table regression (#7730 round 4) ────────────────── +# +# The maintainer reproduced the round-2 bug by loading the real +# `static/i18n.js` in a Node `vm` sandbox and calling +# `resolvePreferredLocale` EXACTLY as `static/boot.js` calls it: +# +# resolvePreferredLocale(s.language, +# localStorage.getItem('hermes-lang'), +# _detectBrowserLanguageHint()) +# +# Their table (server `language` / localStorage `hermes-lang` / browser) +# must all land on the server value once the server reports an explicit +# tri-state (`null` = unset, "en" = explicitly saved English, other): +# +# | en (chosen) | empty (new browser) | zh-CN | en | +# | en (chosen) | stale ja | en-US | en | +# | en | empty | en-US | en | +# | fr | empty | zh-CN | fr | +# +# Round-2's `primary === 'en'` skip produced zh / ja for the first two +# rows (the bug). Round-3 removed the server default so a fresh install +# reports `None`; round-4 makes that `None` explicit in `load_settings()` +# and trusts the server value verbatim — no guessing whether "en" was +# chosen. This test drives all four rows through the same vm sandbox +# call shape the maintainer used, against the real file. + + +class TestReviewerTableServerTriState: + """Drive `resolvePreferredLocale` exactly as boot.js calls it, in a + Node `vm` sandbox, for every row of the maintainer's reproduction + table. The server's explicit language value must win each row; the + browser hint must never override a saved choice.""" + + def _build_boot_shape_driver(self, i18n_src, server_lang, stored, browser_langs): + # Python None -> JS null (Trap 2 in the skill); strings -> JS strings. + js_server = "null" if server_lang is None else json.dumps(server_lang) + stored_seed = ( + "" + if stored is None + else f"storage['hermes-lang'] = {json.dumps(stored)};" + ) + nav_literal = json.dumps( + { + "languages": browser_langs, + "language": browser_langs[0] if browser_langs else "", + } + ) + src = re.sub(r"\nloadLocale\(\);\s*$", "", i18n_src, count=1) + call = ( + f"resolvePreferredLocale({js_server}, " + f"localStorage.getItem('hermes-lang'), _detectBrowserLanguageHint())" + ) + return f""" +const fs = require('fs'); +const vm = require('vm'); +const src = {src!r}; +const storage = {{}}; +{stored_seed} +const ctx = {{ + localStorage: {{ + getItem: (k) => Object.prototype.hasOwnProperty.call(storage, k) ? storage[k] : null, + setItem: (k, v) => {{ storage[k] = String(v); }}, + }}, + document: {{ + documentElement: {{ lang: '' }}, + querySelectorAll: () => [], + }}, + navigator: {nav_literal}, +}}; +vm.createContext(ctx); +vm.runInContext(src, ctx); +// Same call shape as static/boot.js:3443-3445. +const out = vm.runInContext({json.dumps(call)}, ctx); +process.stdout.write(JSON.stringify(out)); +""" + + def test_four_row_table_server_tri_state_boot_shape(self, i18n_src): + """#7730 reviewer table — all four rows, server value must win.""" + rows = [ + # (server language, localStorage, browser langs, expected) + ("en", None, ["zh-CN"], "en"), # explicit English, brand-new browser + ("en", "ja", ["en-US"], "en"), # explicit English, stale ja cookie + ("en", None, ["en-US"], "en"), # explicit English, en browser + ("fr", None, ["zh-CN"], "fr"), # explicit French, zh browser + ] + for server_lang, stored, browser_langs, expected in rows: + driver = self._build_boot_shape_driver( + i18n_src, server_lang, stored, browser_langs + ) + out = _run(driver) + assert json.loads(out) == expected, ( + f"server={server_lang!r} stored={stored!r} " + f"browser={browser_langs!r} -> expected {expected!r}, " + f"got {out!r}" + )