Skip to content

Commit a1c2816

Browse files
committed
fix: treat aria-hidden={{true}} as an explicit escape hatch in click-events check
Premise: jsx-a11y accepts `aria-hidden={true}` (JSX boolean literal) as a static opt-out of the click-events-have-key-events check. The HBS analog is `aria-hidden={{true}}` — a GlimmerMustacheStatement whose path is a GlimmerBooleanLiteral with value `true`. The previous `isHiddenFromScreenReader` helper happened to treat this correctly, but only because it also treated ANY mustache value as truthy (e.g. `aria-hidden={{this.maybeHidden}}` would silence the rule as if the element were guaranteed hidden). That is too permissive: a dynamic value can't be proven hidden at lint time, so the rule should still fire. Conclusion: rewrite the aria-hidden check to distinguish the two shapes explicitly. - `isHiddenFromScreenReader` now only returns true for: - GlimmerTextNode value with chars `""` (bare attribute) or `"true"` (case-insensitive, trimmed). - GlimmerMustacheStatement whose `path` is a GlimmerBooleanLiteral with value `true` — i.e. the literal `{{true}}`. - Other mustache shapes (path references, helper calls) are now treated as dynamic/unknown, so the rule fires. Authors who want a static escape hatch should write `aria-hidden` or `aria-hidden="true"` or `aria-hidden={{true}}`. - Tests added for the mustache-literal valid case and for the new dynamic-mustache invalid cases (`{{false}}`, `{{this.x}}`). - Audit fixture: adds `aria-hidden={{true}}` as explicit parity with jsx-a11y's `aria-hidden={true}`. Tracks PR #28 item G1 (escape-hatch awareness). Companion to the master-side `template-no-invalid-interactive` fix.
1 parent dbfcbad commit a1c2816

3 files changed

Lines changed: 43 additions & 3 deletions

File tree

lib/rules/template-click-events-have-key-events.js

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,12 +37,32 @@ function getTextAttrValue(attr) {
3737
return undefined;
3838
}
3939

40+
// True iff the attribute's mustache value is the literal boolean `true` —
41+
// e.g. `aria-hidden={{true}}`. Any other expression (path reference, helper
42+
// call, etc.) is left to runtime and not treated as a static escape hatch.
43+
function isMustacheLiteralTrue(attr) {
44+
if (attr?.value?.type !== 'GlimmerMustacheStatement') {
45+
return false;
46+
}
47+
const path = attr.value.path;
48+
return path?.type === 'GlimmerBooleanLiteral' && path.value === true;
49+
}
50+
4051
function isHiddenFromScreenReader(node) {
4152
const ariaHidden = findAttr(node, 'aria-hidden');
4253
if (ariaHidden) {
43-
const v = getTextAttrValue(ariaHidden);
44-
// Presence without "false" is truthy (boolean attribute convention).
45-
if (v === undefined || v === '' || v === 'true') {
54+
// Static text values: bare `aria-hidden` (empty chars, boolean-attribute
55+
// convention) or the literal "true".
56+
if (ariaHidden.value?.type === 'GlimmerTextNode') {
57+
const chars = ariaHidden.value.chars;
58+
if (chars === '' || chars.trim().toLowerCase() === 'true') {
59+
return true;
60+
}
61+
}
62+
// Mustache-literal `{{true}}` — unambiguous static escape hatch. Any
63+
// other mustache shape (path reference, helper invocation) is dynamic
64+
// and intentionally NOT treated as hidden.
65+
if (isMustacheLiteralTrue(ariaHidden)) {
4666
return true;
4767
}
4868
}

tests/audit/click-events-have-key-events/peer-parity.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,9 @@ ruleTester.run('audit:click-events-have-key-events (gts)', rule, {
5252
// angular: valid (static `aria-hidden`, `aria-hidden="true"`).
5353
'<template><div aria-hidden {{on "click" this.onClick}}></div></template>',
5454
'<template><div aria-hidden="true" {{on "click" this.onClick}}></div></template>',
55+
// `aria-hidden={{true}}` — HBS analog of jsx-a11y's `aria-hidden={true}`.
56+
// Our rule treats the mustache-literal boolean as a static opt-out.
57+
'<template><div aria-hidden={{true}} {{on "click" this.onClick}}></div></template>',
5558

5659
// aria-hidden=false paired with a keyboard listener → still valid because
5760
// the keyboard listener is present.
@@ -290,6 +293,7 @@ hbsRuleTester.run('audit:click-events-have-key-events (hbs)', rule, {
290293
// aria-hidden.
291294
'<div aria-hidden {{on "click" this.a}}></div>',
292295
'<div aria-hidden="true" {{on "click" this.a}}></div>',
296+
'<div aria-hidden={{true}} {{on "click" this.a}}></div>',
293297

294298
// Inherently-interactive.
295299
'<button {{on "click" this.a}}></button>',

tests/lib/rules/template-click-events-have-key-events.js

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ ruleTester.run('template-click-events-have-key-events', rule, {
2525
// Hidden from AT.
2626
'<template><div aria-hidden="true" {{on "click" this.noop}}></div></template>',
2727
'<template><div aria-hidden {{on "click" this.noop}}></div></template>',
28+
// Mustache-literal boolean `true` — explicit static opt-out.
29+
'<template><div aria-hidden={{true}} {{on "click" this.noop}}></div></template>',
2830
'<template><div hidden {{on "click" this.noop}}></div></template>',
2931

3032
// Presentation role — content has no semantics for AT.
@@ -66,6 +68,20 @@ ruleTester.run('template-click-events-have-key-events', rule, {
6668
output: null,
6769
errors: [{ messageId: 'needsKeyEvent' }],
6870
},
71+
// Mustache-literal `{{false}}` is explicitly not-hidden.
72+
{
73+
code: '<template><div aria-hidden={{false}} {{on "click" this.onClick}}></div></template>',
74+
output: null,
75+
errors: [{ messageId: 'needsKeyEvent' }],
76+
},
77+
// Dynamic mustache (non-literal) — rule can't prove the element is
78+
// hidden, so it still fires. Authors who intend aria-hidden as a static
79+
// escape hatch should use a literal.
80+
{
81+
code: '<template><div aria-hidden={{this.maybeHidden}} {{on "click" this.onClick}}></div></template>',
82+
output: null,
83+
errors: [{ messageId: 'needsKeyEvent' }],
84+
},
6985
// Presentation on a focusable widget isn't a valid exemption — but the rule
7086
// intentionally only checks role=presentation/none, not the focusable state.
7187
// The separate `no-role-presentation-on-focusable` rule handles that case.

0 commit comments

Comments
 (0)