BUGFIX: template-no-unsupported-role-attributes — honor aria-query attribute constraints#2726
Draft
johanrd wants to merge 7 commits intoember-cli:masterfrom
Draft
BUGFIX: template-no-unsupported-role-attributes — honor aria-query attribute constraints#2726johanrd wants to merge 7 commits intoember-cli:masterfrom
johanrd wants to merge 7 commits intoember-cli:masterfrom
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.
- 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.
5ad45c5 to
3de05b9
Compare
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.
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. Not wired into the default test run.