Skip to content

fix(template-no-invalid-aria-attributes): absorb allowundefined handling into validateByType#2723

Draft
johanrd wants to merge 2 commits intoember-cli:masterfrom
johanrd:fix/aria-orientation-undefined-token
Draft

fix(template-no-invalid-aria-attributes): absorb allowundefined handling into validateByType#2723
johanrd wants to merge 2 commits intoember-cli:masterfrom
johanrd:fix/aria-orientation-undefined-token

Conversation

@johanrd
Copy link
Copy Markdown
Contributor

@johanrd johanrd commented Apr 21, 2026

This PR is part of a broader a11y parity audit comparing our rules against jsx-a11y, vue-a11y, angular-eslint-template, and lit-a11y.

Premise

Per aria-query 5.3.2, a handful of ARIA attributes accept the literal string "undefined" as a spec-valid value:

attr aria-query type / allowundefined spec basis
aria-orientation token / (unset) with 'undefined' in values list WAI-ARIA 1.2 §aria-orientation: `vertical
aria-expanded boolean / true §aria-expanded: `true
aria-hidden boolean / true §aria-hidden: `true
aria-grabbed boolean / true §aria-grabbed
aria-selected boolean / true §aria-selected

Two orthogonal encodings: token with 'undefined' in values (aria-orientation) and boolean with allowundefined: true (the other four). Both should produce "accept the string 'undefined'"; neither is the user's fault to write.

Problem

The pre-fix isValidAriaValue had a clunky two-layer structure:

if (value === 'undefined' && isTokenWithUndefinedInValues) return true;   // precheck
if (value === 'undefined') return Boolean(attrDef.allowundefined);        // short-circuit
return validateByType(...);

The precheck was a late patch that landed because aria-orientation="undefined" (the token case) was falsely rejected by the short-circuit. The short-circuit itself exists for the 4 boolean-type attrs. So validation was split across three locations for one attribute.

Fix

Absorb allowundefined handling directly into validateByType via a single helper allowsUndefinedLiteral(attrDef, value). The token-values check in the 'token' case naturally handles aria-orientation="undefined" without any special-casing. isValidAriaValue becomes a thin dispatcher to validateByType.

function allowsUndefinedLiteral(attrDef, value) {
  return value === 'undefined' && Boolean(attrDef.allowundefined);
}

function validateByType(attrDef, value) {
  if (allowsUndefinedLiteral(attrDef, value)) return true;
  switch (attrDef.type) {
    // 'token' case accepts 'undefined' naturally via the values list for aria-orientation
    // 'boolean' case rejects string 'undefined' (the allowundefined branch above handles the allowed cases)
    // ...
  }
}

Behavior — unchanged outcomes, cleaner structure

Input Before After
aria-orientation="undefined" ✅ accepted (via precheck) ✅ accepted (via token values list)
aria-expanded="undefined" ✅ accepted (via short-circuit) ✅ accepted (via allowsUndefinedLiteral)
aria-hidden="undefined" ✅ accepted ✅ accepted
aria-grabbed="undefined" ✅ accepted ✅ accepted
aria-selected="undefined" ✅ accepted ✅ accepted
aria-pressed="undefined" ❌ rejected (no allowundefined, no 'undefined' in values) ❌ rejected
aria-orientation="sideways" ❌ rejected ❌ rejected

Prior art — peers don't handle the 4 boolean+allowundefined attrs correctly

Verified each peer in source:

Plugin Rule Behavior on aria-expanded="undefined"
jsx-a11y aria-proptypes validityCheck('boolean', value) is typeof value === 'boolean'; string "undefined" fails. allowUndefined && value === undefined only fires on JS undefined, not the string — and is effectively dead code because jsx-a11y reads attributes.allowUndefined (camelCase) but aria-query emits allowundefined (lowercase). Rejects the string.
lit-a11y aria-attr-valid-value Same structure as jsx-a11y. allowundefined lowercase matches aria-query correctly, but still only triggers on JS undefined. Rejects the string.
@angular-eslint/template valid-aria if (allowundefined && isNil(attributeValue)) return true;allowundefined only triggers on null/undefined, not the string. Rejects the string.
vuejs-accessibility aria-props Does NOT validate token values at all — rule only checks aria.has(name). Trivially accepts the string (by not checking).

So of the four peers, three reject a spec-valid construct, one doesn't check values at all. This PR makes our rule more spec-compliant than the three value-validating peers on these 4 attrs.

Note — case-insensitivity gap (out of scope)

jsx-a11y and lit-a11y lowercase the value before the token check (permittedValues.indexOf(value.toLowerCase()) > -1). Our rule does NOT lowercase; a case-variant like aria-orientation="UNDEFINED" or "Horizontal" would still fail here even after this PR. Same class of HTML-enumerated-attribute case-insensitivity gap as #2718 (kind="captions") and #2728 (role tokens). Worth a follow-up PR; explicitly out of scope for this narrow change.

Tests

  • Valid: aria-orientation="undefined" / "horizontal", aria-expanded="undefined", aria-hidden="undefined", aria-grabbed="undefined", aria-selected="undefined", aria-pressed="true" / "false" / "mixed".
  • Invalid: aria-orientation="sideways", aria-pressed="undefined" (no allowundefined), aria-required="undefined" (boolean without allowundefined — pre-existing test).

…ing into validateByType

Removes the top-level string-"undefined" short-circuit in `isValidAriaValue`
(previously a two-layer dance: a token-type precheck then a boolean-ish
short-circuit). Absorbs the `allowundefined` handling directly into
`validateByType` via a new `allowsUndefinedLiteral(attrDef, value)` helper
that honors aria-query's convention for the 4 boolean-type attrs that
encode "string 'undefined' is spec-valid": aria-expanded, aria-hidden,
aria-grabbed, aria-selected.

## Before/after — same outcome, cleaner structure

| Attribute | aria-query type / allowundefined | `"undefined"` string accepted? |
|---|---|---|
| aria-orientation | token / (unset) with `'undefined'` in values | yes — via token-values check |
| aria-expanded | boolean / true | yes — via allowundefined |
| aria-hidden | boolean / true | yes — via allowundefined |
| aria-grabbed | boolean / true | yes — via allowundefined |
| aria-selected | boolean / true | yes — via allowundefined |
| aria-pressed | tristate / (unset) | NO — no allowundefined flag |
| aria-checked | tristate / (unset) | NO — no allowundefined flag |

All behavior preserved; just structured so that validity decisions
happen in one place.

## Why keep the allowundefined path at all

The 4 boolean-type attrs with `allowundefined: true` have spec-valid
`"undefined"` values per WAI-ARIA 1.2 (e.g. aria-expanded accepts
true/false/undefined). aria-query encodes this. Peers (jsx-a11y,
lit-a11y, angular-eslint) effectively reject `aria-expanded="undefined"`
because their `allowundefined` branch only fires on JS undefined, not
the string. That's a peer bug; our path keeps us spec-compliant.

Tests added for each of the 4 allowundefined attrs; new negative test
for `aria-pressed="undefined"` (tristate without allowundefined) pins
the correct rejection path.
@johanrd johanrd force-pushed the fix/aria-orientation-undefined-token branch from c0e0fcd to bcadc82 Compare April 21, 2026 20:37
@johanrd johanrd changed the title BUGFIX: template-no-invalid-aria-attributes — accept "undefined" as valid aria-orientation token fix(template-no-invalid-aria-attributes): absorb allowundefined handling into validateByType Apr 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant