Skip to content

fix(#404): cover every locale tag the sanitizer handles, and what it leaves behind - #406

Open
Matobi98 wants to merge 1 commit into
MostroP2P:mainfrom
Matobi98:fix/404-smoke-locale-matrix
Open

fix(#404): cover every locale tag the sanitizer handles, and what it leaves behind#406
Matobi98 wants to merge 1 commit into
MostroP2P:mainfrom
Matobi98:fix/404-smoke-locale-matrix

Conversation

@Matobi98

@Matobi98 Matobi98 commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

fix(#404): cover every locale tag the sanitizer handles, and what it leaves behind

Closes #404. Follow-up to #370, from @grunch's third finding there.

No app code changes: lib/, web/ and rust/ are untouched, and the bundle that ships is byte-identical to main's. What changes is what CI checks before letting a change through.

The gap

The locale guard added in #370 only ever exercised C. Playwright's locale option normalizes the tag before the page sees it — en_US arrives as a valid en-US, "" falls back to the system locale — so two of the three tags named in #227's acceptance criterion were handled by the sanitizer but never reached it in CI. The mixed list (["C","es-AR"]["es-AR"]) was tested by nothing at all.

That is a limit of that one Playwright option, not of the browser. addInitScript runs inside the page before any of its own scripts, so it can hand the engine a tag Playwright would never deliver.

What this adds

SMOKE_NAVIGATOR_LANGUAGES shadows navigator.language(s) from an init script. Comma-separated, used verbatim, unset means "do not touch" — so every existing run is unaffected. !== undefined rather than a truthiness check, because the empty string is one of the broken tags. configurable: true is load-bearing: the sanitizer bails out when either property is locked down, so a non-configurable shadow would make it skip the very path under test and pass for the wrong reason.

SMOKE_EXPECT_LANGUAGES asserts what navigator.languages reads after the sanitizer ran. This is what makes the mixed list worth running — see below.

The matrix in web-build.yml runs C, en_US, "" and C,es-AR, each against the shipped bundle (must pass, with the expected locale) and against the control bundle (must fail with a locale error).

The control bundle is now built once in its own step instead of being rebuilt inside the control, since two places would otherwise duplicate the strip and drift. The "did the strip actually happen" check moved with it: if the marker is ever renamed, the cut is a no-op and every control silently compares the bundle against itself.

The SMOKE_LOCALE=C step and its control stay. It is the one path where the tag reaches the page as the browser's own locale rather than as a shadow this repo installed.

Two things this turned up

1. The empty tag crashes with a different message. Three of the four controls die with Incorrect locale information provided; the empty one dies with First argument to Intl.Locale constructor can't be empty or missing. Both are the engine refusing a locale before runApp, but the control introduced in #370 required the first string exactly — so the empty case would have reported "the control failed, but not with the locale RangeError", a false negative on a control that worked perfectly.

That check is not wrong today: it only ever runs C, and for C the signature is right. It becomes too narrow the moment this PR starts feeding it the other tags, so widening it belongs here rather than in a follow-up. The accepted signatures are now a job-level LOCALE_CRASH_SIGNATURE, kept exact rather than loosened to something like locale — a lax pattern is what lets a control celebrate an unrelated crash.

2. "The view mounted" cannot judge the mixed list. With C,es-AR, a sanitizer that keeps es-AR and one that drops the whole list for the fallback both boot perfectly; only the second silently costs the user their language. Without reading the result back, that case tests nothing C does not already test.

Hence SMOKE_EXPECT_LANGUAGES. Mutation-checked: patching the sanitizer to discard the whole list whenever any tag is invalid leaves C, en_US and "" green and fails only the mixed case, with navigator.languages is "en-US", expected "es-AR".

Verified

Against a release bundle built on this branch (build-web.sh --release + flutter build web --release, Flutter 3.38.2), running the matrix exactly as the workflow does:

tag expected after with sanitizer without
C en-US passes dies with the locale signature
en_US en-US passes dies with the locale signature
"" en-US passes dies with the locale signature
C,es-AR es-AR passes dies with the locale signature

Eight for eight. Plus pages_bundle_test.dart 17/17 and selftest.mjs 3/3, both of which cover this workflow and this script.

Cost

Four extra control runs at ~20s each — each waits out SMOKE_TIMEOUT_MS for a view that will never mount — so roughly 80s added per PR. The positive runs are ~1s each. That is the price of the guard being self-validating; the alternative is a matrix that can go green for the wrong reason.

For #227

Its second acceptance criterion carries a caveat that CI only reaches C. Once this lands that no longer holds and the caveat can go — happy to edit it, or leave it to whoever owns the issue.

@coderabbitai

coderabbitai Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: bcb304dd-bff7-49a9-9a02-d0395101071e


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Web smoke test: shadow navigator.languages so CI can cover en_US, "" and mixed locale lists

1 participant