feat: add template-no-aria-hidden-on-focusable#19
Closed
Conversation
🏎️ Benchmark Comparison
Full mitata output |
This was referenced Apr 21, 2026
johanrd
added a commit
that referenced
this pull request
Apr 21, 2026
…ec + correct peer-plugin claims
Two corrections to the previous revision:
1. Valueless / empty-string `aria-hidden` is no longer treated as a
non-interactive escape hatch. Per WAI-ARIA 1.2 §6.6 + aria-hidden
value table, a missing or empty-string value resolves to the default
`undefined` — NOT `true`. Only an explicit `aria-hidden="true"`
(ASCII case-insensitive) or mustache-literal `{{true}}` opts out.
This matches ember-cli#2717 / #19's spec-first resolution.
2. Code comment corrections. jsx-a11y's util is named
`isPresentationRole`, not `hasPresentationRole`. The comment also
claimed jsx-a11y's `isPresentationRole` does "first token of a
space-separated role list" — it does not (jsx-a11y does plain
`presentationRoles.has(rawValue)`, no trim/lowercase/split). Our
first-token behavior is a deliberate superset, not parity.
Moved `<div aria-hidden onclick>` and `<div aria-hidden="" onclick>` from
the valid section to invalid. Added `<div aria-hidden="TRUE">` as
additional valid coverage for the case-insensitive path.
johanrd
added a commit
that referenced
this pull request
Apr 21, 2026
…ML boolean semantics
Per HTML Living Standard on boolean attributes, the presence of `autofocus`
indicates TRUE regardless of value — `autofocus="false"` and
`autofocus="autofocus"` are equally truthy. jsx-a11y's `no-autofocus`
treats the literal string `"false"` as an opt-out (via `getPropValue`),
but that's a peer-plugin convention that diverges from HTML semantics;
vue-a11y and lit-a11y are presence-based, consistent with the spec.
Narrow opt-out to the only case that is spec-consistent:
- `autofocus={{false}}` in angle-bracket syntax — renders no attribute.
- `{{input autofocus=false}}` in mustache hash-pair syntax — no attribute.
Revert peer-parity opt-outs for `autofocus="false"`, `autofocus={{"false"}}`,
and `{{input autofocus="false"}}` — these are now flagged per HTML spec
semantics. Moved from valid → invalid in the test suite.
Dialog exemption unchanged — keeps MDN-backed behavior for autofocus on
and within <dialog>.
Follows the spec-first direction established in ember-cli#2717 (aria-hidden),
#19, #33.
johanrd
added a commit
that referenced
this pull request
Apr 21, 2026
…I-ARIA spec Per WAI-ARIA 1.2 §6.6 + aria-hidden value table, a missing or empty-string aria-hidden resolves to the default `undefined` — NOT `true`. So <span aria-hidden>X</span> as a child of <a href="/x"> does NOT hide the span; its content still contributes to the anchor's accessible name. The prior behavior inherited jsx-a11y's JSX-coercion convention and vue-a11y's "anything-not-literal-false" shortcut. Both are peer-plugin conventions that diverge from normative ARIA. Matches the spec-first resolution of ember-cli#2717, #19, and #33. Moved valueless / empty aria-hidden cases from invalid → valid. Kept the explicit aria-hidden="true" and {{true}} cases as invalid.
There was a problem hiding this comment.
Pull request overview
Adds a new template accessibility rule to the plugin: ember/template-no-aria-hidden-on-focusable, intended to prevent the “aria-hidden + focusable = keyboard trap” anti-pattern in Ember templates (HBS and GJS/GTS).
Changes:
- Add
template-no-aria-hidden-on-focusablerule with direct + descendant-focusable detection. - Introduce shared utilities for component-invocation detection and HTML “interactive content” classification.
- Add rule docs, README rule-table entry, and a comprehensive set of tests (plus a peer-parity audit fixture).
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/lib/utils/is-component-invocation-test.js | Unit tests for new isComponentInvocation utility. |
| tests/lib/utils/html-interactive-content-test.js | Unit tests for new HTML interactive content utility. |
| tests/lib/rules/template-no-aria-hidden-on-focusable.js | Main rule test suite for both GJS/GTS and HBS modes. |
| tests/audit/no-aria-hidden-on-focusable/peer-parity.js | Peer-plugin parity audit fixture for translated upstream cases. |
| lib/utils/is-component-invocation.js | New helper to treat PascalCase / path-based tags as component invocations (opaque). |
| lib/utils/html-interactive-content.js | New helper classifying tags per HTML interactive content model (+ <summary>). |
| lib/rules/template-no-aria-hidden-on-focusable.js | New rule implementation: detects aria-hidden="true" on focusables and ancestors of focusables. |
| docs/rules/template-no-aria-hidden-on-focusable.md | Documentation for the new rule, including examples and caveats. |
| README.md | Adds the new rule to the published rule list. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
johanrd
force-pushed
the
feat/template-no-aria-hidden-on-focusable
branch
from
822a3ab to
88ba3ca
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 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
johanrd
added a commit
that referenced
this pull request
Apr 24, 2026
…view) Per HTML §6.6.3 'Sequential focus navigation', none of <details>, <option>, or <datalist> are focusable by default: - <details>: the focusable control is its <summary> child - <option>: not in default tab order; <select> is focused and arrow keys navigate options within it - <datalist>: no user-facing UI; the paired <input list> is focused Including them in UNCONDITIONAL_FOCUSABLE_TAGS was over-aggressive and caused false positives on this rule. Tests updated to pin the now-allowed cases.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 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
… 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
added a commit
that referenced
this pull request
Apr 26, 2026
…ec + correct peer-plugin claims
Two corrections to the previous revision:
1. Valueless / empty-string `aria-hidden` is no longer treated as a
non-interactive escape hatch. Per WAI-ARIA 1.2 §6.6 + aria-hidden
value table, a missing or empty-string value resolves to the default
`undefined` — NOT `true`. Only an explicit `aria-hidden="true"`
(ASCII case-insensitive) or mustache-literal `{{true}}` opts out.
This matches ember-cli#2717 / #19's spec-first resolution.
2. Code comment corrections. jsx-a11y's util is named
`isPresentationRole`, not `hasPresentationRole`. The comment also
claimed jsx-a11y's `isPresentationRole` does "first token of a
space-separated role list" — it does not (jsx-a11y does plain
`presentationRoles.has(rawValue)`, no trim/lowercase/split). Our
first-token behavior is a deliberate superset, not parity.
Moved `<div aria-hidden onclick>` and `<div aria-hidden="" onclick>` from
the valid section to invalid. Added `<div aria-hidden="TRUE">` as
additional valid coverage for the case-insensitive path.
johanrd
added a commit
that referenced
this pull request
Apr 26, 2026
…I-ARIA spec Per WAI-ARIA 1.2 §6.6 + aria-hidden value table, a missing or empty-string aria-hidden resolves to the default `undefined` — NOT `true`. So <span aria-hidden>X</span> as a child of <a href="/x"> does NOT hide the span; its content still contributes to the anchor's accessible name. The prior behavior inherited jsx-a11y's JSX-coercion convention and vue-a11y's "anything-not-literal-false" shortcut. Both are peer-plugin conventions that diverge from normative ARIA. Matches the spec-first resolution of ember-cli#2717, #19, and #33. Moved valueless / empty aria-hidden cases from invalid → valid. Kept the explicit aria-hidden="true" and {{true}} cases as invalid.
johanrd
force-pushed
the
feat/template-no-aria-hidden-on-focusable
branch
from
932d431 to
1d3c863
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
johanrd
force-pushed
the
feat/template-no-aria-hidden-on-focusable
branch
from
b629330 to
921767d
Compare
johanrd
added a commit
that referenced
this pull request
Apr 27, 2026
…I-ARIA spec Per WAI-ARIA 1.2 §6.6 + aria-hidden value table, a missing or empty-string aria-hidden resolves to the default `undefined` — NOT `true`. So <span aria-hidden>X</span> as a child of <a href="/x"> does NOT hide the span; its content still contributes to the anchor's accessible name. The prior behavior inherited jsx-a11y's JSX-coercion convention and vue-a11y's "anything-not-literal-false" shortcut. Both are peer-plugin conventions that diverge from normative ARIA. Matches the spec-first resolution of ember-cli#2717, #19, and #33. Moved valueless / empty aria-hidden cases from invalid → valid. Kept the explicit aria-hidden="true" and {{true}} cases as invalid.
johanrd
added a commit
that referenced
this pull request
Apr 27, 2026
…ec + correct peer-plugin claims
Two corrections to the previous revision:
1. Valueless / empty-string `aria-hidden` is no longer treated as a
non-interactive escape hatch. Per WAI-ARIA 1.2 §6.6 + aria-hidden
value table, a missing or empty-string value resolves to the default
`undefined` — NOT `true`. Only an explicit `aria-hidden="true"`
(ASCII case-insensitive) or mustache-literal `{{true}}` opts out.
This matches ember-cli#2717 / #19's spec-first resolution.
2. Code comment corrections. jsx-a11y's util is named
`isPresentationRole`, not `hasPresentationRole`. The comment also
claimed jsx-a11y's `isPresentationRole` does "first token of a
space-separated role list" — it does not (jsx-a11y does plain
`presentationRoles.has(rawValue)`, no trim/lowercase/split). Our
first-token behavior is a deliberate superset, not parity.
Moved `<div aria-hidden onclick>` and `<div aria-hidden="" onclick>` from
the valid section to invalid. Added `<div aria-hidden="TRUE">` as
additional valid coverage for the case-insensitive path.
johanrd
force-pushed
the
feat/template-no-aria-hidden-on-focusable
branch
from
921767d to
5f0a5da
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+12
to
+78
| valid: [ | ||
| // aria-hidden on non-focusable elements — fine. | ||
| '<template><div aria-hidden="true"></div></template>', | ||
| '<template><span aria-hidden="true">decorative</span></template>', | ||
| '<template><img src="/x.png" alt="" aria-hidden="true" /></template>', | ||
|
|
||
| // Focusable elements without aria-hidden — fine. | ||
| '<template><button>Click me</button></template>', | ||
| '<template><a href="/x">Link</a></template>', | ||
| '<template><input type="text" /></template>', | ||
|
|
||
| // aria-hidden="false" — explicit opt-out. Not flagged. | ||
| '<template><button aria-hidden="false">Click me</button></template>', | ||
|
|
||
| // Valueless / empty aria-hidden resolves to default `undefined` per | ||
| // WAI-ARIA 1.2 §6.6 — not hidden, not flagged even on focusable hosts. | ||
| '<template><button aria-hidden>Click me</button></template>', | ||
| '<template><button aria-hidden="">Click me</button></template>', | ||
| '<template><button aria-hidden={{false}}>Click me</button></template>', | ||
|
|
||
| // <input type="hidden"> isn't focusable, so aria-hidden on it is fine. | ||
| '<template><input type="hidden" aria-hidden="true" /></template>', | ||
| // Mustache string literal — statically resolvable to "hidden". | ||
| '<template><input type={{"hidden"}} aria-hidden="true" /></template>', | ||
|
|
||
| // <a> without href isn't focusable by default. | ||
| '<template><a aria-hidden="true">Not a link</a></template>', | ||
|
|
||
| // <label> is HTML interactive content but NOT keyboard-focusable by default | ||
| // (clicks forward to the associated control; the label itself isn't in the | ||
| // tab order). So aria-hidden on it is fine. | ||
| '<template><label aria-hidden="true">Name</label></template>', | ||
|
|
||
| // Disabled form controls are removed from the tab order (HTML §4.10.18.5), | ||
| // so they're not keyboard-focusable and aria-hidden on them isn't a trap. | ||
| '<template><button disabled aria-hidden="true">Click me</button></template>', | ||
| '<template><input disabled aria-hidden="true" /></template>', | ||
|
|
||
| // Components — we don't know if they render a focusable element. | ||
| '<template><CustomBtn aria-hidden="true" /></template>', | ||
|
|
||
| // <audio> / <video> without `controls` are not interactive — no focusable UI. | ||
| '<template><video aria-hidden="true"></video></template>', | ||
| '<template><audio aria-hidden="true"></audio></template>', | ||
| '<template><div aria-hidden="true"><video></video></div></template>', | ||
| '<template><div aria-hidden="true"><audio></audio></div></template>', | ||
|
|
||
| // Descendant-focusable check — valid cases. | ||
| // No focusable descendant. | ||
| '<template><div aria-hidden="true"><span>Just text</span></div></template>', | ||
| // Component descendants are opaque — conservatively not flagged. | ||
| '<template><div aria-hidden="true"><Button>X</Button></div></template>', | ||
| // No focusable descendants (alt-less img is decorative, not focusable). | ||
| '<template><div aria-hidden="true"><img alt="static" /></div></template>', | ||
| // <input type="hidden"> is non-focusable per isFocusable. | ||
| '<template><div aria-hidden="true"><input type="hidden" /></div></template>', | ||
| // Event modifiers (`{{on "click" ...}}`) do not make an element focusable — | ||
| // only tabindex / inherent native focusability does. | ||
| '<template><div {{on "click" this.handler}} aria-hidden="true"></div></template>', | ||
|
|
||
| // Dynamic mustache descendants are not inspected. | ||
| '<template><div aria-hidden="true">{{this.label}}</div></template>', | ||
| // `@arg`-prefixed tag is opaque. | ||
| '<template><div aria-hidden="true"><@thing /></div></template>', | ||
| // `this.`-prefixed tag is opaque. | ||
| '<template><div aria-hidden="true"><this.Item /></div></template>', | ||
| ], |
There was a problem hiding this comment.
Add regression tests for the static-true cases that currently won’t be detected if aria-hidden is expressed as a quoted mustache/concat (e.g. aria-hidden="{{true}}" / aria-hidden="{{'true'}}"). These are common template forms and help ensure isAriaHiddenTrue handles GlimmerConcatStatement correctly.
Comment on lines
+45
to
+222
| // Disabled form controls are removed from the tab order (HTML §4.10.18.5), | ||
| // so they're not keyboard-focusable and aria-hidden on them isn't a trap. | ||
| '<template><button disabled aria-hidden="true">Click me</button></template>', | ||
| '<template><input disabled aria-hidden="true" /></template>', | ||
|
|
||
| // Components — we don't know if they render a focusable element. | ||
| '<template><CustomBtn aria-hidden="true" /></template>', | ||
|
|
||
| // <audio> / <video> without `controls` are not interactive — no focusable UI. | ||
| '<template><video aria-hidden="true"></video></template>', | ||
| '<template><audio aria-hidden="true"></audio></template>', | ||
| '<template><div aria-hidden="true"><video></video></div></template>', | ||
| '<template><div aria-hidden="true"><audio></audio></div></template>', | ||
|
|
||
| // Descendant-focusable check — valid cases. | ||
| // No focusable descendant. | ||
| '<template><div aria-hidden="true"><span>Just text</span></div></template>', | ||
| // Component descendants are opaque — conservatively not flagged. | ||
| '<template><div aria-hidden="true"><Button>X</Button></div></template>', | ||
| // No focusable descendants (alt-less img is decorative, not focusable). | ||
| '<template><div aria-hidden="true"><img alt="static" /></div></template>', | ||
| // <input type="hidden"> is non-focusable per isFocusable. | ||
| '<template><div aria-hidden="true"><input type="hidden" /></div></template>', | ||
| // Event modifiers (`{{on "click" ...}}`) do not make an element focusable — | ||
| // only tabindex / inherent native focusability does. | ||
| '<template><div {{on "click" this.handler}} aria-hidden="true"></div></template>', | ||
|
|
||
| // Dynamic mustache descendants are not inspected. | ||
| '<template><div aria-hidden="true">{{this.label}}</div></template>', | ||
| // `@arg`-prefixed tag is opaque. | ||
| '<template><div aria-hidden="true"><@thing /></div></template>', | ||
| // `this.`-prefixed tag is opaque. | ||
| '<template><div aria-hidden="true"><this.Item /></div></template>', | ||
| ], | ||
| invalid: [ | ||
| // Native interactive elements. | ||
| { | ||
| code: '<template><button aria-hidden="true">Trapped</button></template>', | ||
| output: null, | ||
| errors: [{ messageId: 'noAriaHiddenOnFocusable' }], | ||
| }, | ||
| { | ||
| code: '<template><a href="/x" aria-hidden="true">Link</a></template>', | ||
| output: null, | ||
| errors: [{ messageId: 'noAriaHiddenOnFocusable' }], | ||
| }, | ||
| { | ||
| code: '<template><input type="text" aria-hidden="true" /></template>', | ||
| output: null, | ||
| errors: [{ messageId: 'noAriaHiddenOnFocusable' }], | ||
| }, | ||
| { | ||
| code: '<template><select aria-hidden="true"><option /></select></template>', | ||
| output: null, | ||
| errors: [{ messageId: 'noAriaHiddenOnFocusable' }], | ||
| }, | ||
| { | ||
| code: '<template><textarea aria-hidden="true"></textarea></template>', | ||
| output: null, | ||
| errors: [{ messageId: 'noAriaHiddenOnFocusable' }], | ||
| }, | ||
|
|
||
| // Non-interactive element made focusable via tabindex. | ||
| { | ||
| code: '<template><div tabindex="0" aria-hidden="true"></div></template>', | ||
| output: null, | ||
| errors: [{ messageId: 'noAriaHiddenOnFocusable' }], | ||
| }, | ||
| { | ||
| // tabindex="-1" still makes it programmatically focusable — still flag. | ||
| code: '<template><div tabindex="-1" aria-hidden="true"></div></template>', | ||
| output: null, | ||
| errors: [{ messageId: 'noAriaHiddenOnFocusable' }], | ||
| }, | ||
| // DIVERGENCE from jsx-a11y + vue-a11y: both accept button[tabindex="-1"][aria-hidden="true"] | ||
| // reasoning that tabindex="-1" removes the element from the tab order. | ||
| // Our rule treats tabindex="-1" as still programmatically focusable (reachable via | ||
| // .focus() and click), so aria-hidden on it still creates an AT-invisibility mismatch. | ||
| { | ||
| code: '<template><button aria-hidden="true" tabindex="-1"></button></template>', | ||
| output: null, | ||
| errors: [{ messageId: 'noAriaHiddenOnFocusable' }], | ||
| }, | ||
|
|
||
| // Mustache-boolean + case-variant aria-hidden = true — truthy per spec. | ||
| { | ||
| code: '<template><button aria-hidden={{true}}></button></template>', | ||
| output: null, | ||
| errors: [{ messageId: 'noAriaHiddenOnFocusable' }], | ||
| }, | ||
| { | ||
| code: '<template><button aria-hidden="TRUE"></button></template>', | ||
| output: null, | ||
| errors: [{ messageId: 'noAriaHiddenOnFocusable' }], | ||
| }, | ||
| { | ||
| // Whitespace-padded "true" is still a truthy aria-hidden per enumerated- | ||
| // attribute normalization (trim + case-insensitive). | ||
| code: '<template><button aria-hidden=" true "></button></template>', | ||
| output: null, | ||
| errors: [{ messageId: 'noAriaHiddenOnFocusable' }], | ||
| }, | ||
|
|
||
| // Descendant-focusable check. Per WAI-ARIA 1.2 §aria-hidden | ||
| // "may receive focus" — focusable descendants are keyboard-reachable | ||
| // under an aria-hidden ancestor, creating a keyboard trap. | ||
| { | ||
| // Classic modal-backdrop trap. | ||
| code: '<template><div aria-hidden="true"><button>Close</button></div></template>', | ||
| output: null, | ||
| errors: [{ messageId: 'noAriaHiddenOnAncestorOfFocusable' }], | ||
| }, | ||
| { | ||
| // Deeper descendant. | ||
| code: '<template><div aria-hidden="true"><span><button>Deep</button></span></div></template>', | ||
| output: null, | ||
| errors: [{ messageId: 'noAriaHiddenOnAncestorOfFocusable' }], | ||
| }, | ||
| { | ||
| code: '<template><div aria-hidden="true"><a href="/x">Link</a></div></template>', | ||
| output: null, | ||
| errors: [{ messageId: 'noAriaHiddenOnAncestorOfFocusable' }], | ||
| }, | ||
| { | ||
| code: '<template><div aria-hidden="true"><input /></div></template>', | ||
| output: null, | ||
| errors: [{ messageId: 'noAriaHiddenOnAncestorOfFocusable' }], | ||
| }, | ||
| { | ||
| // Depth check — focusable descendant two levels deep. | ||
| code: '<template><section aria-hidden="true"><div><textarea></textarea></div></section></template>', | ||
| output: null, | ||
| errors: [{ messageId: 'noAriaHiddenOnAncestorOfFocusable' }], | ||
| }, | ||
| { | ||
| // tabindex on a descendant makes it focusable. | ||
| code: '<template><div aria-hidden="true"><span tabindex="0">x</span></div></template>', | ||
| output: null, | ||
| errors: [{ messageId: 'noAriaHiddenOnAncestorOfFocusable' }], | ||
| }, | ||
| // DIVERGENCE from vue-a11y: it considers tabindex="-1" on a descendant as "escaped from | ||
| // tab order = not focusable". Our isFocusable treats any tabindex (including "-1") as | ||
| // programmatically focusable, so the ancestor aria-hidden still creates a trap. | ||
| { | ||
| code: '<template><div aria-hidden="true"><button tabindex="-1">Trapped</button></div></template>', | ||
| output: null, | ||
| errors: [{ messageId: 'noAriaHiddenOnAncestorOfFocusable' }], | ||
| }, | ||
| { | ||
| code: '<template><div aria-hidden="true"><a href="#" tabindex="-1">Link</a></div></template>', | ||
| output: null, | ||
| errors: [{ messageId: 'noAriaHiddenOnAncestorOfFocusable' }], | ||
| }, | ||
|
|
||
| // <audio controls> / <video controls> expose focusable UI; flag directly. | ||
| { | ||
| code: '<template><video controls aria-hidden="true"></video></template>', | ||
| output: null, | ||
| errors: [{ messageId: 'noAriaHiddenOnFocusable' }], | ||
| }, | ||
| { | ||
| code: '<template><audio controls aria-hidden="true"></audio></template>', | ||
| output: null, | ||
| errors: [{ messageId: 'noAriaHiddenOnFocusable' }], | ||
| }, | ||
| // <audio controls> / <video controls> as focusable descendants of an | ||
| // aria-hidden ancestor. | ||
| { | ||
| code: '<template><div aria-hidden="true"><video controls></video></div></template>', | ||
| output: null, | ||
| errors: [{ messageId: 'noAriaHiddenOnAncestorOfFocusable' }], | ||
| }, | ||
| { | ||
| code: '<template><div aria-hidden="true"><audio controls></audio></div></template>', | ||
| output: null, | ||
| errors: [{ messageId: 'noAriaHiddenOnAncestorOfFocusable' }], | ||
| }, | ||
| ], |
There was a problem hiding this comment.
Consider adding a test case for disabled={{false}} combined with aria-hidden="true" (and the analogous descendant case) to lock in the intended behavior that literal-false boolean attributes render as absent in Glimmer. Right now the rule would treat disabled={{false}} as disabled due to attribute presence checks.
Comment on lines
+82
to
+92
| describe('<img>', () => { | ||
| it('is interactive when usemap is present', () => { | ||
| expect(isHtmlInteractiveContent(makeNode('img', { usemap: '#m' }), getTextAttrValue)).toBe( | ||
| true | ||
| ); | ||
| }); | ||
|
|
||
| it('is NOT interactive without usemap', () => { | ||
| expect(isHtmlInteractiveContent(makeNode('img'), getTextAttrValue)).toBe(false); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
isHtmlInteractiveContent has an ignoreUsemap option but the test suite doesn’t currently exercise it. Add a test asserting <img usemap> is treated as non-interactive when { ignoreUsemap: true } to prevent regressions in callers that rely on this opt-out.
Comment on lines
+52
to
+56
| Component invocations, argument/`this`/path-based tags, and namespace-pathed | ||
| tags are "opaque" — we can't statically know what they render. The descendant | ||
| check skips these branches to avoid false positives. If a component renders a | ||
| focusable element beneath an `aria-hidden` ancestor, the keyboard trap still | ||
| exists at runtime; this rule can't detect it. |
There was a problem hiding this comment.
The “Caveats” section lists several kinds of opaque tags but doesn’t mention custom elements (e.g. <my-widget>). Since the rule uses isNativeElement, custom elements are also skipped, which can surprise readers expecting tabindex-based focusability on custom elements to be reported. Consider explicitly mentioning custom elements in this caveat list to align docs with behavior.
Suggested change
| Component invocations, argument/`this`/path-based tags, and namespace-pathed | |
| tags are "opaque" — we can't statically know what they render. The descendant | |
| check skips these branches to avoid false positives. If a component renders a | |
| focusable element beneath an `aria-hidden` ancestor, the keyboard trap still | |
| exists at runtime; this rule can't detect it. | |
| Component invocations, argument/`this`/path-based tags, namespace-pathed tags, | |
| and custom elements (for example, `<my-widget>`) are "opaque" — we can't | |
| statically know what they render. The descendant check skips these branches to | |
| avoid false positives. If one of these renders a focusable element beneath an | |
| `aria-hidden` ancestor, the keyboard trap still exists at runtime; this rule | |
| can't detect it. |
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.aria-hidden="true"creates a keyboard trap — reachable via Tab, hidden from assistive technology. This is the anti-pattern flagged by axe'saria-hidden-focusrule and by jsx-a11y'sno-aria-hidden-on-focusable. The WAI-ARIA 1.2 spec itself only says authors MAY "with caution" use aria-hidden; the specific "keyboard-reachable + AT-hidden = trap" framing is community/axe guidance, not a normative MUST-NOT.Fix: add
template-no-aria-hidden-on-focusable.Four ecosystem positions on valueless aria-hidden
The question "what does
<el aria-hidden>(bare),aria-hidden=""(empty), oraria-hidden={{false}}mean?" has no single authoritative answer. Four defensible positions exist:jsx-ast-utilscoercing valueless JSX attrs to booleantrue, combined with rule checkariaHidden === true. Quirk: stringaria-hidden="true"is NOT recognized because"true" !== true. Not a deliberate ARIA interpretation."false"→ hiddenisHiddenFromScreenReader.ts:(value || "").toString() !== "false". Catches valueless, empty,"TRUE","anything". Non-spec shortcut.undefined→ not hiddentrue/false/undefined (default). Missing/empty resolves to the default.aria-hiddenis NOT an HTML boolean attribute — the HTML spec never designates it as such.Browser testing shows disagreement even on the explicit
aria-hidden="true"case (see Steve Faulkner's post and Mozilla bug 948540); no documented browser testing on valueless specifically.Design choice for this rule
We lean toward fewer false positives. For this rule that means: only flag when aria-hidden is unambiguously "true" — the author has clearly asserted the element is hidden but also made it keyboard-focusable. When the signal is ambiguous (valueless, empty, "false"), we don't report a keyboard trap.
Flag (explicit hide + focusable = trap):
aria-hidden="true"/"TRUE"/"True"(ASCII case-insensitive)aria-hidden={{true}},{{"true"}}/ case-variantsDon't flag:
<button aria-hidden>(valueless — ambiguous)<button aria-hidden="">(empty — ambiguous)<button aria-hidden={{false}}>/"false"— explicit opt-outFlags
Allows
Prior art
<button aria-hidden>no-aria-hidden-on-focusableno-aria-hidden-on-focusableif (hasAriaHidden)short-circuits on null).