From b461d52981c7bbcfbd640666412c8a07eeafd8d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Salavisa?= Date: Mon, 3 Aug 2026 16:57:40 +0200 Subject: [PATCH] test(i18n): make the pseudo-locale oracle read attribute copy (#2211) Co-authored-by: Kandev Agent Co-authored-by: Claude Opus 5 (1M context) --- .../e2e/tests/i18n/pseudo-coverage.spec.ts | 263 +++++++++++++++--- docs/i18n.md | 81 ++++-- 2 files changed, 293 insertions(+), 51 deletions(-) diff --git a/apps/web/e2e/tests/i18n/pseudo-coverage.spec.ts b/apps/web/e2e/tests/i18n/pseudo-coverage.spec.ts index 7865191c5c..68d1a4dad3 100644 --- a/apps/web/e2e/tests/i18n/pseudo-coverage.spec.ts +++ b/apps/web/e2e/tests/i18n/pseudo-coverage.spec.ts @@ -6,10 +6,16 @@ import { test, expect } from "../../fixtures/test-base"; * Pseudo-locale coverage oracle. * * Under the pseudo-locale every EXTRACTED message is accented - * (`Language` → `Ĺàńĝũàĝē`). So any user-facing text that is still plain ASCII + * (`Language` → `Ĺàńĝũàĝē`). So any user-facing copy that is still plain ASCII * is, by definition, a string that was never wrapped in a Lingui macro. This * spec crawls chrome-heavy screens and reports those leftovers. * + * It runs TWO passes, because copy reaches the user by two routes: + * - visible text nodes, and + * - `COPY_ATTRIBUTES` — the copy a screen-reader user receives and a sighted + * one never sees. That pass is why the oracle is no longer blind to an + * `aria-label` that was never externalized; see docs/i18n.md. + * * `SCREENS` mirrors `i18nGuardFiles` in eslint.i18n.options.mjs: a screen belongs * here once the components that render it have been migrated. The eslint guard * only sees plain literals in JSX, so it and this spec cover different halves of @@ -196,41 +202,190 @@ async function activatePseudo(page: Page, url: string) { } /** - * Collect visible text nodes that look like un-externalized English copy. - * Nodes inside script/style, and nodes that are wholly allowlisted, are ignored. + * Attributes that carry display copy rather than a value the app compares. + * Kept in sync with `jsx-attributes.include` in apps/web/eslint.i18n.options.mjs, + * so the guard and this oracle answer the same question about the same set: on an + * allowlisted path lint flags the literal, and here it is checked at render. + * + * `aria-labelledby` / `aria-describedby` / `aria-controls` are deliberately + * absent — they carry element ids, not prose, and are excluded by the guard too. + * + * `title` is checked on EVERY element, not just interactive ones. A `title` is + * exposed as an accessible name or description whatever the element is, and the + * browser renders its native tooltip on hover for any of them — so "the app never + * shows this one" is not true of a rendered `title`. Restricting it to + * interactive elements would need a tag/role/tabindex heuristic, and that + * heuristic would immediately become the place attribute copy hides. The guard + * checks `title` unconditionally too; the two stay in step. */ -async function findUnlocalizedText(page: Page, allowed: string[]): Promise { - return page.evaluate((allowedList) => { - const skipTags = new Set(["SCRIPT", "STYLE", "NOSCRIPT", "CODE", "PRE", "SVG"]); - const wordlike = /[A-Za-z]{4,}/; - const accented = /[À-ɏ]/; - const found = new Set(); +const COPY_ATTRIBUTES = ["aria-label", "aria-description", "title", "placeholder", "alt"]; - const walker = document.createTreeWalker(document.body, NodeFilter.SHOW_TEXT); - let node: Node | null; - while ((node = walker.nextNode())) { - const text = (node.textContent ?? "").trim(); - if (!text || !wordlike.test(text) || accented.test(text)) continue; +/** + * Collect copy that looks un-externalized: plain-English visible text nodes, and + * plain-English values of the five copy-bearing attributes. + * + * Attribute findings are returned prefixed with the attribute and the element + * that carries it (`aria-label on button[data-testid=…]: "More information"`), + * because an attribute string is invisible on screen — a bare value gives you + * nothing to grep for and nowhere to look. + * + * Two counters make an empty `leftovers` mean something, because a selector that + * matched no elements reports exactly as clean as a clean screen: + * - `inspectedAttributes` — non-empty attribute values examined. Asserted per + * screen; this is the one that catches a selector matching nothing. + * - `localizedAttributes` — those that rendered FULLY accented, i.e. migrated + * attribute copy the pass demonstrably reached. Not asserted per screen: a + * page of plain inputs and text can legitimately have none, and secrets, + * terminal and sprites do. Pinned once instead, in its own test below. + */ +async function findUnlocalizedCopy( + page: Page, + allowed: string[], +): Promise<{ leftovers: string[]; localizedAttributes: string[]; inspectedAttributes: number }> { + return page.evaluate( + ({ allowedList, copyAttributes }) => { + const skipTags = new Set(["SCRIPT", "STYLE", "NOSCRIPT", "CODE", "PRE", "SVG"]); + // A narrower set for attributes. `skipTags` above encodes "this element's + // CONTENT is not prose", which is a different question from whether its + // LABEL is: an `aria-label` on an icon, or a `title` on a + // chip, is copy the user receives either way. Only the three tags that + // cannot present a label at all are skipped here. + const attrSkipTags = new Set(["SCRIPT", "STYLE", "NOSCRIPT"]); + const wordlike = /[A-Za-z]{4,}/; + const accented = /[À-ɏ]/; - const el = node.parentElement; - if (!el || skipTags.has(el.tagName)) continue; - // Ignore hidden nodes — not user-visible copy. - const rect = el.getBoundingClientRect(); - if (rect.width === 0 && rect.height === 0) continue; - - // Strip allowlisted tokens; if nothing word-like remains, it's fine. // Longest first: stripping "VS Code" before "Agent and VS Code side by // side" would leave "side by side" behind and report it as a leftover. - let residue = text; - for (const token of [...allowedList].sort((a, b) => b.length - a.length)) { - residue = residue.split(token).join(" "); - } - if (!wordlike.test(residue)) continue; + const tokens = [...allowedList].sort((a, b) => b.length - a.length); - found.add(text.slice(0, 120)); - } - return [...found]; - }, allowed); + /** Still word-like ASCII once allowlisted tokens are removed. */ + const hasUnmigratedAscii = (text: string) => { + if (!text || !wordlike.test(text)) return false; + let residue = text; + for (const token of tokens) residue = residue.split(token).join(" "); + return wordlike.test(residue); + }; + + /** The text pass's rule, unchanged: any accented character clears a node. */ + const looksUnlocalized = (text: string) => !accented.test(text) && hasUnmigratedAscii(text); + + const collectText = () => { + const found = new Set(); + const walker = document.createTreeWalker(document.body, NodeFilter.SHOW_TEXT); + let node: Node | null; + while ((node = walker.nextNode())) { + const text = (node.textContent ?? "").trim(); + if (!looksUnlocalized(text)) continue; + + const el = node.parentElement; + if (!el || skipTags.has(el.tagName)) continue; + // Ignore hidden nodes — not user-visible copy. + const rect = el.getBoundingClientRect(); + if (rect.width === 0 && rect.height === 0) continue; + + found.add(text.slice(0, 120)); + } + return [...found]; + }; + + /** Enough to find the element in the DOM and in the source. */ + const describe = (el: Element) => { + const tag = el.tagName.toLowerCase(); + const testId = el.getAttribute("data-testid"); + if (testId) return `${tag}[data-testid=${testId}]`; + if (el.id) return `${tag}#${el.id}`; + const role = el.getAttribute("role"); + return role ? `${tag}[role=${role}]` : tag; + }; + + // The text pass drops zero-size nodes because nothing is on screen to read. + // That test is WRONG for an attribute: a visually hidden control with an + // `aria-label` is precisely the case this pass exists to catch, and every + // `sr-only` node measures as good as zero. What actually disqualifies a + // label is the element not being rendered at all — `display: none`, + // `visibility: hidden`, or a skipped `content-visibility` subtree — because + // then no user receives it, sighted or not. `checkVisibility` asks that + // directly; deliberately WITHOUT `opacityProperty`, since an opacity-0 + // element is still in the accessibility tree. + const isRendered = (el: Element) => + typeof el.checkVisibility !== "function" || + el.checkVisibility({ contentVisibilityAuto: true, visibilityProperty: true }); + + const seen = new Map(); + // The positive control. An attribute that HAS been migrated renders as + // `Mōŕē ĩńfōŕmàţĩōń`, which contains no 4-letter ASCII run — so it is + // dropped at the `wordlike` gate and never even reaches the `accented` + // test. Soundness is unaffected (a real miss is pure ASCII and is still + // caught), but it means a working pass and a pass whose selector matched + // NOTHING produce identical empty output. Collecting the accented values + // proves the pass actually reached migrated attribute copy on this + // screen, so the caller can tell those two apart. + const localized: string[] = []; + let inspected = 0; + + const inspectAttribute = (el: Element, attr: string) => { + // An empty value is never a missed string, and `alt=""` is load-bearing: + // it marks an image as decorative so screen readers skip it. Reporting + // one would push authors into writing alt text for spacers. + const value = (el.getAttribute(attr) ?? "").trim(); + if (!value) return; + inspected += 1; + + // NOTE the divergence from the text pass, which clears a node the moment + // it contains one accented character. That rule hides a real shape here: + // an `aria-label` built as "Collapse ${label}" renders + // "Collapse Ĝēńēŕàĺ" — an un-migrated English FRAME around a migrated + // value, which accent-clears-all would call done. Allowlist stripping is + // what separates it from the legitimate case, where the ASCII fragment is + // the allowlisted DATA rather than the frame ("Àćţĩōńś ƒōŕ Agent" on the + // layouts screen strips to nothing). + if (!hasUnmigratedAscii(value)) { + // Fully accented, nothing ASCII left: a migrated attribute, and the + // positive control. + if (accented.test(value)) { + localized.push(`${attr} on ${describe(el)}: "${value.slice(0, 60)}"`); + } + return; + } + + // Keyed by attribute+value, not by element: one un-migrated shared + // component renders on twenty rows, and twenty identical findings bury + // the other nineteen strings. The first element carrying it is the + // sample you go and look at. + const key = `${attr} ${value}`; + const hit = seen.get(key); + if (hit) { + hit.count += 1; + return; + } + const label = `${attr} on ${describe(el)}: "${value.slice(0, 120)}"`; + seen.set(key, { + label: accented.test(value) ? `${label} - English frame, migrated value` : label, + count: 1, + }); + }; + + const collectAttributes = () => { + const selector = copyAttributes.map((attr) => `[${attr}]`).join(","); + for (const el of Array.from(document.querySelectorAll(selector))) { + if (attrSkipTags.has(el.tagName.toUpperCase()) || !isRendered(el)) continue; + for (const attr of copyAttributes) inspectAttribute(el, attr); + } + const leftovers = [...seen.values()].map(({ label, count }) => + count > 1 ? `${label} (×${count})` : label, + ); + return { leftovers, localized, inspected }; + }; + + const attributes = collectAttributes(); + return { + leftovers: [...collectText(), ...attributes.leftovers], + localizedAttributes: attributes.localized, + inspectedAttributes: attributes.inspected, + }; + }, + { allowedList: allowed, copyAttributes: COPY_ATTRIBUTES }, + ); } test.describe("i18n pseudo-locale coverage", () => { @@ -245,11 +400,55 @@ test.describe("i18n pseudo-locale coverage", () => { // Let lazy panels settle before scanning. await testPage.waitForTimeout(1_000); - const leftovers = await findUnlocalizedText(testPage, [...ALLOWED, ...(screen.allow ?? [])]); + const { leftovers, localizedAttributes, inspectedAttributes } = await findUnlocalizedCopy( + testPage, + [...ALLOWED, ...(screen.allow ?? [])], + ); + + // Control FIRST. A migrated attribute renders accented and so is invisible + // to the leftover check by construction, which means a screen that is + // clean and a selector that matched NOTHING report identically. Assert the + // pass actually read attributes before believing what it did not find. + expect( + inspectedAttributes, + `The attribute pass examined no attribute at all on ${screen.name}, so a ` + + `green leftover check proves nothing about attribute copy here.`, + ).toBeGreaterThan(0); + expect( leftovers, - `Un-externalized strings on ${screen.name}:\n${leftovers.map((s) => ` - ${s}`).join("\n")}`, + `Un-externalized strings on ${screen.name}:\n${leftovers.map((s) => ` - ${s}`).join("\n")}` + + `\n\nAttribute pass examined ${inspectedAttributes} attribute(s), of which ` + + `${localizedAttributes.length} rendered fully accented.`, ).toEqual([]); }); } + + /** + * The positive control, pinned once rather than per screen. + * + * `inspectedAttributes` above proves the selector matched; it does not prove + * the pass can tell migrated attribute copy apart from a miss. This does, on + * the one element we know is migrated: `language-settings.tsx` renders + * `aria-label={t("settings:displayLanguage")}` on `#language-select`, so under + * pseudo it must come back accented. If this test fails while the screens + * above pass, the accent detection is broken and every green screen is + * meaningless — which is the failure this whole file exists to not have. + * + * A per-screen version of this assertion was tried and dropped: secrets, + * terminal and sprites legitimately render no fully-accented attribute at all, + * so it fired on three screens that were fine. + */ + test("the attribute pass recognizes migrated attribute copy", async ({ testPage }) => { + await activatePseudo(testPage, "/settings/general/appearance"); + await testPage.waitForTimeout(1_000); + + const { localizedAttributes } = await findUnlocalizedCopy(testPage, ALLOWED); + + expect( + localizedAttributes.join("\n"), + `Expected the migrated aria-label on #language-select to render accented. Got:\n` + + localizedAttributes.join("\n"), + ).toContain("language-select"); + }); }); diff --git a/docs/i18n.md b/docs/i18n.md index b1e60a91b3..1af0024421 100644 --- a/docs/i18n.md +++ b/docs/i18n.md @@ -424,33 +424,76 @@ One thing it shows you is **not** a miss: an interpolated API or network payload stays English by design, so it renders unaccented and is still correct. See [the interpolated-value limit](#and-it-is-weakest-at-an-interpolated-value). -Three things it cannot show you at all — the second and third are detailed below: +Two things it cannot show you at all — the second is detailed below: 1. **Module-scope `t()`.** That copy *is* translated, just frozen at the boot locale, so it renders accented and looks correct while never responding to a switch. `check-module-scope-t.mjs` is what catches it. -2. **Copy that never becomes a text node** — an attribute string leaves no trace - on screen at all. -3. **An un-migrated value interpolated into a migrated frame** — it renders +2. **An un-migrated value interpolated into a migrated frame** — it renders inside accented copy and reads as done. -### It cannot see copy that is not a text node +A third used to belong on that list — copy that never becomes a text node — and +the spec now reads it directly. That pass is described next, because what it does +*not* check still matters when you migrate. -`findUnlocalizedText` in `e2e/tests/i18n/pseudo-coverage.spec.ts` walks -`NodeFilter.SHOW_TEXT`, and a human looking at the screen is no different: -**copy carried in an attribute — `aria-label`, `aria-description`, `title`, -`placeholder`, `alt` — renders nothing visible and leaves no trace under -pseudo.** The Settings → System → Storage review demonstrated it by reverting an -`aria-label` to an English template literal; nothing failed. +### It reads attribute copy, which nothing on screen shows you -The guard is the primary defence here, not the oracle: those five attributes are -in `jsx-attributes.include`, so on an allowlisted path the lint rule does flag -them. The consequence is that for attribute copy there is **no second check** — on -a path not yet allowlisted, or for a fragment the `words.exclude` patterns skip, -nothing downstream catches it. The copy only screen-reader users receive is -therefore the copy with the weakest safety net. Read attribute props by eye when -you migrate, and prefer a `t()` call over a template literal in them so the value -stays greppable. +**Copy carried in an attribute — `aria-label`, `aria-description`, `title`, +`placeholder`, `alt` — renders nothing visible, so a human eyeballing the +pseudo-locale can never catch it.** The Settings → System → Storage review +demonstrated the gap by reverting an `aria-label` to an English template literal: +nothing failed. + +`findUnlocalizedCopy` in `e2e/tests/i18n/pseudo-coverage.spec.ts` therefore runs a +second pass over `document.querySelectorAll("[aria-label],…")` alongside the +`NodeFilter.SHOW_TEXT` walk, applying the same accented/word-like tests and the +same allowlist stripping. Findings name the attribute and the element that carries +it, since a bare string gives you nowhere to look: + +```text +aria-label on button#language-select: "Display language (pseudo)" +``` + +**Read the control line before you trust a green screen.** A migrated attribute +renders as `Mōŕē ĩńfōŕmàţĩōń`, which has no 4-letter ASCII run and so is dropped +at the `wordlike` gate before the accented test ever sees it. Soundness is fine — +a real miss is pure ASCII — but it means a working pass and a pass whose selector +matched *nothing* produce the same empty output. One agent scanning by hand +concluded their scan was broken on exactly this. The spec therefore asserts +`inspectedAttributes > 0` per screen, prints how many rendered fully accented, and +pins accent detection once against `#language-select`'s known-migrated +`aria-label`. If you scan by hand, emit a known-migrated attribute as your own +positive control. + +Four things about that pass are deliberate, and worth knowing before you read its +output: + +- **It does not reuse the text pass's zero-size-rect check.** A visually hidden + control with an `aria-label` is the exact case the pass exists to catch, and + every `sr-only` node measures as good as zero. It uses `checkVisibility()` + instead — `display: none`, `visibility: hidden` and skipped `content-visibility` + subtrees are dropped, because then nobody receives the label; opacity and + off-screen positioning are kept, because the accessibility tree keeps them. +- **An empty value is never reported.** `alt=""` is load-bearing markup — it marks + an image as decorative so screen readers skip it — and flagging it would push + authors into writing alt text for spacers. +- **Findings are keyed by attribute and value, not by element**, with a `(×N)` + count. One un-migrated shared component rendered on twenty rows would otherwise + bury every other string on the screen. +- **One accented character does not clear an attribute**, unlike the text pass. + An `aria-label` built as `` `Collapse ${label}` `` renders `Collapse Ĝēńēŕàĺ` — + an un-migrated English *frame* around a migrated value, which accent-clears-all + would call done. Allowlist stripping is what separates that from the legitimate + case, where the ASCII fragment is allowlisted *data* rather than the frame + (`Àćţĩōńś ƒōŕ Agent` on the layouts screen strips to nothing). Such findings are + tagged `— English frame, migrated value`. + +The guard is still the primary defence: those five attributes are in +`jsx-attributes.include`, so on an allowlisted path the lint rule flags the +literal before it ever renders. The oracle is the second check that attribute copy +previously had none of — on a path not yet allowlisted, or for a fragment the +`words.exclude` patterns skip. Prefer a `t()` call over a template literal in an +attribute either way, so the value stays greppable. ### …and it is weakest at an interpolated value