Skip to content

Commit 5be2acd

Browse files
committed
fix(template-no-aria-hidden-on-focusable): handle aria-hidden concat form; treat only bare-mustache disabled={{false}} as omitted; doc custom-element caveat; ignoreUsemap test
1 parent 5f0a5da commit 5be2acd

4 files changed

Lines changed: 52 additions & 12 deletions

File tree

docs/rules/template-no-aria-hidden-on-focusable.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,11 @@ check skips these branches to avoid false positives. If a component renders a
5555
focusable element beneath an `aria-hidden` ancestor, the keyboard trap still
5656
exists at runtime; this rule can't detect it.
5757

58+
Custom elements (hyphenated tags like `<my-widget>`) are similarly skipped: we
59+
can't know whether their shadow DOM defines a focusable region. If
60+
`<my-widget aria-hidden="true">` renders a focusable element internally, the
61+
trap still exists at runtime — this rule can't detect it.
62+
5863
Dynamic content inside `{{...}}` mustache statements is similarly not inspected.
5964

6065
## References

lib/rules/template-no-aria-hidden-on-focusable.js

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -27,18 +27,13 @@ function isAriaHiddenTrue(node) {
2727
if (!value) {
2828
return false;
2929
}
30-
if (value.type === 'GlimmerTextNode') {
31-
return value.chars.trim().toLowerCase() === 'true';
32-
}
33-
if (value.type === 'GlimmerMustacheStatement' && value.path) {
34-
if (value.path.type === 'GlimmerBooleanLiteral') {
35-
return value.path.value === true;
36-
}
37-
if (value.path.type === 'GlimmerStringLiteral') {
38-
return value.path.value.trim().toLowerCase() === 'true';
39-
}
30+
// Resolve through getStaticAttrValue so quoted-mustache concat forms
31+
// (e.g. aria-hidden="{{true}}") and case variants normalize uniformly.
32+
const resolved = getStaticAttrValue(value);
33+
if (typeof resolved !== 'string') {
34+
return false;
4035
}
41-
return false;
36+
return resolved.trim().toLowerCase() === 'true';
4237
}
4338

4439
// Tags with an unconditional default focusable UI (sequentially focusable per
@@ -67,7 +62,23 @@ function isDisabledFormControl(node, tag) {
6762
if (!DISABLEABLE_TAGS.has(tag)) {
6863
return false;
6964
}
70-
return Boolean(findAttr(node, 'disabled'));
65+
const attr = findAttr(node, 'disabled');
66+
if (!attr) {
67+
return false;
68+
}
69+
// Per docs/glimmer-attribute-behavior.md, ONLY bare-mustache boolean-false
70+
// (`disabled={{false}}`) renders as omitted at runtime — concat
71+
// (`disabled="{{false}}"`) and string-literal (`disabled={{"false"}}`) forms
72+
// still render the attribute as present and the control IS disabled.
73+
const v = attr.value;
74+
if (
75+
v?.type === 'GlimmerMustacheStatement' &&
76+
v.path?.type === 'GlimmerBooleanLiteral' &&
77+
v.path.value === false
78+
) {
79+
return false;
80+
}
81+
return true;
7182
}
7283

7384
// Narrow rule-local "keyboard-focusable" check. Intentionally distinct from

tests/lib/rules/template-no-aria-hidden-on-focusable.js

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,22 @@ ruleTester.run('template-no-aria-hidden-on-focusable', rule, {
132132
output: null,
133133
errors: [{ messageId: 'noAriaHiddenOnFocusable' }],
134134
},
135+
// GlimmerConcatStatement form. Per docs/glimmer-attribute-behavior.md,
136+
// `aria-hidden="{{true}}"` renders as `aria-hidden="true"`.
137+
{
138+
code: '<template><button aria-hidden="{{true}}"></button></template>',
139+
output: null,
140+
errors: [{ messageId: 'noAriaHiddenOnFocusable' }],
141+
},
142+
// disabled={{false}} is the bare-mustache boolean-false case — Glimmer
143+
// omits the attribute at runtime, so the button stays focusable and
144+
// aria-hidden="true" traps it. (`disabled="{{false}}"` and
145+
// `disabled={{"false"}}` would NOT omit; those keep the button disabled.)
146+
{
147+
code: '<template><button aria-hidden="true" disabled={{false}}>click</button></template>',
148+
output: null,
149+
errors: [{ messageId: 'noAriaHiddenOnFocusable' }],
150+
},
135151
{
136152
code: '<template><button aria-hidden="TRUE"></button></template>',
137153
output: null,

tests/lib/utils/html-interactive-content-test.js

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,14 @@ describe('isHtmlInteractiveContent', () => {
8989
it('is NOT interactive without usemap', () => {
9090
expect(isHtmlInteractiveContent(makeNode('img'), getTextAttrValue)).toBe(false);
9191
});
92+
93+
it('is NOT interactive when usemap is present but { ignoreUsemap: true }', () => {
94+
expect(
95+
isHtmlInteractiveContent(makeNode('img', { usemap: '#m' }), getTextAttrValue, {
96+
ignoreUsemap: true,
97+
})
98+
).toBe(false);
99+
});
92100
});
93101

94102
describe('<audio> / <video>', () => {

0 commit comments

Comments
 (0)