Skip to content

Commit 8ec2054

Browse files
johanrdjohanrd
committed
fix: template-no-autofocus-attribute — value-aware + <dialog> exception
Closes two gaps identified in PR #28 Phase 3 audit (B10 fixture on `audit/phase3/no-autofocus`): G3 — value-aware detection. The rule previously flagged `autofocus` purely on presence, producing false positives for explicit opt-out forms: - `autofocus="false"` (GlimmerTextNode chars === "false") - `autofocus={{false}}` (BooleanLiteral false) - `autofocus={{"false"}}` (StringLiteral "false") These are now treated as valid. jsx-a11y's `no-autofocus` reads the value via `getPropValue` and exits early on falsy results, which is the behavior encoded here. Truthy literals (`="true"`, `="autofocus"`, `={{true}}`, `={{"true"}}`) and any dynamic expression (`={{this.x}}`) still flag — the rule cannot prove a dynamic value is safe. Mustache-hash-pair forms (`{{input autofocus=false}}`, `{{input autofocus="false"}}`) receive the same value-aware treatment so `autofocus=true` and `autofocus=false` do not behave inconsistently between syntaxes. G4 — `<dialog>` exception. The rule now skips reporting when the element carrying `autofocus` is a `<dialog>` itself OR is nested at any depth inside a `<dialog>`. Per MDN's `<dialog>` documentation, a dialog is expected to move focus to its initial control on open, so `autofocus` on or within a dialog is the recommended pattern rather than an accessibility defect. This matches `@angular-eslint/template/no-autofocus`, which exempts the same subtree. The descendant walk is a full parent-chain traversal via `node.parent`; the Glimmer AST in this codebase exposes `parent` on every element node, so no scope narrowing was required. Behavior unchanged for: bare `<input autofocus>`, truthy string/mustache values, dynamic mustache values, and any `autofocus` outside a dialog subtree. 22 rule tests (10 valid, 12 invalid) pass; full suite (9089 tests) green. Audit fixture `tests/audit/no-autofocus/peer-parity.js` lives on branch `audit/phase3/no-autofocus` and will need separate updating to reflect the new parity status for G3 and G4. Refs: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/dialog Refs: https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/blob/main/src/rules/no-autofocus.js Refs: #28 (G3, G4)
1 parent 24882a3 commit 8ec2054

2 files changed

Lines changed: 221 additions & 23 deletions

File tree

‎lib/rules/template-no-autofocus-attribute.js‎

Lines changed: 112 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,64 @@
1+
/**
2+
* Returns true when the attribute value is statically known to be falsy
3+
* (i.e. the developer has written `autofocus="false"`, `autofocus={{false}}`,
4+
* or `autofocus={{"false"}}`). These forms are aligned with jsx-a11y's
5+
* `no-autofocus` rule, which reads the attribute value via `getPropValue` and
6+
* skips reporting when the value is falsy (e.g. `<div autoFocus={false} />`).
7+
*
8+
* Valueless attributes (`<input autofocus>`) are TRUTHY per the HTML spec
9+
* (boolean attribute present == "on") and must still be flagged.
10+
*/
11+
function isExplicitlyFalsy(value) {
12+
if (!value) {
13+
// No value property at all — treat as bare boolean attribute (truthy).
14+
return false;
15+
}
16+
17+
if (value.type === 'GlimmerTextNode') {
18+
// `autofocus="false"` → chars === "false". Bare `autofocus` has chars === "".
19+
return value.chars === 'false';
20+
}
21+
22+
if (value.type === 'GlimmerMustacheStatement') {
23+
const expr = value.path;
24+
if (!expr) {
25+
return false;
26+
}
27+
// `autofocus={{false}}` → BooleanLiteral(false).
28+
if (expr.type === 'GlimmerBooleanLiteral' && expr.value === false) {
29+
return true;
30+
}
31+
// `autofocus={{"false"}}` → StringLiteral("false").
32+
if (expr.type === 'GlimmerStringLiteral' && expr.value === 'false') {
33+
return true;
34+
}
35+
}
36+
37+
return false;
38+
}
39+
40+
/**
41+
* Returns true when the given GlimmerElementNode is a `<dialog>` element
42+
* or is nested (at any depth) inside a `<dialog>` element. Per MDN,
43+
* autofocus on (or within) a dialog is recommended because a dialog should
44+
* focus its initial element when opened.
45+
*
46+
* https://developer.mozilla.org/en-US/docs/Web/HTML/Element/dialog
47+
*/
48+
function isInsideDialog(node) {
49+
if (node.type === 'GlimmerElementNode' && node.tag === 'dialog') {
50+
return true;
51+
}
52+
let ancestor = node.parent;
53+
while (ancestor) {
54+
if (ancestor.type === 'GlimmerElementNode' && ancestor.tag === 'dialog') {
55+
return true;
56+
}
57+
ancestor = ancestor.parent;
58+
}
59+
return false;
60+
}
61+
162
/** @type {import('eslint').Rule.RuleModule} */
263
module.exports = {
364
meta: {
@@ -27,38 +88,66 @@ module.exports = {
2788
GlimmerElementNode(node) {
2889
const autofocusAttr = node.attributes?.find((attr) => attr.name === 'autofocus');
2990

30-
if (autofocusAttr) {
31-
context.report({
32-
node: autofocusAttr,
33-
messageId: 'noAutofocus',
34-
fix(fixer) {
35-
const sourceCode = context.sourceCode;
36-
const text = sourceCode.getText();
37-
const attrStart = autofocusAttr.range[0];
38-
const attrEnd = autofocusAttr.range[1];
39-
40-
let removeStart = attrStart;
41-
while (removeStart > 0 && /\s/.test(text[removeStart - 1])) {
42-
removeStart--;
43-
}
44-
45-
return fixer.removeRange([removeStart, attrEnd]);
46-
},
47-
});
91+
if (!autofocusAttr) {
92+
return;
4893
}
94+
95+
// jsx-a11y parity: `autofocus="false"` / `={{false}}` / `={{"false"}}`
96+
// explicitly opt out and should not be flagged.
97+
if (isExplicitlyFalsy(autofocusAttr.value)) {
98+
return;
99+
}
100+
101+
// MDN dialog exception: autofocus on a <dialog> or inside a <dialog>
102+
// is recommended behavior, not an accessibility defect.
103+
if (isInsideDialog(node)) {
104+
return;
105+
}
106+
107+
context.report({
108+
node: autofocusAttr,
109+
messageId: 'noAutofocus',
110+
fix(fixer) {
111+
const sourceCode = context.sourceCode;
112+
const text = sourceCode.getText();
113+
const attrStart = autofocusAttr.range[0];
114+
const attrEnd = autofocusAttr.range[1];
115+
116+
let removeStart = attrStart;
117+
while (removeStart > 0 && /\s/.test(text[removeStart - 1])) {
118+
removeStart--;
119+
}
120+
121+
return fixer.removeRange([removeStart, attrEnd]);
122+
},
123+
});
49124
},
50125

51126
GlimmerMustacheStatement(node) {
52127
if (!node.hash || !node.hash.pairs) {
53128
return;
54129
}
55130
const autofocusPair = node.hash.pairs.find((pair) => pair.key === 'autofocus');
56-
if (autofocusPair) {
57-
context.report({
58-
node: autofocusPair,
59-
messageId: 'noAutofocus',
60-
});
131+
if (!autofocusPair) {
132+
return;
133+
}
134+
135+
// Value-aware check for component/helper invocations such as
136+
// `{{input autofocus=false}}` or `{{input autofocus="false"}}`.
137+
const pairValue = autofocusPair.value;
138+
if (pairValue) {
139+
if (pairValue.type === 'GlimmerBooleanLiteral' && pairValue.value === false) {
140+
return;
141+
}
142+
if (pairValue.type === 'GlimmerStringLiteral' && pairValue.value === 'false') {
143+
return;
144+
}
61145
}
146+
147+
context.report({
148+
node: autofocusPair,
149+
messageId: 'noAutofocus',
150+
});
62151
},
63152
};
64153
},

‎tests/lib/rules/template-no-autofocus-attribute.js‎

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,39 @@ ruleTester.run('template-no-autofocus-attribute', rule, {
2222
`<template>
2323
<button>Click me</button>
2424
</template>`,
25+
// Value-aware: explicit falsy values opt out (jsx-a11y parity).
26+
`<template>
27+
<input autofocus="false" />
28+
</template>`,
29+
`<template>
30+
<input autofocus={{false}} />
31+
</template>`,
32+
`<template>
33+
<input autofocus={{"false"}} />
34+
</template>`,
35+
`<template>
36+
{{input autofocus=false}}
37+
</template>`,
38+
`<template>
39+
{{input autofocus="false"}}
40+
</template>`,
41+
// Dialog exception (MDN): autofocus on <dialog> is recommended.
42+
`<template>
43+
<dialog autofocus></dialog>
44+
</template>`,
45+
// Dialog descendants are also exempt (angular-eslint parity).
46+
`<template>
47+
<dialog>
48+
<button autofocus>Close</button>
49+
</dialog>
50+
</template>`,
51+
`<template>
52+
<dialog>
53+
<div>
54+
<input autofocus />
55+
</div>
56+
</dialog>
57+
</template>`,
2558
],
2659

2760
invalid: [
@@ -123,5 +156,81 @@ ruleTester.run('template-no-autofocus-attribute', rule, {
123156
},
124157
],
125158
},
159+
// Value-aware: truthy literals and any dynamic value still flag.
160+
{
161+
code: `<template>
162+
<input autofocus="true" />
163+
</template>`,
164+
output: `<template>
165+
<input />
166+
</template>`,
167+
errors: [
168+
{
169+
messageId: 'noAutofocus',
170+
type: 'GlimmerAttrNode',
171+
},
172+
],
173+
},
174+
{
175+
code: `<template>
176+
<input autofocus={{true}} />
177+
</template>`,
178+
output: `<template>
179+
<input />
180+
</template>`,
181+
errors: [
182+
{
183+
messageId: 'noAutofocus',
184+
type: 'GlimmerAttrNode',
185+
},
186+
],
187+
},
188+
{
189+
code: `<template>
190+
<input autofocus={{"true"}} />
191+
</template>`,
192+
output: `<template>
193+
<input />
194+
</template>`,
195+
errors: [
196+
{
197+
messageId: 'noAutofocus',
198+
type: 'GlimmerAttrNode',
199+
},
200+
],
201+
},
202+
{
203+
code: `<template>
204+
<input autofocus={{this.shouldFocus}} />
205+
</template>`,
206+
output: `<template>
207+
<input />
208+
</template>`,
209+
errors: [
210+
{
211+
messageId: 'noAutofocus',
212+
type: 'GlimmerAttrNode',
213+
},
214+
],
215+
},
216+
// Dialog exception only applies within <dialog>; siblings elsewhere still flag.
217+
{
218+
code: `<template>
219+
<section>
220+
<button autofocus>Focus</button>
221+
</section>
222+
</template>`,
223+
output: `<template>
224+
<section>
225+
<button>Focus</button>
226+
</section>
227+
</template>`,
228+
errors: [
229+
{
230+
messageId: 'noAutofocus',
231+
type: 'GlimmerAttrNode',
232+
},
233+
],
234+
},
126235
],
127236
});

0 commit comments

Comments
 (0)