Skip to content

BUGFIX: template-no-redundant-role — case-insensitive match + <select>→combobox#2727

Merged
NullVoxPopuli merged 11 commits intoember-cli:masterfrom
johanrd:fix/no-redundant-role-case-and-select
Apr 25, 2026
Merged

BUGFIX: template-no-redundant-role — case-insensitive match + <select>→combobox#2727
NullVoxPopuli merged 11 commits intoember-cli:masterfrom
johanrd:fix/no-redundant-role-case-and-select

Conversation

@johanrd
Copy link
Copy Markdown
Contributor

@johanrd johanrd commented Apr 21, 2026

Note

This is part of a series where Claude has audited eslint-plugin-ember against jsx-a11y, vuejs-accessibility, angular-eslint, lit-a11y and html-validate, ember-template-lint, and the HTML and WCAG specs.

Two fixes in one PR (shared rule, no semantic overlap).

1. Case-insensitive role comparison

  • Premise: Authors are guided to lowercase ARIA role tokens, and modern browsers parse them ASCII case-insensitively. Per html-aria §4.4 — Case requirements for ARIA role, state and property attributes: authors "SHOULD use ASCII lowercase for all role token values…"; the section's note observes that "modern browsers treat the role or aria- attribute values as ASCII case-insensitive"*. The HTML enumerated-attribute machinery the role attribute relies on is also case-insensitive (HTML Living Standard — enumerated attributes).
  • Problem: The rule compared the raw attribute value against a lowercase-keyed table, so <body role="DOCUMENT"> slipped through.

Fix: lowercase the role value before the ROLE_TO_ELEMENTS / LANDMARK_ROLES lookups.

2. <select> maps to combobox (conditional)

Fix: add combobox: ['select'], and gate the combobox redundancy check on the multiple / size attributes so <select role="combobox" multiple> and <select role="combobox" size="5"> are not falsely flagged. The implicit-role check mirrors jsx-a11y's src/util/implicitRoles/select.js.

Tests cover both fixes, including the uppercase + conditional-combobox interaction.

Prior art

Verified each peer in source:

Plugin Rule Verified behavior
jsx-a11y no-redundant-roles Calls getExplicitRole (lowercases via role.toLowerCase() at src/util/getExplicitRole.js:18–23); src/util/implicitRoles/select.js:9–17 honors multiple and size > 1.
vuejs-accessibility no-redundant-roles Exact-match comparison at line 78 (implicitRoleSet.includes(explicitRole)); no lowercasing in the rule or its utils. <body role="DOCUMENT"> is not flagged there either.
@angular-eslint/template no equivalent rule angular-eslint ships valid-aria, role-has-required-aria, and related ARIA-validity rules, but no redundant-role check. Role-vs-implicit-role redundancy is not audited.
lit-a11y no-redundant-role Same getExplicitRole-style pattern as jsx-a11y (case-insensitive via its own lib/utils/getExplicitRole.js:11–12).

Audit fixture

Translated peer-plugin test fixture at tests/audit/no-redundant-roles/peer-parity.js. Runs as part of the default Vitest suite (picked up by the tests/**/*.js include glob) — serves double-duty as a peer-parity audit and as regression coverage pinning CURRENT behavior.

johanrd added 3 commits April 21, 2026 07:41
…x mapping

Two changes.

1. Compare role tokens case-insensitively. ARIA role values are
   ASCII-case-insensitive (HTML-AAM inherits HTML's attribute-comparison
   semantics). Before: <body role="DOCUMENT"> was not flagged because
   "DOCUMENT" didn't match "document" in ROLE_TO_ELEMENTS.

2. Add 'combobox' → ['select'] mapping. <select> without `multiple` or
   `size > 1` has an implicit role of "combobox" per HTML-AAM §4.1.
   Before: <select role="combobox"> was silently accepted as a redundant
   role.

Two new invalid tests cover the fixes.
Per HTML-AAM, <select>'s implicit role is "combobox" only when neither
`multiple` nor `size > 1` is present; otherwise it is "listbox". The
previous commit added `combobox: ['select']` unconditionally, which
caused false positives for <select role="combobox" multiple> and
<select role="combobox" size="5"> (where combobox disagrees with the
implicit listbox role and therefore is not redundant).

Add a selectHasComboboxImplicitRole helper mirroring jsx-a11y's
src/util/implicitRoles/select.js, and short-circuit the redundancy
check for <select role="combobox"> when the implicit role is actually
listbox.

Update the code comment on the `combobox: ['select']` entry to reflect
this (and drop the inaccurate "§4.1" reference — the <select> mapping
lives in the HTML-AAM main conformance table, section 4, without a
numbered subsection).

Tests:
- valid: <select role="combobox" multiple></select>
- valid: <select role="combobox" size="5"></select>
- invalid: <select role="combobox" size="1"></select>  (size=1 → combobox)
- invalid: <select role="COMBOBOX"></select>           (case + implicit)
…ases

Translates 35 cases from peer-plugin rules:
  - jsx-a11y no-redundant-roles
  - vuejs-accessibility no-redundant-roles
  - lit-a11y no-redundant-role

Fixture documents parity after this fix:
  - Case-insensitive role comparison (body role="DOCUMENT" now flagged).
  - Default <select role="combobox"> flagged; <select multiple>/size>1 still
    valid (implicit listbox, not combobox).

Remaining divergences (ul/ol role="list", <a role="link"> without href)
are annotated inline. Not wired into the default vitest run.
johanrd added 2 commits April 22, 2026 14:03
- Normalize role via trim() + first-token split before lookup so values
  like 'COMBOBOX' or 'combobox listbox' are handled consistently
  (ARIA role attribute is a space-separated fallback list; only the
  first supported token is effective).
- getSelectImplicitRole() now returns 'combobox' | 'listbox' | 'unknown'.
  A dynamic <select size={{...}}> previously fell through as 'combobox'
  and produced false positives on role="combobox"; we now bail on
  'unknown' instead of flagging.
@johanrd johanrd force-pushed the fix/no-redundant-role-case-and-select branch from df79aaf to 00c823d Compare April 22, 2026 17:10
johanrd added 6 commits April 23, 2026 21:28
…d role per ARIA §4.1 (Copilot review)

Bundle with Q7/Q10/Q18/Q30 cross-rule pattern. Use aria-query's roles
set as the "recognised" oracle (not the local ROLE_TO_ELEMENTS table,
which would over-walk to roles it has mappings for).

`role="xxyxyz button"` on <button> → first recognised is button → flag
redundancy (autofix drops the whole role attr; implicit button role is
preserved natively). `role="tab button"` on <button> → first recognised
is tab → no implicit mapping for tab → nothing to flag.
…elect> implicit role (Copilot review)

Ember omits bound boolean attributes at runtime when the value is falsy, so
`<select multiple={{this.isMulti}}>` can resolve to either the combobox or
listbox implicit role. Previously treated any presence as listbox, which
could misclassify redundancy. Return 'unknown' for the dynamic case so the
caller skips flagging — matching the existing 'unknown' handling for dynamic
`size`.
…ox (Copilot review)

Per HTML boolean-attr semantics, a missing-value attribute's value is the
empty string — Number('') is 0; 0 is not > 1, so the implicit role of a
<select size> (no value) stays combobox. Previously we bailed out to
'unknown' for any sizeAttr.value that wasn't a GlimmerTextNode, conflating
valueless with dynamic. Split the two: valueless → combobox (static),
dynamic (mustache / concat) → unknown (skip).

Add an invalid test: <select role='combobox' size> flags as redundant
combobox.
…dit fixture

Upstream maintainers don't want the per-PR `tests/audit/peer-parity`
pattern. Port two basic non-landmark redundancy cases that were only
in the audit fixture into the regular suite:
- `<button role="button">` (autofix drops role)
- `<img role="img">` (autofix drops role)

Other audit cases were already covered by the regular tests.
@johanrd johanrd closed this Apr 25, 2026
@johanrd johanrd reopened this Apr 25, 2026
@johanrd johanrd closed this Apr 25, 2026
@johanrd johanrd reopened this Apr 25, 2026
@johanrd johanrd marked this pull request as ready for review April 25, 2026 15:29
@NullVoxPopuli NullVoxPopuli merged commit ca16804 into ember-cli:master Apr 25, 2026
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants