BUGFIX: template-no-unsupported-role-attributes — honor aria-query attribute constraints#52
Closed
BUGFIX: template-no-unsupported-role-attributes — honor aria-query attribute constraints#52
Conversation
…ute constraints in implicit-role lookup
The old getImplicitRole picked the first elementRoles entry whose name
matched the tag. For <input> it tried a type-attribute match but
ignored the rest of aria-query's constraint annotations.
Concrete consequences of the old logic:
- <input type="text"> without a `list` attribute returned "combobox"
(because aria-query's {type=text, list=set}→combobox entry appears
before {type=text, list=undefined}→textbox). Correct implicit role is
textbox.
- <input type="email|tel|url"> behaved the same way.
- <input type="password"> isn't mapped by aria-query; the fallback
branch returned the first input entry — "button".
The new implementation walks every elementRoles key whose tag matches,
checks each attribute spec (honoring `constraints: ["set"]` and
`constraints: ["undefined"]` as well as required values), and picks the
entry with the most attribute constraints satisfied.
Behavioral impact:
- Text-like inputs without a list attribute now correctly resolve to
textbox and accept aria-required / aria-readonly / aria-placeholder.
- <input type="password"> now resolves to no implicit role, so global
ARIA attrs don't trip the rule (matching jsx-a11y's treatment).
- One existing test updated: <input type="email"> now maps to textbox
in the error message; added a sibling case with `list="x"` to cover
the combobox path.
- Eight new valid tests cover the textbox/password paths.
…peer cases Translates 33 cases from peer-plugin rules: - jsx-a11y role-supports-aria-props - lit-a11y role-supports-aria-attr - vuejs-accessibility (no direct equivalent; divergence noted) The fixture documents where our rule now matches jsx-a11y (notably <input type="email"> without `list` mapping to "textbox" per the aria-query attribute constraints) plus the sibling `list=…` case mapping to "combobox".
…euristic The "pick entry with most attribute constraints satisfied" resolution is a heuristic, not a spec-documented approach. aria-query's elementRoles is an unordered Map; aria-query does not publish a resolution order for multi-match cases. Name the inference, cite the motivating bug cases, and note that if aria-query ever publishes resolution semantics we should switch to those.
🏎️ Benchmark Comparison
Full mitata output |
There was a problem hiding this comment.
Pull request overview
Fixes template-no-unsupported-role-attributes implicit-role resolution to honor aria-query’s per-element attribute constraints, preventing incorrect implicit roles (notably for various <input> types) and the resulting false positives/negatives in aria-* supported-props validation.
Changes:
- Reworked implicit-role lookup to evaluate
aria-queryelementRolesentries against the element’s static attribute state and choose the most specific match. - Added/updated rule tests covering
<input type="text|email|tel|url">vs combobox paths and ensuring<input type="password">yields “no implicit role”. - Added an “audit” RuleTester fixture for peer-plugin parity comparisons.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| lib/rules/template-no-unsupported-role-attributes.js | Implements attribute-constraint-aware implicit role selection. |
| tests/lib/rules/template-no-unsupported-role-attributes.js | Adds/updates test cases for textbox/password and email+list behaviors. |
| tests/audit/role-supports-aria-props/peer-parity.js | Adds parity/audit fixture documenting behavior vs jsx-a11y/lit-a11y. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- getStaticAttrValue() now trims chars so callers don't handle whitespace. - Lowercase the node value when comparing against aria-query's attrSpec.value (HTML enumerated attribute values are ASCII case-insensitive per HTML common-microsyntaxes §2.3.3). - Remove unused createUnsupportedAttributeErrorMessage() dead code; the rule uses messageId/data exclusively.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
johanrd
force-pushed
the
fix/implicit-role-attribute-constraints
branch
from
5ad45c5 to
3de05b9
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
… by tag (Copilot review) Benchmarked ~2.6× speedup on getImplicitRole — bucketing the static elementRoles Map by tag turns the per-call scan of ~80 keys into a 1-5 key lookup per tag. Parity verified across 15 representative tag/attr combinations before landing. Matches the Q29 optimization pattern on #51's isSemanticRoleElement.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
johanrd
added a commit
that referenced
this pull request
Apr 24, 2026
…ementRoles, valueless-label, fixable meta (Copilot review) - scoreMatch now uses three tiers: exact value (3) > set (2) > undefined (1). Previously set and undefined both added +1, so a stricter 'set' constraint could lose to a looser 'undefined' when both matched. Exact-value entries still win over 'set' (e.g. <img alt=''> → presentation still beats the generic alt-set entry). - Pre-index elementRoles by tag name at module load (same pattern as PR #52's template-no-unsupported-role-attributes). Turns a ~80-key scan per call into a 1–5-key lookup. - hasNonEmptyLabelAttr returns false when attr.value == null — a valueless aria-label / aria-labelledby carries no accessible name, so it shouldn't be treated as a non-empty (author-declared) label. - Add meta.fixable: null — rule is not autofixable, be explicit.
…ases, drop audit fixture
Upstream maintainers don't want the per-PR `tests/audit/peer-parity`
pattern. Port two unique audit cases into the regular suite:
- `<body aria-expanded=...>` — pins our implicit-role resolution
("generic" via aria-query, vs jsx-a11y's "document"); both flag,
message differs.
- `<a aria-checked />` — acknowledged false-positive vs jsx-a11y; we
resolve href-less <a> to implicit role "generic" where aria-checked
is unsupported, jsx-a11y treats it as having no role and accepts.
All other audit cases were already covered by the regular tests.
…n <a> divergence note Researched the href-less <a> divergence: per HTML-AAM 1.2 §3.5.3 the implicit role IS `generic` (which is what we resolve via aria-query). jsx-a11y's `getImplicitRoleForAnchor` returns `''` for href-less <a>, and its role-supports-aria-props rule then early-returns on no-role and silently allows any aria-* — jsx-a11y's own source comments this as "This actually isn't true - should fix in future release". vue-a11y walks aria-query the same way we do and reaches the same conclusion. We're spec-current; jsx-a11y is spec-stale. Updated the test comment to reflect this rather than calling our behavior an "acknowledged false-positive".
johanrd
added a commit
that referenced
this pull request
Apr 25, 2026
johanrd
added a commit
that referenced
this pull request
Apr 26, 2026
…ementRoles, valueless-label, fixable meta (Copilot review) - scoreMatch now uses three tiers: exact value (3) > set (2) > undefined (1). Previously set and undefined both added +1, so a stricter 'set' constraint could lose to a looser 'undefined' when both matched. Exact-value entries still win over 'set' (e.g. <img alt=''> → presentation still beats the generic alt-set entry). - Pre-index elementRoles by tag name at module load (same pattern as PR #52's template-no-unsupported-role-attributes). Turns a ~80-key scan per call into a 1–5-key lookup. - hasNonEmptyLabelAttr returns false when attr.value == null — a valueless aria-label / aria-labelledby carries no accessible name, so it shouldn't be treated as a non-empty (author-declared) label. - Add meta.fixable: null — rule is not autofixable, be explicit.
johanrd
added a commit
that referenced
this pull request
Apr 27, 2026
…ementRoles, valueless-label, fixable meta (Copilot review) - scoreMatch now uses three tiers: exact value (3) > set (2) > undefined (1). Previously set and undefined both added +1, so a stricter 'set' constraint could lose to a looser 'undefined' when both matched. Exact-value entries still win over 'set' (e.g. <img alt=''> → presentation still beats the generic alt-set entry). - Pre-index elementRoles by tag name at module load (same pattern as PR #52's template-no-unsupported-role-attributes). Turns a ~80-key scan per call into a 1–5-key lookup. - hasNonEmptyLabelAttr returns false when attr.value == null — a valueless aria-label / aria-labelledby carries no accessible name, so it shouldn't be treated as a non-empty (author-declared) label. - Add meta.fixable: null — rule is not autofixable, be explicit.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note
This is part of a series where Claude has audited
eslint-plugin-emberagainst jsx-a11y, vuejs-accessibility, angular-eslint, lit-a11y and html-validate,ember-template-lint, and the HTML and WCAG specs.[Mirror of ember-cli#2726 for Copilot review]
aria-query'selementRolesmap encodes element → implicit-role pairings that depend on attribute state. Each entry lists attributes that must beset,undefined, or match a specificvalue.getImplicitRolehelper returned the first entry whose tag name matched, ignoring the attribute constraints on subsequent entries.Concrete consequences of the old logic:
<input type="text">(nolist)(nolist`)<input type="password">The first two block valid usages involving attributes that
textboxsupports butcomboboxdoes not — notablyaria-placeholder(covered by the new tests) andaria-multiline. Bothtextboxandcomboboxlistaria-requiredin aria-query 5.3.2, so that attribute is not the concern here. The third row is arbitrarily wrong — there's no reason a password input should ever be treated as a button.<input type="password">being left unmapped is consistent with HTML-AAM, which assigns it "No corresponding role."Fix: walk every
elementRolesentry whose name matches the tag; evaluate each attribute spec (checkingconstraints: ["set"],constraints: ["undefined"], and required values); pick the entry with the most attribute constraints satisfied.Eight new valid tests cover the textbox/password paths. One existing test updated:
<input type="email" aria-level={{this.level}} />now maps to textbox in the error message; a sibling case withlist="x"covers the combobox path.Prior art
role-supports-aria-propssrc/util/implicitRoles/(e.g.a.jschecks forhrefbefore returning "link"); then validates that everyaria-*attribute is in the role's supported-props list fromaria-query.aria-propsrule only validates that attribute names beginning witharia-are known ARIA attributes (viaaria.has(lowered)); it does not compute implicit roles or check role↔prop compatibility.valid-ariavalidates ARIA attribute names and literal values againstaria-query's definitions; it does not resolve implicit roles from element + attributes, so the role-supports-props check does not apply.role-has-required-ariais the inverse check (role → required props), not role → supported props.role-supports-aria-attrrole=attribute (Object.keys(element.attribs).includes('role')); does not compute an implicit role from tag + attributes, so the implicit-role false positive does not arise.Audit fixture
Translated peer-plugin test fixture at
tests/audit/role-supports-aria-props/peer-parity.jsdocumenting our rule's behavior relative to jsx-a11y and lit-a11y. Runs as part of the default Vitest suite (included via thetests/**/*.jsglob) and serves double-duty as an auditable peer-parity record and as regression coverage pinning current behavior.