Add template-no-aria-hidden-focusable: flag aria-hidden on focusable elements and ancestors#41
Closed
johanrd wants to merge 7 commits into
Closed
Add template-no-aria-hidden-focusable: flag aria-hidden on focusable elements and ancestors#41johanrd wants to merge 7 commits into
template-no-aria-hidden-focusable: flag aria-hidden on focusable elements and ancestors#41johanrd wants to merge 7 commits into
Conversation
🏎️ Benchmark Comparison
Full mitata output |
There was a problem hiding this comment.
Pull request overview
Adds a new Ember template accessibility rule to detect aria-hidden="true" applied to focusable elements and to ancestors of focusable elements (within the same template), aligning with “aria-hidden on focusable” guidance and similar ecosystem rule names.
Changes:
- Added
template-no-aria-hidden-focusablerule implementation with focusability and descendant-walk logic. - Added comprehensive rule documentation and a README rules-list entry.
- Added RuleTester coverage for both
.hbsand<template>(GJS) parsing modes.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
lib/rules/template-no-aria-hidden-focusable.js |
Implements the new rule (aria-hidden truthiness, focusability detection, descendant scanning, reporting). |
tests/lib/rules/template-no-aria-hidden-focusable.js |
Adds valid/invalid cases for .hbs and <template> wrapped inputs. |
docs/rules/template-no-aria-hidden-focusable.md |
Documents rule intent, scope caveats, examples, and references. |
README.md |
Adds the rule to the auto-generated rules list table. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Adds a new Ember template accessibility rule to catch cases where aria-hidden hides focusable content within the same template, aligning behavior with ACT/axe guidance while remaining template-scoped.
Changes:
- Introduces new rule
template-no-aria-hidden-focusableto flagaria-hiddenon focusable elements and on ancestors with focusable descendants (within-template walk). - Adds rule docs + README rule index entry.
- Adds HBS + GJS RuleTester coverage for valid/invalid scenarios.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/lib/rules/template-no-aria-hidden-focusable.js | Adds comprehensive test matrix for the new rule in both HBS and GJS modes. |
| lib/utils/is-native-element.js | Introduces a native-element detection helper with scope-shadowing awareness. |
| lib/rules/template-no-aria-hidden-focusable.js | Implements the new rule, including aria-hidden truthiness + focusability classification + descendant walk. |
| docs/rules/template-no-aria-hidden-focusable.md | Documents rule intent, scope caveat, examples, and references. |
| README.md | Adds the rule to the public rule list. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
johanrd
force-pushed
the
html-validate/template-no-aria-hidden-focusable
branch
from
02b03f9 to
52f2aa1
Compare
johanrd
added a commit
that referenced
this pull request
Apr 23, 2026
There was a problem hiding this comment.
Pull request overview
This PR adds a new opt-in Ember template accessibility rule, template-no-aria-hidden-focusable, to detect cases where aria-hidden is applied to focusable content (either directly or via an aria-hidden ancestor within the same template scope).
Changes:
- Added
template-no-aria-hidden-focusablerule that reportsaria-hiddenon focusable elements andaria-hiddenancestors containing focusable descendants (template-scoped walk). - Added comprehensive RuleTester coverage for both GJS (
<template>…</template>) and standalone HBS parsing. - Added end-user documentation and listed the rule in the README rules table.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
lib/rules/template-no-aria-hidden-focusable.js |
Implements focusability detection, aria-hidden truthiness parsing, and descendant walking/reporting. |
tests/lib/rules/template-no-aria-hidden-focusable.js |
Adds valid/invalid cases covering self + ancestor/descendant reporting for both HBS and GJS. |
lib/utils/is-native-element.js |
Provides a shared helper to distinguish native elements from component invocations (including scope-shadowing). |
docs/rules/template-no-aria-hidden-focusable.md |
Documents rule behavior, scope caveats, examples, fixes, and references. |
README.md |
Adds the rule to the Accessibility rules list. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Adds a new opt-in accessibility rule to catch aria-hidden applied to focusable elements (including when the focusable element is a descendant of an aria-hidden subtree) in Ember templates.
Changes:
- Introduce
template-no-aria-hidden-focusablerule with descendant walking limited to same-template native elements. - Add comprehensive test coverage for both
.gjs<template>and.hbsparsing modes. - Document the rule and list it in the README rules table.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
lib/rules/template-no-aria-hidden-focusable.js |
Implements the rule logic: aria-hidden truthiness parsing, focusability detection, and descendant traversal with deduped reports. |
tests/lib/rules/template-no-aria-hidden-focusable.js |
Adds valid/invalid cases for self + ancestor/descendant scenarios across HBS and GJS parsing. |
lib/utils/is-native-element.js |
Adds/updates a utility for native element detection with scope-shadowing awareness. |
docs/rules/template-no-aria-hidden-focusable.md |
Documents intent, scope caveats, examples, and references. |
README.md |
Adds the rule to the Accessibility rules list. |
💡 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
… via shared helper (Copilot review)
Extract a new `getStaticAttrValue` util that resolves literal-valued
mustaches (`{{"foo"}}`, `{{true}}`, `{{-1}}`) and single-part concat
statements (`"{{true}}"`) to their static string value. `isAriaHiddenTruthy`
now delegates to the helper and compares the resolved string to `'true'`
(case-insensitive, whitespace-trimmed).
Behavior change: valueless `<h1 aria-hidden>`, `aria-hidden=""`, and the
mustache-empty-string equivalents (`aria-hidden={{""}}`, `aria-hidden="{{""}}"`,
`aria-hidden={{" "}}`) are no longer treated as hidden. Per WAI-ARIA 1.2
§6.6 value table, those shapes resolve to the default `undefined` — NOT
`true` — so the empty-content check still applies. Drops the previous
"fewer false positives" deviation rationale in favour of spec-literal
consistency with sibling rules (#19, #35, #41) that share the same
aria-hidden resolution.
Byte-identical carrier of lib/utils/static-attr-value.js across all PRs
that land it.
johanrd
force-pushed
the
html-validate/template-no-aria-hidden-focusable
branch
from
4c99210 to
8c3af11
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Ports html-validate's hidden-focusable. Reports aria-hidden=true on focusable elements and on elements that contain a focusable descendant (walked within the current template only — component/yield boundaries are out of scope). Focusability is approximated without MDN-scale metadata: buttons, select, textarea, iframe, details/summary, links with href, non-hidden inputs, media with controls, contenteditable, and explicit tabindex>=0. Logic adapted from html-validate (MIT).
WAI-ARIA 1.2's aria-hidden section uses MAY-level language and does not contain a MUST-NOT against hiding focusable content. The authoritative normative statement is W3C "Using ARIA" 4th Rule: "Do not use role='presentation' or aria-hidden='true' on a focusable element." Expanded References to: Using ARIA 4th Rule, ACT rule 6cfa84, axe-core aria-hidden-focus, WCAG F70/F86 failure techniques, and WAI-ARIA 1.2 as the attribute definition (not the prohibition).
…t and fix doc config block
…nents (Copilot review)
… handling; add GJS shadow test
- isImplicitlyFocusable now calls isNativeElement(node, sourceCode) instead of
the heuristic isHtmlElementNode, so GJS/GTS scope-shadowed lowercase tags are
not mistaken for native elements; sourceCode is threaded through isFocusable
- isContentEditable recognises GlimmerMustacheStatement with BooleanLiteral and
StringLiteral paths, so contenteditable={{true}} / contenteditable={{"true"}}
are correctly identified as focusable
- isAriaHiddenTruthy no longer treats empty-string as truthy; per WAI-ARIA 1.2
only the explicit string "true" enables aria-hidden, so bare aria-hidden and
aria-hidden="" are no longer flagged; tests updated accordingly
- Adds a GJS-only valid test case where a lowercase tag is bound to a component
in scope, confirming it is not flagged as a native focusable element
johanrd
force-pushed
the
html-validate/template-no-aria-hidden-focusable
branch
from
42f149e to
a75d262
Compare
Owner
Author
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.Summary
Add
template-no-aria-hidden-focusable: flagaria-hidden="true"on a focusable element, and on an element whose focusable descendant is in the same template. Matches the existing jsx-a11y / vuejs-accessibility rule name (peers check self only; we also walk descendants within the template).6cfa84("Element with aria-hidden has no content in sequential focus navigation") operationalizes this: a focusable descendant of anaria-hidden="true"subtree is a conformance failure because the element is reachable via Tab but removed from the accessibility tree.aria-hidden-focusrule. jsx-a11y and vue-a11y also implement it (self-only), though their implementations miss the ancestor-descendant case that "Using ARIA" and the ACT rule explicitly cover.aria-hidden="true"on any element that is focusable, and on any element that contains a focusable descendant within the same template. Skip mustache-unknownaria-hidden/tabindexvalues. Walking descendants is the spec-grounded behavior (peers' self-only check is incomplete); the scope caveat is template-file-only visibility, not an opinion about ARIA.Fix
lib/rules/template-no-aria-hidden-focusable.js; tests intests/lib/rules/template-no-aria-hidden-focusable.js(56 cases).isAriaHiddenTruthy(template-no-aria-hidden-focusable.js:19-42) — classifies the attribute; valueless and baretrue/"true"count as hidden;{{false}}/"false"/ mustache-unknown are not.isFocusable(template-no-aria-hidden-focusable.js:80-88) — hand-coded focusability: inherently focusable tags (button, select, textarea, iframe, summary, details),<a href>,<input>(not hidden), media withcontrols,contenteditable, and explicittabindex >= 0. Mustachetabindex={{...}}is neither focusable nor non-focusable — skipped.walkElementDescendants(template-no-aria-hidden-focusable.js:141-151) — recurses into same-templateGlimmerElementNodechildren only. Does not cross component invocation or{{yield}}boundaries.Prior art
no-aria-hidden-on-focusablearia-hidden="true"only on the element itself; reusesisFocusableutil. Does not walk descendants.no-aria-hidden-on-focusablehidden-focusable— specFlags
Allows
Notes
{{yield}}boundaries. A focusable element rendered by a child component underaria-hiddenwon't be flagged. The alternative (flag every component underaria-hidden) produced unacceptable noise in early testing. This trade matches the plugin's general "fewer false positives" stance.