Skip to content

BUGFIX: template-require-mandatory-role-attributes — lowercase role + split whitespace role lists#2728

Closed
johanrd wants to merge 18 commits intoember-cli:masterfrom
johanrd:fix/role-required-aria-case-and-space-split
Closed

BUGFIX: template-require-mandatory-role-attributes — lowercase role + split whitespace role lists#2728
johanrd wants to merge 18 commits intoember-cli:masterfrom
johanrd:fix/role-required-aria-case-and-space-split

Conversation

@johanrd
Copy link
Copy Markdown
Contributor

@johanrd johanrd commented Apr 21, 2026

Depends on #2725 — merge #2725 first. Without it, the <input type="checkbox" role="switch"> exemption from #2725's axobject-query-driven isSemanticRoleElement helper is missing, and this PR's existing test fixtures (plus the audit-parity file) would flag those shapes as missing aria-checked. #2725 + #2728 together produce the peer-aligned behavior; alone, #2728 shifts some tests out of sync until #2725 lands.

Two related fixes (shared root cause — both deal with role-token normalisation).

1. Case-insensitive role comparison

  • Premise: ARIA role token comparison inherits case-insensitivity from HTML's enumerated-attribute rules — role="COMBOBOX" is the same token as role="combobox". (WAI-ARIA 1.2 §4.1 explicitly defers case-sensitivity to the host language: "Case-sensitivity of the comparison inherits from the case-sensitivity of the host language.")
  • Problem: <div role="COMBOBOX"> was silently accepted — roles.get("COMBOBOX") returns undefined in aria-query, so the rule skipped the check entirely.

Fix: lowercase the role value before the lookup.

2. Whitespace-separated role fallback lists

  • Premise: Per WAI-ARIA 1.2 §4.1, a role attribute may list multiple tokens and "User agents MUST use the first token in the sequence of tokens in the role attribute value that matches the name of any non-abstract WAI-ARIA role."
  • Problem: role="combobox listbox" was treated as one opaque string. aria-query returned undefined, the rule skipped silently, and <div role="combobox listbox"> without aria-expanded + aria-controls wasn't flagged.

Fix: split on whitespace, validate the first recognised role per ARIA §4.1's first-token semantics. This diverges from jsx-a11y and vue-a11y, which validate every recognised token — our approach is closer to the UA behavior the spec prescribes. Noted as a deliberate divergence.

Helpers renamed to plural forms (getStaticRolesFromElement, getStaticRolesFromMustache). getMissingRequiredAttributes now returns { role, missing } so the reporter can cite the actually-checked role in the error message.

Four new tests cover both fixes (valid + invalid of each).

Prior art

Verified each peer in source:

Plugin Rule Verified behavior
jsx-a11y role-has-required-aria-props String(roleAttrValue).toLowerCase().split(' ').filter(recognised).forEach(validate). Validates EVERY recognised token.
vuejs-accessibility role-has-required-aria-props roleValue.toLowerCase().split(" ").forEach(role => { if (isAriaRoleDefinitionKey(role)) validate(role) }). Same pattern.
@angular-eslint/template role-has-required-aria Raw role string passed directly to roles.get(role). No .toLowerCase(), no .split().
lit-a11y role-has-required-aria-attrs Raw role string passed to isAriaRole / roles.get. No splitting, no lowercasing.

johanrd added a commit to johanrd/eslint-plugin-ember that referenced this pull request Apr 21, 2026
…d exemption to {{input}} helper

Two changes:

1. `getInputType` now recognises the classic Ember `{{input type=...}}`
   helper as an input host, in addition to the native `<input>` element.
   The `GlimmerMustacheStatement` handler is updated to pass `node` into
   `getMissingRequiredAttributes`, matching the element-node handler.

2. Reverts the defensive `role.toLowerCase()` in the whitelist-key build
   from the previous commit — it was dead code. The caller
   (`getMissingRequiredAttributes`) short-circuits via aria-query's
   case-sensitive `roles.get(role)`, so `role` is guaranteed lowercase
   by the time `isNativelyChecked` runs. Rule-wide role case-
   insensitivity is not in scope for this PR; that is ember-cli#2728.

Tests adjusted accordingly: drop mixed-case-role valid tests (they
passed for the wrong reason — via the case-sensitivity short-circuit in
aria-query, not via the whitelist). Add coverage for `{{input}}`
helper on both gts and hbs parsers, including off-whitelist pairings
that must still flag.
johanrd added 2 commits April 22, 2026 00:31
…or semantic-role exemptions

Replaces the 3-entry hand-list (`{input type}:{role}` pairings) with a
lookup against axobject-query's `elementAXObjects` + `AXObjectRoles`
maps. Mirrors the approach used by eslint-plugin-jsx-a11y (its
`isSemanticRoleElement` util) and @angular-eslint/template.

## Why

The hand-list covered 3 pairings: `checkbox:checkbox`, `checkbox:switch`,
`radio:radio`. axobject-query encodes substantially more — including
`input[type=range]:slider`, `input[type=number]:spinbutton`,
`input[type=text]:textbox`, `input[type=search]:searchbox`. Each of
these is a case where the native element already provides the role's
required ARIA state (e.g., `<input type=range>` provides value via its
native `value` attribute, satisfying `role=slider`'s `aria-valuenow`
requirement).

Using axobject-query directly:
- Gives us strict superset coverage of the hand-list.
- Stays in sync when axobject-query updates (the hand-list had already
  drifted — the earlier revision incorrectly claimed menuitemcheckbox
  / menuitemradio pairings were in axobject-query when they aren't).
- Matches jsx-a11y / angular-eslint behavior, closing a documented
  parity gap.

Adds `axobject-query@^4.1.0` as a direct dep. It's already a transitive
dep via other ecosystem packages; this elevates it to first-class.

## Changes

- `lib/rules/template-require-mandatory-role-attributes.js` — replace
  `NATIVELY_CHECKED_INPUT_ROLE_PAIRS` + `isNativelyChecked` with
  `isSemanticRoleElement` that walks `elementAXObjects` and checks
  `AXObjectRoles`. Handles both `GlimmerElementNode` (angle-bracket
  syntax) and `GlimmerMustacheStatement` (classic `{{input}}` helper).
- `package.json` — add `axobject-query@^4.1.0`.
- `tests/lib/rules/template-require-mandatory-role-attributes.js` —
  add tests for the broadened coverage (`<input type=range role=slider>`
  now valid in both gts and hbs forms).
- `docs/rules/template-require-mandatory-role-attributes.md` — rewrite
  the exemption section to describe the axobject-query-backed lookup
  with a table of known pairings.
…imary source

axobject-query is the data package the rule queries, but the normative
source for "native <input type=checkbox> exposes aria-checked via the
checked IDL attribute" is HTML-AAM §el-input-checkbox. Adding the HTML-AAM
link makes the exemption's spec grounding explicit instead of leaving
axobject-query as a free-floating data source.
@johanrd johanrd force-pushed the fix/role-required-aria-case-and-space-split branch from 88ab040 to 3d69b55 Compare April 22, 2026 17:10
johanrd added 7 commits April 23, 2026 21:39
…e {{input}} assumption (Copilot review)

In strict GJS/GTS, {{input}} could resolve to any imported identifier — not
just Ember's classic helper. We assume the classic helper (renders native
input); false-positive risk is low in practice because strict-mode authors
rarely use the mustache form at all. Document the assumption inline so
future readers don't need to re-derive the tradeoff.
…Objects by tag (Copilot review)

Benchmarked ~12.5x speedup on isSemanticRoleElement. The naive impl walked
the full elementAXObjects map per call (O(concepts × axObjects × roles));
pre-indexing resolves each concept's exposed-role set once at module load
and buckets concepts by tag, reducing the per-call hot path to a handful
of entries per tag.

Benchmark: 200k calls on a realistic tag/role mix — current 154 ms, indexed
12 ms. Behavior-preserving (140/140 parity combos verified before landing;
84/84 rule-test suite passes).
…t whitespace-separated role lists

Two changes, shared root cause (both deal with role-token normalisation).

1. Case-insensitive role matching. Per HTML-AAM, ARIA role tokens
   compare as ASCII-case-insensitive. Before: <div role="COMBOBOX">
   was silently accepted because "COMBOBOX" didn't match "combobox" in
   aria-query. Now: lowercase before lookup.

2. Whitespace-separated role fallback lists. Per ARIA 1.2 §5.4, a role
   attribute may list multiple tokens to express a fallback; a UA picks
   the first one it recognises. Before: role="combobox listbox" was
   treated as one opaque string, aria-query returned undefined, and
   the rule skipped silently. Now: split on whitespace, check against
   the first recognised role's required attributes (matching jsx-a11y).

Helpers renamed to plural forms (getStaticRolesFromElement,
getStaticRolesFromMustache) and getMissingRequiredAttributes now
returns { role, missing } so the reporter can use the actually-checked
role in the error message.

Four new tests cover the two cases (valid + invalid of each).
…havior

Rebased onto #51's axobject-query semantic-role-element exemption. The
audit fixture previously captured pre-#54 divergences (our rule accepted
space-separated and case-insensitive role values) as VALID-for-us. After
rebase, the rule splits whitespace + lowercases before lookup, so
`<div role="combobox listbox">` and `<div role="COMBOBOX">` now flag —
matching jsx-a11y. Move those cases from valid → invalid and rewrite the
divergence comments as parity.
@johanrd johanrd force-pushed the fix/role-required-aria-case-and-space-split branch from dc20a4b to d3d5abc Compare April 24, 2026 08:27
johanrd added 7 commits April 24, 2026 10:57
…ired-ARIA description (Copilot review)

Per WAI-ARIA, role=spinbutton requires aria-valuenow. The docs table
previously said 'no required ARIA'. Update to reflect that <input
type='number'> supplies the required value state via native value/min/max,
which accessibility mappings expose as aria-valuenow — the role IS required,
the native element provides it.
…tic-role exemption in strict mode (Copilot review)

In classic Handlebars (.hbs), `{{input}}` globally resolves to Ember's
built-in input helper, which renders a native `<input>` — so the
semantic-role exemption (e.g., `role="switch"` on `{{input
type="checkbox"}}` is satisfied by the native `checked` property) is
correct.

In strict-mode GJS/GTS there is no lowercase `input` export from
`@ember/component` (only the PascalCase `<Input>` component), so
`{{input}}` in strict mode is always a user-bound identifier. We can't
prove it renders a native `<input>`, so applying the exemption risks
silently skipping required-ARIA checks on arbitrary components. Detect
strict mode by filename (matching the convention already established by
template-builtin-component-arguments) and return null for `{{input}}`
there; the regular required-ARIA flow then reports missing attributes.
…e cases, drop audit fixture

Upstream maintainers don't want the per-PR `tests/audit/peer-parity`
pattern. Port two basic widget-role-missing-required-attrs cases that
peer plugins also flag (slider missing aria-valuenow, checkbox missing
aria-checked) into the regular suite.

Other audit cases were already covered by the regular tests on this
branch — including the previous DIVERGENCE cases (role="COMBOBOX",
role="combobox listbox") which this PR's lowercase + split-whitespace
fix turns into INVALID assertions in the regular suite.
@johanrd johanrd closed this Apr 25, 2026
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.

1 participant