From 85bb65d5088c6c9e7ac46c09cc42e72ac4d478ca Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 25 Sep 2026 00:09:01 +0000 Subject: [PATCH 1/4] fix(components): required asterisk is a real aria-hidden span, not CSS generated content (objectui#10368) The six labelled form renderers drew the required marker as a Tailwind `::after` content utility on the control's associated label. The accessible-name computation includes generated content, so Chromium named a required field labelled "Title" as "Title*". A pseudo-element cannot carry `aria-hidden`, so the marker is now a real `span aria-hidden="true"` with `data-required-marker`, the shape the form renderer's FormLabel already uses. The FieldContainer comment and the CheckboxSchema.required docblock that described the old style are corrected. The two pins that asserted the clean name under happy-dom (which computes no generated content) now carry their verdict on the markup; a new test covers all six sites plus a source guard. Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_01BP8CMtACxTdLjqR6rhd33C --- .../field-container-aria-required.test.tsx | 52 +++- .../__tests__/required-marker-markup.test.tsx | 226 ++++++++++++++++++ ...ext-input-description-association.test.tsx | 47 +++- packages/components/src/custom/field.tsx | 33 ++- .../src/renderers/basic/text-input.tsx | 19 +- .../src/renderers/form/checkbox.tsx | 13 +- .../components/src/renderers/form/input.tsx | 16 +- .../components/src/renderers/form/select.tsx | 17 +- .../src/renderers/form/textarea.tsx | 16 +- packages/types/src/form.ts | 11 +- 10 files changed, 421 insertions(+), 29 deletions(-) create mode 100644 packages/components/src/__tests__/required-marker-markup.test.tsx diff --git a/packages/components/src/__tests__/field-container-aria-required.test.tsx b/packages/components/src/__tests__/field-container-aria-required.test.tsx index 5a79bd0d13..15a81eb90a 100644 --- a/packages/components/src/__tests__/field-container-aria-required.test.tsx +++ b/packages/components/src/__tests__/field-container-aria-required.test.tsx @@ -12,8 +12,11 @@ * the form renderer). * * `FieldContainer` computed `required` and spent it on exactly one thing: the - * label's CSS pseudo-element asterisk (`after:content-['*']`) — pure paint - * that never enters the accessibility tree as a state. Meanwhile its Slot + * label's visual asterisk — paint, never a STATE. (That asterisk did reach the + * accessible NAME: it was CSS generated content, which the name computation + * includes, so Chromium named the control "Title*". objectui#10368 made it a + * real `aria-hidden` element; the last case below pins that by markup, and + * says why a happy-dom name could never have caught it.) Meanwhile its Slot * injection already delivered `id` / `aria-describedby` / `aria-invalid` to * whatever control the caller slots in, so the container was ALREADY the * single a11y-wiring authority for its children — it just skipped this one @@ -89,18 +92,51 @@ describe('FieldContainer — `aria-required` rides the Slot injection (objectui# expect(ctl).toHaveAttribute('aria-describedby', 'my-field-error'); }); - it('keeps the label association intact so the name and the state stay separate channels', () => { + it('keeps the asterisk out of the name: a real aria-hidden element, never CSS generated content (objectui#10368)', () => { + // REWRITTEN VERDICT (objectui#10368). This case used to rest on + // `toHaveAccessibleName('Title')` under happy-dom, beside a comment saying + // the CSS asterisk "can never leak into the accessible name". Both were + // wrong in the same direction: the accessible-name computation INCLUDES + // `::after` content, and Chromium named this control "Title*". The case was + // green only because happy-dom computes no generated content, so it could + // not go red on the very defect it claimed to exclude. + // + // The verdict is now the MARKUP, which fails on that defect in any DOM: + // 1. nothing in the associated label draws generated content — a Tailwind + // `content-[…]` / `content-(…)` utility, bare or behind a variant; + // 2. the visible `*` is a real element carrying `aria-hidden="true"`; + // 3. the label's text outside `aria-hidden` subtrees is exactly the label. render( , ); - // The asterisk is a CSS pseudo-element (`after:content-['*']`), so it can - // never leak into the accessible name — the state channel is the ONLY - // required signal, exactly the converged shape. - const ctl = screen.getByLabelText('Title'); - expect(ctl).toHaveAccessibleName('Title'); + const label = document.querySelector('label[for="assoc-field"]') as HTMLLabelElement; + const ctl = document.getElementById('assoc-field') as HTMLInputElement; + expect(label).not.toBeNull(); + + const generated = [label, ...Array.from(label.querySelectorAll('*'))] + .flatMap((el) => Array.from(el.classList)) + .filter((c) => /(^|:)content-[[(]/.test(c)); + expect(generated).toEqual([]); + + const markers = label.querySelectorAll('[data-required-marker]'); + expect(markers).toHaveLength(1); + expect(markers[0]).toHaveAttribute('aria-hidden', 'true'); + expect(markers[0].textContent).toBe('*'); + + const clone = label.cloneNode(true) as HTMLElement; + clone.querySelectorAll('[aria-hidden="true"]').forEach((el) => el.remove()); + expect(clone.textContent).toBe('Title'); + + // The state channel is still the ONLY required signal. expect(ctl).toHaveAttribute('aria-required', 'true'); + + // Corroboration, NOT the evidence: `dom-accessibility-api` honours + // `aria-hidden` on a REAL element, so this line does go red if the span + // loses its `aria-hidden`. It still cannot see generated content — it was + // green on the defect — which is why the markup above carries the verdict. + expect(ctl).toHaveAccessibleName('Title'); }); }); diff --git a/packages/components/src/__tests__/required-marker-markup.test.tsx b/packages/components/src/__tests__/required-marker-markup.test.tsx new file mode 100644 index 0000000000..f185df35c7 --- /dev/null +++ b/packages/components/src/__tests__/required-marker-markup.test.tsx @@ -0,0 +1,226 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * The required asterisk on a labelled control is a REAL `aria-hidden` element, + * never CSS generated content (objectui#10368). + * + * ## The defect + * + * Six sites in this package drew the marker as a Tailwind `::after` content + * utility on the control's label. The accessible-name computation includes CSS + * generated content, so in Chromium a required field labelled "Title" was + * NAMED "Title*". A pseudo-element cannot carry `aria-hidden`, so there was no + * way to keep that paint out of the name short of making it an element. + * + * ## Why these cases read MARKUP, not a name + * + * happy-dom does not compute generated content. A name computed here was + * "Title" on the defective build too — two pins in this package said so, and + * were green for exactly that reason. So a happy-dom name is never the evidence + * in this file. Each case reads the markup instead, which fails on the defect + * in any DOM: + * + * 1. nothing in the label draws generated content: no Tailwind `content-[…]` + * / `content-(…)` utility, bare or behind a variant; + * 2. the visible `*` is ONE real element carrying `aria-hidden="true"` + * (located by `data-required-marker`, the form renderer's locator); + * 3. the label's text outside `aria-hidden` subtrees is exactly the label; + * 4. the control still carries the required STATE it carried before — this + * change moves paint, never validation. + * + * The before/after Chromium accessibility-tree reading on the compiled sheet is + * a one-off measurement recorded on the pull request; nothing in this repo + * re-derives it (this package has no browser test harness), said here rather + * than left to read as live (AGENTS.md #9). + * + * The last block is a SOURCE guard over this package's `src`: no generated- + * content asterisk utility anywhere, so a seventh site cannot bring the defect + * back where no per-site case is looking. + */ + +import { describe, it, expect, afterEach } from 'vitest'; +import { render, screen, cleanup } from '@testing-library/react'; +import { readdirSync, readFileSync } from 'node:fs'; +import { dirname, join, relative, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { FieldContainer } from '../custom/field'; +import { renderComponent } from './test-utils'; +// Registers the renderers at module scope, not in a hook +// (object-ui/no-dynamic-import-in-test-hook, objectui#3010). +import '../renderers'; + +afterEach(cleanup); + +/** Every class token on `root` or below it that draws CSS generated content. */ +function generatedContentClasses(root: Element): string[] { + return [root, ...Array.from(root.querySelectorAll('*'))] + .flatMap((el) => Array.from(el.classList)) + .filter((c) => /(^|:)content-[[(]/.test(c)); +} + +/** The label's text with every `aria-hidden="true"` subtree removed. */ +function textOutsideAriaHidden(node: Node): string { + if (node.nodeType === Node.TEXT_NODE) return node.textContent ?? ''; + if (node instanceof Element && node.getAttribute('aria-hidden') === 'true') return ''; + return Array.from(node.childNodes).map(textOutsideAriaHidden).join(''); +} + +interface Site { + site: string; + render: (required: boolean) => void; + /** The label that carries the marker. */ + label: () => HTMLElement | null; + /** The control, and the required STATE it must carry. */ + expectRequiredState: () => void; +} + +const SITES: Site[] = [ + { + site: 'FieldContainer (custom/field.tsx)', + render: (required) => + render( + + + , + ), + label: () => document.querySelector('label[for="fc-ctl"]'), + expectRequiredState: () => { + const ctl = document.getElementById('fc-ctl') as HTMLInputElement; + expect(ctl).toHaveAttribute('aria-required', 'true'); + // Unchanged by objectui#10368: still no native `required` (#3290). + expect(ctl).not.toHaveAttribute('required'); + }, + }, + { + site: 'element:text_input (renderers/basic/text-input.tsx)', + render: (required) => + renderComponent({ type: 'element:text_input', id: 'ti-ctl', properties: { label: 'Title', required } } as any), + label: () => document.querySelector('label[for="ti-ctl"]'), + expectRequiredState: () => expect(document.getElementById('ti-ctl')).toHaveAttribute('required'), + }, + { + site: 'input (renderers/form/input.tsx)', + render: (required) => renderComponent({ type: 'input', id: 'in-ctl', label: 'Title', required } as any), + label: () => document.querySelector('label[for="in-ctl"]'), + expectRequiredState: () => expect(document.getElementById('in-ctl')).toHaveAttribute('required'), + }, + { + site: 'textarea (renderers/form/textarea.tsx)', + render: (required) => renderComponent({ type: 'textarea', id: 'ta-ctl', label: 'Title', required } as any), + label: () => document.querySelector('label[for="ta-ctl"]'), + expectRequiredState: () => expect(document.getElementById('ta-ctl')).toHaveAttribute('required'), + }, + { + site: 'checkbox (renderers/form/checkbox.tsx)', + render: (required) => renderComponent({ type: 'checkbox', id: 'cb-ctl', label: 'Title', required } as any), + label: () => document.querySelector('label[for="cb-ctl"]'), + // Radix writes `aria-required` on the role=checkbox button from `required`. + expectRequiredState: () => expect(screen.getByRole('checkbox')).toHaveAttribute('aria-required', 'true'), + }, + { + site: 'select (renderers/form/select.tsx)', + render: (required) => + renderComponent({ + type: 'select', + id: 'sel-ctl', + label: 'Title', + required, + options: [{ label: 'A', value: 'a' }], + } as any), + // This label carries no `for` today. Whether it SHOULD name the trigger is + // outside objectui#10368 and deliberately not pinned either way here; the + // marker must be an element regardless, because it would enter the name + // the moment the label is associated. + label: () => document.querySelector('label'), + // Radix writes `aria-required` on the combobox trigger from `required`. + expectRequiredState: () => expect(screen.getByRole('combobox')).toHaveAttribute('aria-required', 'true'), + }, +]; + +describe('required marker is a real aria-hidden element, never CSS generated content (objectui#10368)', () => { + describe.each(SITES)('$site', ({ render: renderSite, label, expectRequiredState }) => { + it('required: draws the asterisk as one aria-hidden element and no generated content', () => { + renderSite(true); + const el = label(); + expect(el).not.toBeNull(); + + expect(generatedContentClasses(el!)).toEqual([]); + + const markers = el!.querySelectorAll('[data-required-marker]'); + expect(markers).toHaveLength(1); + expect(markers[0]).toHaveAttribute('aria-hidden', 'true'); + expect(markers[0].textContent).toBe('*'); + + expect(textOutsideAriaHidden(el!)).toBe('Title'); + + expectRequiredState(); + }); + + it('optional: draws no marker at all', () => { + renderSite(false); + const el = label(); + expect(el).not.toBeNull(); + + expect(generatedContentClasses(el!)).toEqual([]); + expect(el!.querySelector('[data-required-marker]')).toBeNull(); + expect(el!.textContent).toBe('Title'); + }); + }); +}); + +describe('source guard: no generated-content asterisk anywhere in this package (objectui#10368)', () => { + // Rooted on this file, never on `process.cwd()` (AGENTS.md, objectui#7791). + const SRC = resolve(dirname(fileURLToPath(import.meta.url)), '..'); + // A Tailwind content utility whose value starts with an asterisk, quoted or + // not, with `_` / spaces allowed before it: every spelling the six sites + // used, and the obvious respellings of it. + const ASTERISK_CONTENT = /content-\[\s*['"]?[_ ]*\*/; + + function sourceFiles(dir: string): string[] { + return readdirSync(dir, { withFileTypes: true }).flatMap((d) => { + const p = join(dir, d.name); + if (d.isDirectory()) return d.name === '__tests__' || d.name === 'node_modules' ? [] : sourceFiles(p); + return /\.(ts|tsx)$/.test(d.name) && !/\.test\.(ts|tsx)$/.test(d.name) ? [p] : []; + }); + } + + it('scans a real population', () => { + // The control that makes the zero below a reading: the files that held + // the six sites are in the scanned set. + const files = sourceFiles(SRC).map((f) => relative(SRC, f).split('\\').join('/')); + for (const f of [ + 'custom/field.tsx', + 'renderers/basic/text-input.tsx', + 'renderers/form/input.tsx', + 'renderers/form/textarea.tsx', + 'renderers/form/checkbox.tsx', + 'renderers/form/select.tsx', + ]) { + expect(files).toContain(f); + } + }); + + it('the pattern matches every spelling it guards', () => { + for (const hit of [`after:content-['*']`, `before:content-["*"]`, `after:content-['_*']`, `after:content-[*]`]) { + expect(ASTERISK_CONTENT.test(hit)).toBe(true); + } + expect(ASTERISK_CONTENT.test('prose-code:after:content-none')).toBe(false); + }); + + it('finds no generated-content asterisk in any non-test source file', () => { + const offenders = sourceFiles(SRC).flatMap((f) => + readFileSync(f, 'utf8') + .split('\n') + .map((line, i) => ({ line, i })) + .filter(({ line }) => ASTERISK_CONTENT.test(line)) + .map(({ line, i }) => `${relative(SRC, f)}:${i + 1}: ${line.trim()}`), + ); + expect(offenders).toEqual([]); + }); +}); diff --git a/packages/components/src/__tests__/text-input-description-association.test.tsx b/packages/components/src/__tests__/text-input-description-association.test.tsx index 5f810d1fc6..56986b4b58 100644 --- a/packages/components/src/__tests__/text-input-description-association.test.tsx +++ b/packages/components/src/__tests__/text-input-description-association.test.tsx @@ -34,10 +34,15 @@ * than on the text being unreachable, which was measured and is now false. * * Second limit, worth stating because it is the one that could mislead: CSS - * generated content does not exist in happy-dom, so the `required` asterisk - * (`after:content-['*']` on the `Label`) is invisible to every name/description - * computation here. It IS part of the accessible name in a real browser. No - * case below depends on that either way. + * generated content does not exist in happy-dom, so NO name or description + * computed here can be evidence about it, either way. While the `required` + * asterisk was an `::after` utility on the `Label`, every computation here was + * blind to it, and Chromium appended it to the required input's name — "Title*" + * for a label "Title" (objectui#10368). It is now a real `aria-hidden` element, + * and the required case below pins it by MARKUP (no generated-content utility + * in the label, an `aria-hidden` `*` element) — never by a happy-dom name. + * The other cases author no `required`, so the asterisk never touched their + * names. * * ## Why the renderer is driven through the registry, not `SchemaRenderer` * @@ -226,6 +231,40 @@ describe('element:text_input — description is the field\'s accessible descript expect(input).toHaveAccessibleDescription('负责人'); }); + it('keeps a required label\'s asterisk out of both channels, pinned by markup (objectui#10368)', () => { + // The verdict is the MARKUP, not a happy-dom name (see the header's second + // limit): happy-dom computes no CSS generated content, so a name read here + // stayed green while Chromium put the asterisk into the input's name. + renderInput({ label: 'Workspace', description: HELP, required: true }); + + const input = screen.getByRole('textbox'); + const label = document.querySelector('label[for="ws_input"]') as HTMLLabelElement; + expect(label).not.toBeNull(); + + // 1. Nothing in the associated label draws generated content. + const generated = [label, ...Array.from(label.querySelectorAll('*'))] + .flatMap((el) => Array.from(el.classList)) + .filter((c) => /(^|:)content-[[(]/.test(c)); + expect(generated).toEqual([]); + + // 2. The visible `*` is one real element, hidden from assistive tech. + const markers = label.querySelectorAll('[data-required-marker]'); + expect(markers).toHaveLength(1); + expect(markers[0]).toHaveAttribute('aria-hidden', 'true'); + expect(markers[0].textContent).toBe('*'); + + // 3. The description association is untouched by the marker: still the + // one paragraph, and the marker lives in the label, not in it. + const described = describedElements(input); + expect(described).toHaveLength(1); + expect(described[0].textContent).toBe(HELP); + expect(described[0].contains(markers[0])).toBe(false); + + // The required STATE is the native attribute, as before. + expect(input).toBeRequired(); + expect(input).toHaveAttribute('required'); + }); + it('survives the real render path through SchemaRenderer', () => { render( ( return (
{label && ( - )} @@ -71,9 +92,11 @@ const FieldContainer = React.forwardRef( aria-invalid={!!error} // Required is a STATE, so it rides the same Slot injection as the // other a11y wiring (objectui#3299) — one line here covers every - // consumer of FieldContainer. Until now `required` drove only the - // label's CSS asterisk (`after:content-['*']`), which never enters - // the a11y tree as a state. `|| undefined` so an optional field + // consumer of FieldContainer. Before objectui#3299 `required` drove + // only the label's visual asterisk, which is paint, not a STATE: an + // asterisk can reach the accessible NAME (it did, as CSS generated + // content, until objectui#10368) but never the required state. + // `|| undefined` so an optional field // carries no attribute. Deliberately NOT the native `required` // attribute (#3290 ruling: it arms browser constraint validation // alongside the host's own `error` slot). diff --git a/packages/components/src/renderers/basic/text-input.tsx b/packages/components/src/renderers/basic/text-input.tsx index 3850f48ea4..1898b94005 100644 --- a/packages/components/src/renderers/basic/text-input.tsx +++ b/packages/components/src/renderers/basic/text-input.tsx @@ -124,11 +124,22 @@ function ElementTextInputRenderer({ schema }: { schema: any }) { data-input-id={schema?.id} > {label && ( -
); diff --git a/packages/components/src/renderers/form/input.tsx b/packages/components/src/renderers/form/input.tsx index 7093d0d7be..4e68bb4b3d 100644 --- a/packages/components/src/renderers/form/input.tsx +++ b/packages/components/src/renderers/form/input.tsx @@ -44,7 +44,21 @@ const InputRenderer = ({ schema, className, onChange, value, disabled: hostDisab data-obj-type={dataObjType} style={style} > - {schema.label && } + {schema.label && ( + + )} - {schema.label && } + {schema.label && ( + + )}