Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions .changeset/10368-required-marker-real-span.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
---
'@object-ui/components': patch
---

fix(components): the required asterisk no longer enters a field's accessible name (objectui#10368)

`FieldContainer`, `element:text_input` and the `input`, `textarea`, `checkbox` and `select` renderers drew the required asterisk as CSS generated content (an `::after` utility) on the field's label. The accessible-name computation includes generated content, so where that label is associated with its control, Chromium named a required field labelled "Title" as "Title*". A pseudo-element cannot carry `aria-hidden`.

The asterisk is now a real `span` with `aria-hidden="true"` and `data-required-marker`, the shape the form renderer's `FormLabel` already uses, so it stays out of the name. It keeps the same colour and spacing. Every control keeps the required state it had before: the native `required` attribute, or `aria-required`. Validation does not change.

The `select` renderer's label is not associated with its trigger, so its name never contained the asterisk; its marker changes shape only. The generated-content utility these labels used is no longer compiled into `style.css`.

For test suites that render these fields: Testing Library's exact-string `getByLabelText('Title')` matches the label's whole text, and that text now includes the hidden `*`, so it no longer finds a required field. `getByRole('textbox', { name: 'Title' })` does, because it computes the accessible name.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(
<FieldContainer label="Title" required htmlFor="assoc-field">
<input />
</FieldContainer>,
);

// 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');
});
});
226 changes: 226 additions & 0 deletions packages/components/src/__tests__/required-marker-markup.test.tsx
Original file line number Diff line number Diff line change
@@ -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(
<FieldContainer label="Title" required={required} htmlFor="fc-ctl">
<input />
</FieldContainer>,
),
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 } }),
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 }),
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 }),
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 }),
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' }],
}),
// 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([]);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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`
*
Expand Down Expand Up @@ -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(
<SchemaRenderer
Expand Down
Loading
Loading