BUGFIX: template-no-empty-headings — recognize boolean aria-hidden#48
Merged
BUGFIX: template-no-empty-headings — recognize boolean aria-hidden#48
Conversation
Before: isHidden only matched aria-hidden="true" as a string literal.
Boolean / valueless / empty / mustache forms (<h1 aria-hidden />,
<h1 aria-hidden="" />, <h1 aria-hidden={{true}} />) slipped past as
"not hidden", so empty headings in those forms were flagged as empty
even when the author had intentionally hidden them from AT.
Fix: extract isAriaHiddenTruthy(). Recognize:
- valueless attribute (HBS AST has value=null or empty-string TextNode)
- "true" string literal (preserved)
- "" empty string
- {{true}} boolean mustache literal
- {{"true"}} string mustache literal
Per HTML boolean-attribute semantics (and jsx-a11y/vue-a11y convention),
presence of aria-hidden without an explicit "false" value is treated as
truthy. The strict ARIA spec treats bare aria-hidden as "undefined"
rather than "true", but every major linter in the ecosystem (and most
screen readers) treats it as true.
Four new test cases covering each of the recognized forms.
…itively HTML attribute value comparison is ASCII case-insensitive per spec, so `aria-hidden="TRUE"` and `aria-hidden="True"` (and their mustache-string equivalents) should be recognised as truthy. Mirrors the same case- handling choice made in ember-cli#2718 for `kind="captions"`. Tests cover `"TRUE"`, `"True"`, `{{"TRUE"}}`, `{{"True"}}`.
Adds invalid tests for `aria-hidden={{false}}` and `aria-hidden={{"false"}}`
to lock down that falsy mustache values do not exempt an otherwise-empty
heading.
…ARIA spec
Per WAI-ARIA 1.2 §6.6, `aria-hidden` has value type true/false/undefined
with default `undefined`. Per §8.5, missing or empty-string attribute
values resolve to the default. So a valueless `aria-hidden` is NOT
hidden per spec — only an explicit `"true"` (ASCII case-insensitive per
HTML enumerated-attribute rules) hides the element.
The earlier direction of this PR borrowed the HTML boolean-attribute
intuition (presence = truthy) from jsx-a11y. That's a peer-plugin
convention, not a spec mandate — aria-hidden is an enumerated ARIA
attribute, not a boolean HTML one. vue-a11y's heading-has-content
doesn't exempt aria-hidden headings at all; lit-a11y has the inverse
rule.
Behaviour now:
- Exempt (hidden): `aria-hidden="true"` / "TRUE" / "True", `{{true}}`,
`{{"true"}}` / case-variants.
- Flag (NOT hidden per spec): valueless `<h1 aria-hidden>`, empty
`<h1 aria-hidden="">`, `{{false}}`, `{{"false"}}`, `"false"`.
…less aria-hidden
The valueless / empty-string aria-hidden case is genuinely contested in
the ecosystem — four positions exist (jsx-a11y / vue-a11y / axe-core /
WAI-ARIA spec), and no single authoritative source is decisive. Rather
than pick one interpretation and live with its false positives, this
rule leans toward fewer-false-positives: any aria-hidden form that could
plausibly mean "hide this" exempts the heading from the empty-content
check.
Truthy (exempt heading):
- valueless `<h1 aria-hidden>` — undefined-default per spec, but
authors who write bare aria-hidden plausibly intend to hide.
- empty `<h1 aria-hidden="">` — same.
- `aria-hidden="true"` (ASCII case-insensitive) — unambiguous.
- `aria-hidden={{true}}` / `{{"true"}}` (case-insensitive) — unambiguous.
Falsy (still flag empty heading):
- `aria-hidden="false"`, `{{false}}`, `{{"false"}}` — explicit opt-out.
This reverses the previous spec-first direction on the valueless/empty
case. Rationale: a linter that flags intentional decorative markup
creates friction and loss of trust; a linter that misses some genuinely-
empty headings is preferable when the signal is ambiguous. The explicit
`aria-hidden="true"` cases, which ARE clearly hidden per spec, remain
exempt.
Move the explanation of valueless / empty-string aria-hidden handling from the PR body into the published rule docs. The rule deviates from WAI-ARIA 1.2 §aria-hidden (which resolves valueless aria-hidden to the default 'undefined', not 'true') in order to favor fewer false positives for this specific check. Also document the 'opposite-direction' split with template-no-aria-hidden-on-focusable / template-anchor-has-content (where spec-literal interpretation applies), and the unambiguous cases that always follow the spec.
🏎️ Benchmark Comparison
Full mitata output |
There was a problem hiding this comment.
Pull request overview
Updates template-no-empty-headings to avoid false positives by treating more aria-hidden “truthy” forms (including valueless/empty and case-insensitive "true", plus mustache boolean/string literals) as hidden, thereby exempting decorative headings from the empty-content check.
Changes:
- Add
isAriaHiddenTruthy()and use it inisHidden()to broaden recognizedaria-hiddentruthy forms. - Extend rule tests with additional valid/invalid
aria-hiddenvariants. - Document the rule’s intentional deviation from spec-literal semantics for valueless/empty
aria-hidden.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tests/lib/rules/template-no-empty-headings.js | Adds coverage for additional aria-hidden variants (truthy exemptions and explicit falsy cases). |
| lib/rules/template-no-empty-headings.js | Implements isAriaHiddenTruthy() and wires it into hidden detection for headings. |
| docs/rules/template-no-empty-headings.md | Explains the rule’s aria-hidden interpretation and its rationale. |
💡 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 3 out of 3 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
`isAriaHiddenTruthy` previously only handled raw TextNode and bare
MustacheStatement attribute values. The quoted-mustache form
`aria-hidden="{{true}}"` produces a `GlimmerConcatStatement` with a
single mustache part — resolve that case by descending into the single
static-literal part, mirroring the pattern established in
template-no-aria-hidden-focusable.
Leans toward "truthy" only on literal true / empty / bare-valueless to
match the rule's doc-stated ethos of fewer false positives.
johanrd
force-pushed
the
fix/heading-accept-boolean-aria-hidden
branch
from
0bcced1 to
a02b3e9
Compare
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
💡 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 3 out of 3 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
… 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.
The rule and test comments cited "WAI-ARIA 1.2 §6.6 aria-hidden value table". §6.6 is "Taxonomy of WAI-ARIA States and Properties" and only categorizes attributes; the aria-hidden value table lives in §6.7 "Definitions of States and Properties". Use the bare #aria-hidden anchor, which is unambiguous.
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.
[Mirror of ember-cli#2717 for Copilot review]
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.This PR is part of a Phase 3 a11y-parity audit against jsx-a11y / vue-a11y / angular-eslint-template / lit-a11y.
aria-hiddenis intentionally invisible to assistive technology — requiring text content in that heading is a false positive.isHiddencheck only matchedaria-hidden="true"as a case-sensitive string literal, so<h1 aria-hidden>,<h1 aria-hidden="">,<h1 aria-hidden="TRUE">,<h1 aria-hidden={{true}}>, etc. were treated as visible and flagged for missing content.Fix: extract
isAriaHiddenTruthy()that recognizes every plausible "hide-this" form (valueless, empty-string,"true"case-insensitive, mustache boolean/string-literal true/case-variants).Prior art
heading-has-content<h1>–<h6>; skips hidden headings viaisHiddenFromAT(jsx-a11y'saria-hiddeninterpretation — see Contested-semantics table below).heading-has-content<h1>–<h6>; usesisHiddenFromScreenReader(vue's(value || "").toString() !== "false"— see Contested-semantics table below).elements-content<h1>–<h6>together with<a>/<button>via a single regex matcher; skips hidden viaisHiddenFromScreenReader. Not a dedicated heading rule.heading-hidden(headings must not be hidden) but no "heading has content" rule. Inverse concern.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 hiddenaria-hiddenvalue table lists exactlytrue / false / undefined (default). A missing attribute resolves to that default; an empty-string value isn't enumerated, so it isn't interpreted astrue.aria-hiddenis also not in HTML's boolean attributes list, so the "presence-implies-true" rule that applies to e.g.disableddoes not extend here.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 — most likely a no-op matching the spec's undefined-default.Design choice for this rule
We lean toward fewer false positives. A linter that flags a heading the author intentionally marked decorative (via bare
aria-hidden) creates friction and loss of trust; a linter that silently accepts some genuinely-empty unhidden headings is the smaller cost when the signal is ambiguous. So any aria-hidden form that could plausibly mean "hide this" exempts the heading from the empty-content check.Exempt (don't flag empty heading):
<h1 aria-hidden>,<h1 aria-hidden="">,<h1 aria-hidden="true">,"TRUE","True"<h1 aria-hidden={{true}}>,{{"true"}},{{"TRUE"}},{{"True"}}Still flag (explicit opt-in to the content check):
<h1 aria-hidden="false">,{{false}},{{"false"}}Tests
Valid (heading exempted):
"true"/"TRUE"/"True",{{true}},{{"true"}}/ case-variantsInvalid (falsy explicitly → flagged):
<h1 aria-hidden="false"></h1>,{{false}},{{"false"}}