Skip to content

Commit ceaf0cf

Browse files
committed
fix(template-no-empty-headings): align aria-hidden with WAI-ARIA spec 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.
1 parent 9dd106d commit ceaf0cf

5 files changed

Lines changed: 241 additions & 78 deletions

File tree

docs/rules/template-no-empty-headings.md

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -52,18 +52,11 @@ If violations are found, remediation should be planned to ensure text content is
5252

5353
## Notes on `aria-hidden` semantics
5454

55-
This rule treats valueless / empty-string `aria-hidden` (`<h1 aria-hidden>` or `<h1 aria-hidden="">`) as exempting the heading from the empty-content checkthose forms count as "hidden" for this rule.
55+
This rule follows [WAI-ARIA 1.2 §`aria-hidden`](https://www.w3.org/TR/wai-aria-1.2/#aria-hidden) verbatim: only an explicit truthy value hides the element. Ambiguous shapes — valueless `aria-hidden`, empty string, and mustache literals that resolve to an empty / whitespace-only stringall resolve to the default `undefined` and do NOT exempt the heading from the empty-content check.
5656

57-
**This is a deliberate deviation from [WAI-ARIA 1.2 §`aria-hidden`](https://www.w3.org/TR/wai-aria-1.2/#aria-hidden)**, which resolves valueless / empty-string `aria-hidden` to the default value `undefined` — not `true` — and therefore does not hide the element per spec. The spec-literal reading would say "valueless `aria-hidden` doesn't hide, so the empty heading is still a violation."
58-
59-
We lean toward fewer false positives here: if the author wrote `aria-hidden` at all, they signaled an intent to hide, and flagging the empty heading on top of what is already a malformed `aria-hidden` usage layers a second-order complaint on a first-order problem. Axe-core and the W3C ACT rules consistently treat this shape as INCOMPLETE (needs manual review) rather than a definitive failure, which is consistent with leaning away from a hard flag here.
60-
61-
For rules that ask the _opposite_ question ("is this element authoritatively hidden?"), the spec-literal reading applies, and valueless `aria-hidden` should be treated as **not** hidden. This split is applied per-rule, picking the interpretation that produces the fewest false positives for each specific check.
62-
63-
Unambiguous forms always follow the spec:
64-
65-
- `aria-hidden="true"` / `aria-hidden={{true}}` / `aria-hidden={{"true"}}` (any case) → hidden, exempts the heading.
66-
- `aria-hidden="false"` / `aria-hidden={{false}}` / `aria-hidden={{"false"}}` → not hidden, the empty-content check still applies.
57+
- `aria-hidden="true"` / `aria-hidden={{true}}` / `aria-hidden={{"true"}}` (any case, whitespace-trimmed) → hidden, exempts the heading.
58+
- `aria-hidden="false"` / `aria-hidden={{false}}` / `aria-hidden={{"false"}}` → not hidden, the empty-content check applies.
59+
- `<h1 aria-hidden>` / `aria-hidden=""` / `aria-hidden={{""}}` / `aria-hidden={{" "}}` → spec-default `undefined`, the empty-content check applies.
6760

6861
## References
6962

lib/rules/template-no-empty-headings.js

Lines changed: 12 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -1,74 +1,23 @@
1+
const { getStaticAttrValue } = require('../utils/static-attr-value');
2+
13
const HEADINGS = new Set(['h1', 'h2', 'h3', 'h4', 'h5', 'h6']);
24

3-
// aria-hidden semantics for valueless / empty / "false" are genuinely
4-
// contested across common accessibility tooling and spec interpretations.
5-
// For this rule, we prefer FEWER false positives: when the author has written
6-
// `aria-hidden` in any form that could plausibly mean "hide this", we exempt
7-
// the heading from the empty-content check. The downside (missing some
8-
// genuinely-empty headings) is preferable to flagging correctly-authored
9-
// headings the developer intentionally decorated. See
10-
// docs/rules/template-no-empty-headings.md for the rule-level rationale.
11-
//
12-
// Truthy:
13-
// - valueless attr (`<h1 aria-hidden>`) — default-undefined per spec, but
14-
// authors who write bare `aria-hidden` plausibly intend hidden.
15-
// - empty string `aria-hidden=""` — same.
16-
// - `aria-hidden="true"` / "TRUE" / "True" (ASCII case-insensitive).
17-
// - `aria-hidden={{true}}` mustache boolean literal.
18-
// - `aria-hidden={{"true"}}` / case-variants as mustache string literal.
19-
// Not truthy (falls through):
20-
// - `aria-hidden="false"` / `{{false}}` / `{{"false"}}` — explicit opt-out.
5+
// Aligned with WAI-ARIA 1.2 §6.6 aria-hidden value table: only an explicit
6+
// "true" (ASCII case-insensitive, whitespace-trimmed) hides the element.
7+
// Valueless `<h1 aria-hidden>`, empty-string `aria-hidden=""`, and
8+
// `aria-hidden="false"` all resolve to the default `undefined` / explicit
9+
// false — so the empty-content check still applies. All shape-unwrapping
10+
// (mustache/concat) goes through the shared `getStaticAttrValue` helper.
2111
function isAriaHiddenTruthy(attr) {
2212
if (!attr) {
2313
return false;
2414
}
25-
const value = attr.value;
26-
// Valueless attribute — no `value` property at all.
27-
if (!value) {
28-
return true;
29-
}
30-
if (value.type === 'GlimmerTextNode') {
31-
// Normalize like other aria-* value checks: trim incidental whitespace
32-
// and compare case-insensitively. `aria-hidden=" true "` is semantically
33-
// "true" per the trim step used elsewhere in this rule family.
34-
const chars = value.chars.trim().toLowerCase();
35-
// Empty string is exempted (lean toward fewer false positives).
36-
return chars === '' || chars === 'true';
37-
}
38-
if (value.type === 'GlimmerMustacheStatement' && value.path) {
39-
if (value.path.type === 'GlimmerBooleanLiteral') {
40-
return value.path.value === true;
41-
}
42-
if (value.path.type === 'GlimmerStringLiteral') {
43-
return value.path.value.trim().toLowerCase() === 'true';
44-
}
45-
}
46-
if (value.type === 'GlimmerConcatStatement') {
47-
// Quoted-mustache form like aria-hidden="{{true}}" or aria-hidden="{{x}}".
48-
// Only resolve when the concat is a single static-literal part; any
49-
// dynamic path makes the runtime value unknown. Lean toward "truthy"
50-
// only on literal `true` / empty-literal / bare-valueless to stay aligned
51-
// with the doc-stated ethos (fewer false positives — don't flag headings
52-
// the author has intentionally decorated with aria-hidden).
53-
const parts = value.parts || [];
54-
if (parts.length === 1) {
55-
const only = parts[0];
56-
if (only.type === 'GlimmerMustacheStatement' && only.path) {
57-
if (only.path.type === 'GlimmerBooleanLiteral') {
58-
return only.path.value === true;
59-
}
60-
if (only.path.type === 'GlimmerStringLiteral') {
61-
return only.path.value.trim().toLowerCase() === 'true';
62-
}
63-
}
64-
if (only.type === 'GlimmerTextNode') {
65-
const chars = only.chars.trim().toLowerCase();
66-
return chars === '' || chars === 'true';
67-
}
68-
}
15+
const resolved = getStaticAttrValue(attr.value);
16+
if (resolved === undefined) {
17+
// Dynamic — can't prove truthy.
6918
return false;
7019
}
71-
return false;
20+
return resolved.trim().toLowerCase() === 'true';
7221
}
7322

7423
function isHidden(node) {

lib/utils/static-attr-value.js

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
'use strict';
2+
3+
/**
4+
* Return the statically-known string value of a Glimmer attribute value node,
5+
* or `undefined` when the value is dynamic (cannot be resolved at lint time).
6+
*
7+
* Unwraps:
8+
* - GlimmerTextNode → chars
9+
* - GlimmerMustacheStatement with a literal path (boolean/string/number) → stringified value
10+
* - GlimmerConcatStatement whose parts are all statically resolvable → joined string
11+
*
12+
* A missing/undefined value (valueless attribute, e.g. `<input disabled>`)
13+
* returns the empty string. Pass `attr.value` — not the attribute itself.
14+
*/
15+
function getStaticAttrValue(value) {
16+
if (value === null || value === undefined) {
17+
return '';
18+
}
19+
if (value.type === 'GlimmerTextNode') {
20+
return value.chars;
21+
}
22+
if (value.type === 'GlimmerMustacheStatement') {
23+
return extractLiteral(value.path);
24+
}
25+
if (value.type === 'GlimmerConcatStatement') {
26+
const parts = value.parts || [];
27+
let out = '';
28+
for (const part of parts) {
29+
if (part.type === 'GlimmerTextNode') {
30+
out += part.chars;
31+
continue;
32+
}
33+
if (part.type === 'GlimmerMustacheStatement') {
34+
const literal = extractLiteral(part.path);
35+
if (literal === undefined) {
36+
return undefined;
37+
}
38+
out += literal;
39+
continue;
40+
}
41+
return undefined;
42+
}
43+
return out;
44+
}
45+
return undefined;
46+
}
47+
48+
function extractLiteral(path) {
49+
if (!path) {
50+
return undefined;
51+
}
52+
if (path.type === 'GlimmerBooleanLiteral') {
53+
return path.value ? 'true' : 'false';
54+
}
55+
if (path.type === 'GlimmerStringLiteral') {
56+
return path.value;
57+
}
58+
if (path.type === 'GlimmerNumberLiteral') {
59+
return String(path.value);
60+
}
61+
return undefined;
62+
}
63+
64+
module.exports = { getStaticAttrValue };

tests/lib/rules/template-no-empty-headings.js

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,8 @@ ruleTester.run('template-no-empty-headings', rule, {
4444
'<template><h2><@heading /></h2></template>',
4545
'<template><h3><ns.Heading /></h3></template>',
4646

47-
// aria-hidden variants are exempt from the empty-heading check to avoid
48-
// false positives when headings are intentionally hidden from assistive tech.
49-
'<template><h1 aria-hidden></h1></template>',
50-
'<template><h1 aria-hidden=""></h1></template>',
47+
// Explicit "true" exempts the empty-heading check — author has
48+
// signalled the heading is intentionally hidden from assistive tech.
5149
'<template><h1 aria-hidden={{true}}></h1></template>',
5250
'<template><h1 aria-hidden="true">Visible to sighted only</h1></template>',
5351
'<template><h1 aria-hidden="TRUE"></h1></template>',
@@ -170,5 +168,35 @@ ruleTester.run('template-no-empty-headings', rule, {
170168
output: null,
171169
errors: [{ messageId: 'emptyHeading' }],
172170
},
171+
// Per WAI-ARIA 1.2 §6.6 aria-hidden value table: valueless /
172+
// empty-string `aria-hidden` resolves to the default `undefined`,
173+
// not `true`. Empty headings with these forms still flag.
174+
{
175+
code: '<template><h1 aria-hidden></h1></template>',
176+
output: null,
177+
errors: [{ messageId: 'emptyHeading' }],
178+
},
179+
{
180+
code: '<template><h1 aria-hidden=""></h1></template>',
181+
output: null,
182+
errors: [{ messageId: 'emptyHeading' }],
183+
},
184+
// Mustache / concat forms that resolve to an empty / whitespace-only
185+
// string — same spec-aligned treatment.
186+
{
187+
code: '<template><h1 aria-hidden={{""}}></h1></template>',
188+
output: null,
189+
errors: [{ messageId: 'emptyHeading' }],
190+
},
191+
{
192+
code: '<template><h1 aria-hidden="{{""}}"></h1></template>',
193+
output: null,
194+
errors: [{ messageId: 'emptyHeading' }],
195+
},
196+
{
197+
code: '<template><h1 aria-hidden={{" "}}></h1></template>',
198+
output: null,
199+
errors: [{ messageId: 'emptyHeading' }],
200+
},
173201
],
174202
});
Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
'use strict';
2+
3+
const { getStaticAttrValue } = require('../../../lib/utils/static-attr-value');
4+
5+
describe('getStaticAttrValue', () => {
6+
it('returns empty string for null/undefined (valueless attribute)', () => {
7+
expect(getStaticAttrValue(null)).toBe('');
8+
expect(getStaticAttrValue(undefined)).toBe('');
9+
});
10+
11+
it('returns chars for GlimmerTextNode', () => {
12+
expect(getStaticAttrValue({ type: 'GlimmerTextNode', chars: 'hello' })).toBe('hello');
13+
expect(getStaticAttrValue({ type: 'GlimmerTextNode', chars: '' })).toBe('');
14+
});
15+
16+
it('unwraps GlimmerMustacheStatement with BooleanLiteral', () => {
17+
expect(
18+
getStaticAttrValue({
19+
type: 'GlimmerMustacheStatement',
20+
path: { type: 'GlimmerBooleanLiteral', value: true },
21+
})
22+
).toBe('true');
23+
expect(
24+
getStaticAttrValue({
25+
type: 'GlimmerMustacheStatement',
26+
path: { type: 'GlimmerBooleanLiteral', value: false },
27+
})
28+
).toBe('false');
29+
});
30+
31+
it('unwraps GlimmerMustacheStatement with StringLiteral', () => {
32+
expect(
33+
getStaticAttrValue({
34+
type: 'GlimmerMustacheStatement',
35+
path: { type: 'GlimmerStringLiteral', value: 'foo' },
36+
})
37+
).toBe('foo');
38+
expect(
39+
getStaticAttrValue({
40+
type: 'GlimmerMustacheStatement',
41+
path: { type: 'GlimmerStringLiteral', value: '' },
42+
})
43+
).toBe('');
44+
});
45+
46+
it('unwraps GlimmerMustacheStatement with NumberLiteral', () => {
47+
expect(
48+
getStaticAttrValue({
49+
type: 'GlimmerMustacheStatement',
50+
path: { type: 'GlimmerNumberLiteral', value: -1 },
51+
})
52+
).toBe('-1');
53+
expect(
54+
getStaticAttrValue({
55+
type: 'GlimmerMustacheStatement',
56+
path: { type: 'GlimmerNumberLiteral', value: 0 },
57+
})
58+
).toBe('0');
59+
});
60+
61+
it('returns undefined for GlimmerMustacheStatement with a dynamic PathExpression', () => {
62+
expect(
63+
getStaticAttrValue({
64+
type: 'GlimmerMustacheStatement',
65+
path: { type: 'GlimmerPathExpression', original: 'this.foo' },
66+
})
67+
).toBeUndefined();
68+
});
69+
70+
it('joins GlimmerConcatStatement with only static parts', () => {
71+
expect(
72+
getStaticAttrValue({
73+
type: 'GlimmerConcatStatement',
74+
parts: [
75+
{ type: 'GlimmerTextNode', chars: 'prefix-' },
76+
{
77+
type: 'GlimmerMustacheStatement',
78+
path: { type: 'GlimmerStringLiteral', value: 'mid' },
79+
},
80+
{ type: 'GlimmerTextNode', chars: '-suffix' },
81+
],
82+
})
83+
).toBe('prefix-mid-suffix');
84+
});
85+
86+
it('joins concat with boolean and number literal parts', () => {
87+
expect(
88+
getStaticAttrValue({
89+
type: 'GlimmerConcatStatement',
90+
parts: [
91+
{
92+
type: 'GlimmerMustacheStatement',
93+
path: { type: 'GlimmerBooleanLiteral', value: true },
94+
},
95+
],
96+
})
97+
).toBe('true');
98+
expect(
99+
getStaticAttrValue({
100+
type: 'GlimmerConcatStatement',
101+
parts: [
102+
{
103+
type: 'GlimmerMustacheStatement',
104+
path: { type: 'GlimmerNumberLiteral', value: -1 },
105+
},
106+
],
107+
})
108+
).toBe('-1');
109+
});
110+
111+
it('returns undefined for GlimmerConcatStatement with a dynamic part', () => {
112+
expect(
113+
getStaticAttrValue({
114+
type: 'GlimmerConcatStatement',
115+
parts: [
116+
{ type: 'GlimmerTextNode', chars: 'x-' },
117+
{
118+
type: 'GlimmerMustacheStatement',
119+
path: { type: 'GlimmerPathExpression', original: 'this.foo' },
120+
},
121+
],
122+
})
123+
).toBeUndefined();
124+
});
125+
126+
it('returns undefined for an unknown node type', () => {
127+
expect(getStaticAttrValue({ type: 'GlimmerSubExpression' })).toBeUndefined();
128+
});
129+
});

0 commit comments

Comments
 (0)