From 7552033979d402832838bb80ce0881b5755ea464 Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Mon, 21 Sep 2026 19:12:28 -0400 Subject: [PATCH] fix(config): the editable-host census counts every editable tag, not every id MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit, `mobile-web-app-editable-host-font-size.mjs:50`, and right on the code: the pattern started from `id="…"`, so it matched only hosts that carry one. The no-id guard fired for a file with *no* named host at all, which means a file holding a named host beside an anonymous one reported the named one as clean and said nothing about the other. An editable is its tag; the id is read out of the tag afterwards. Also `:145`, also right: the sibling search was `startsWith(directory + '/')`, which reaches the subtree, and the walk stops at the first file whose sheet opens the host's selector. The closure's order is the bundler's rather than alphabetical, so a sheet one directory down could answer for the sibling the host actually gets. Now the immediate directory only. Red first, both cases in the census's own file. The mixed fixture reported one host where two were planted (1 failed, 7 passed); the nested fixture, with the nested sheet first in the closure and a compliant 18 px rule in it, hid a 14 px sibling and reported no offender (1 failed, 8 passed). 9 passed after. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb --- ...mobile-web-app-editable-host-font-size.mjs | 56 ++++++++++++------- ...e-web-app-editable-host-font-size.test.mjs | 51 ++++++++++++++++- 2 files changed, 85 insertions(+), 22 deletions(-) diff --git a/config/scripts/mobile-web-app-editable-host-font-size.mjs b/config/scripts/mobile-web-app-editable-host-font-size.mjs index d112d973c59..04b649c5ed9 100644 --- a/config/scripts/mobile-web-app-editable-host-font-size.mjs +++ b/config/scripts/mobile-web-app-editable-host-font-size.mjs @@ -19,9 +19,17 @@ import { textInputFontSizeFloor } from './mobile-web-app-text-input-font-size-se /** The seam's export, which is how a size states the floor rather than restating the number. */ const SEAM_EXPORT = 'TEXT_INPUT_FONT_SIZE' -/** `contenteditable="true"` in a markup string, with the id the element carries. */ -const EDITABLE_MARKUP = - /id="([A-Za-z][\w-]*)"[^>]*contenteditable="true"|contenteditable="true"[^>]*id="([A-Za-z][\w-]*)"/g +/** + * Each editable tag in a markup string, whole. + * + * The tag is what an editable *is*; its id is optional and is read out of the tag afterwards. A + * pattern that started from the id matched only the hosts that have one, so a file holding a named + * host and an anonymous one reported the named host and said nothing about the other. + */ +const EDITABLE_TAG = /<[A-Za-z][^>]*\bcontenteditable="true"[^>]*>/g + +/** The id a matched tag carries, or null for one that carries none. */ +const TAG_ID = /\bid="([A-Za-z][\w-]*)"/ function readOrNull(path) { try { @@ -32,11 +40,12 @@ function readOrNull(path) { } /** - * Every editable host a closure declares, as `{ file, id }`. + * Every editable host a closure declares, as `{ file, id }`, one entry per tag. * * The completeness half of the verdict below: an empty offender list is only evidence when the - * walk found the editables it is judging. A module that plants an editable with no id lands here - * with `id: null` and is reported as unresolved rather than passing. + * walk found the editables it is judging. A host with no id lands here with `id: null` and is + * reported as unresolved rather than passing, and it does so whether or not a named host sits + * beside it in the same file. */ export function editableHostsIn(mobileDir, closure) { const found = [] @@ -45,13 +54,8 @@ export function editableHostsIn(mobileDir, closure) { if (source === null || !source.includes('contenteditable="true"')) { continue } - const ids = [...source.matchAll(EDITABLE_MARKUP)].map((match) => match[1] ?? match[2] ?? null) - if (ids.length === 0) { - found.push({ file, id: null }) - continue - } - for (const id of ids) { - found.push({ file, id }) + for (const [tag] of source.matchAll(EDITABLE_TAG)) { + found.push({ file, id: TAG_ID.exec(tag)?.[1] ?? null }) } } return found.sort((left, right) => @@ -102,12 +106,19 @@ function ruleFor(source, selector) { return null } -/** What a `font-size` declaration is worth: a literal, a seam substitution, or something else. */ +/** + * What a `font-size` declaration is worth: a literal, a seam substitution, or something else. + * + * Null for a rule that declares no size at all, which is unresolved rather than a pass: the value + * an editable then takes comes from a rule this walk does not read — the host element's own, or the + * page's root — so it can be 14 px and the census cannot prove otherwise. Where the `TextInput` + * half treats an absent prop as inheritance and lets it through, that policy is main's and about a + * prop; this is CSS, and the inherited value is genuinely out of view. + */ function readFontSize(mobileDir, source, declarations) { const match = /font-size:\s*([^;]+);/.exec(declarations) if (match === null) { - // No size of its own, so it inherits, and the floor is about the size an editable declares. - return { text: null, onSeam: true } + return null } const text = match[1].trim() const literal = /^(\d+(?:\.\d+)?)px$/.exec(text) @@ -130,9 +141,9 @@ function readFontSize(mobileDir, source, declarations) { /** * Where each editable host's size is declared, as `{ at, size }`. * - * The size is looked for in the same module the markup came from and in the modules beside it: a - * document's markup and its stylesheet are two exports of one program, so the rule is stated over - * that program's own directory rather than over the whole closure. + * The size is looked for in the same module the markup came from and in the modules directly beside + * it: a document's markup and its stylesheet are two exports of one program, so the rule is stated + * over that program's own directory rather than over the whole closure or over its subtree. */ function editableHostSizes(mobileDir, closure) { const resolutions = [] @@ -142,7 +153,12 @@ function editableHostSizes(mobileDir, closure) { continue } const directory = host.file.slice(0, host.file.lastIndexOf('/')) - const siblings = closure.local.filter((file) => file.startsWith(`${directory}/`)) + // The immediate directory, not the subtree: the walk stops at the first file whose sheet opens + // the host's selector, and the closure's order is the bundler's rather than alphabetical, so a + // sheet one directory down could answer for the sibling the host actually gets. + const siblings = closure.local.filter( + (file) => file.slice(0, file.lastIndexOf('/')) === directory + ) let resolved = null for (const file of siblings) { const source = readOrNull(join(mobileDir, file)) diff --git a/config/scripts/mobile-web-app-editable-host-font-size.test.mjs b/config/scripts/mobile-web-app-editable-host-font-size.test.mjs index cf18bc69223..885b117fc9f 100644 --- a/config/scripts/mobile-web-app-editable-host-font-size.test.mjs +++ b/config/scripts/mobile-web-app-editable-host-font-size.test.mjs @@ -44,6 +44,19 @@ async function fixture(name, markup, style) { return { root, closure: { local: ['src/doc/markup.ts', 'src/doc/style.ts'] } } } +/** The same tree with a second stylesheet one directory down, and a closure that reads it first. */ +async function fixtureWithNested(name, markup, style, nestedStyle) { + const { root } = await fixture(name, markup, style) + await mkdir(join(root, 'src/doc/nested'), { recursive: true }) + await writeFile(join(root, 'src/doc/nested/style.ts'), nestedStyle, 'utf8') + return { + root, + // Nested first, which is what makes this a measurement: the closure's order is the bundler's, + // so a walk that accepted any file under the directory would stop here. + closure: { local: ['src/doc/nested/style.ts', 'src/doc/markup.ts', 'src/doc/style.ts'] } + } +} + beforeAll(async () => { fixtureDir = await mkdtemp(join(tmpdir(), 'orca-editable-host-')) }) @@ -106,6 +119,36 @@ describe('the editable-host font-size rule', () => { expect(editableHostFontSizeOffenders(root, closure)).toEqual(['src/doc/style.ts:3']) }) + it('counts a no-id editable beside a named one, rather than only the named one', async () => { + // The tag is what an editable is, and its id is optional: a walk that started from the id + // matched the named host and never saw the one beside it, so a file holding both reported the + // named one as clean and said nothing at all about the other. + const { root, closure } = await fixture( + 'mixed', + 'export const NAMED = \'
\'\n' + + 'export const ANONYMOUS = \'
\'\n', + 'export function style() {\n return ` #editor {\n font-size: 18px;\n }`\n}\n' + ) + expect(editableHostsIn(root, closure)).toEqual([ + { file: 'src/doc/markup.ts', id: 'editor' }, + { file: 'src/doc/markup.ts', id: null } + ]) + expect(unresolvedEditableHostStyles(root, closure)).toEqual(['src/doc/markup.ts']) + }) + + it('reads the sheet beside the markup, not one a directory down', async () => { + // The walk stops at the first file whose sheet opens `#editor`, and the closure's order is the + // bundler's rather than alphabetical, so a nested sheet could answer for a sibling that is the + // one the host actually gets. + const { root, closure } = await fixtureWithNested( + 'nested', + 'export const MARKUP = \'
\'\n', + 'export function style() {\n return ` #editor {\n font-size: 14px;\n }`\n}\n', + 'export function nested() {\n return ` #editor {\n font-size: 18px;\n }`\n}\n' + ) + expect(editableHostFontSizeOffenders(root, closure)).toEqual(['src/doc/style.ts:2']) + }) + it('reports an editable it cannot judge rather than passing it', async () => { const noId = await fixture( 'no-id', @@ -125,13 +168,17 @@ describe('the editable-host font-size rule', () => { ]) }) - it('passes an editable that declares no size, because it inherits one', async () => { + it('cannot judge an editable that declares no size, and says so', async () => { + // Inheritance is not a pass here. The value would come from a rule in a file this walk does not + // read — the host element's own, or the page's root — so "no declaration" is "cannot say" and + // belongs in the unresolved list, which the closure census holds at empty. const { root, closure } = await fixture( 'inherits', 'export const MARKUP = \'
\'\n', 'export function style() {\n return ` #editor {\n padding: 8px;\n }`\n}\n' ) + expect(unresolvedEditableHostStyles(root, closure)).toEqual(['src/doc/style.ts:2']) + // Not an offender either: an offender is a size this walk read and found under the floor. expect(editableHostFontSizeOffenders(root, closure)).toEqual([]) - expect(unresolvedEditableHostStyles(root, closure)).toEqual([]) }) })