Merge pull request ember-cli#2717 from johanrd/fix/heading-accept-boolean-aria-hidden

BUGFIX: template-no-empty-headings — recognize boolean aria-hidden

Author: NullVoxPopuli

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

@@ -50,6 +50,14 @@ This rule **allows** the following:
 
 If violations are found, remediation should be planned to ensure text content is present and visible and/or screen-reader accessible. Setting `aria-hidden="false"` or removing `hidden` attributes from the element(s) containing heading text may serve as a quickfix.
 
+## Notes on `aria-hidden` semantics
+
+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 string — all resolve to the default `undefined` and do NOT exempt the heading from the empty-content check.
+
+- `aria-hidden="true"` / `aria-hidden={{true}}` / `aria-hidden={{"true"}}` (any case, whitespace-trimmed) → hidden, exempts the heading.
+- `aria-hidden="false"` / `aria-hidden={{false}}` / `aria-hidden={{"false"}}` → not hidden, the empty-content check applies.
+- `<h1 aria-hidden>` / `aria-hidden=""` / `aria-hidden={{""}}` / `aria-hidden={{" "}}` → spec-default `undefined`, the empty-content check applies.
+
 ## References
 
 - [WCAG SC 2.4.6 Headings and Labels](https://www.w3.org/TR/UNDERSTANDING-WCAG20/navigation-mechanisms-descriptive.html)

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

@@ -1,17 +1,34 @@
+const { getStaticAttrValue } = require('../utils/static-attr-value');
+
 const HEADINGS = new Set(['h1', 'h2', 'h3', 'h4', 'h5', 'h6']);
 
+// Aligned with the WAI-ARIA 1.2 [`aria-hidden`](https://www.w3.org/TR/wai-aria-1.2/#aria-hidden)
+// value table (`true | false | undefined (default)`): treat only an explicit
+// "true" (ASCII case-insensitive, whitespace-trimmed) as hiding the element.
+// Valueless `<h1 aria-hidden>`, empty-string `aria-hidden=""`, and
+// `aria-hidden="false"` all resolve to the default `undefined` / explicit
+// false — so the empty-content check still applies. All shape-unwrapping
+// (mustache/concat) goes through the shared `getStaticAttrValue` helper.
+function isAriaHiddenTruthy(attr) {
+  if (!attr) {
+    return false;
+  }
+  const resolved = getStaticAttrValue(attr.value);
+  if (resolved === undefined) {
+    // Dynamic — can't prove truthy.
+    return false;
+  }
+  return resolved.trim().toLowerCase() === 'true';
+}
+
 function isHidden(node) {
   if (!node.attributes) {
     return false;
   }
   if (node.attributes.some((a) => a.name === 'hidden')) {
     return true;
   }
-  const ariaHidden = node.attributes.find((a) => a.name === 'aria-hidden');
-  if (ariaHidden?.value?.type === 'GlimmerTextNode' && ariaHidden.value.chars === 'true') {
-    return true;
-  }
-  return false;
+  return isAriaHiddenTruthy(node.attributes.find((a) => a.name === 'aria-hidden'));
 }
 
 function isComponent(node) {

lib/utils/static-attr-value.js

@@ -0,0 +1,64 @@
+'use strict';
+
+/**
+ * Return the statically-known string value of a Glimmer attribute value node,
+ * or `undefined` when the value is dynamic (cannot be resolved at lint time).
+ *
+ * Unwraps:
+ *   - GlimmerTextNode → chars
+ *   - GlimmerMustacheStatement with a literal path (boolean/string/number) → stringified value
+ *   - GlimmerConcatStatement whose parts are all statically resolvable → joined string
+ *
+ * A missing/undefined value (valueless attribute, e.g. `<input disabled>`)
+ * returns the empty string. Pass `attr.value` — not the attribute itself.
+ */
+function getStaticAttrValue(value) {
+  if (value === null || value === undefined) {
+    return '';
+  }
+  if (value.type === 'GlimmerTextNode') {
+    return value.chars;
+  }
+  if (value.type === 'GlimmerMustacheStatement') {
+    return extractLiteral(value.path);
+  }
+  if (value.type === 'GlimmerConcatStatement') {
+    const parts = value.parts || [];
+    let out = '';
+    for (const part of parts) {
+      if (part.type === 'GlimmerTextNode') {
+        out += part.chars;
+        continue;
+      }
+      if (part.type === 'GlimmerMustacheStatement') {
+        const literal = extractLiteral(part.path);
+        if (literal === undefined) {
+          return undefined;
+        }
+        out += literal;
+        continue;
+      }
+      return undefined;
+    }
+    return out;
+  }
+  return undefined;
+}
+
+function extractLiteral(path) {
+  if (!path) {
+    return undefined;
+  }
+  if (path.type === 'GlimmerBooleanLiteral') {
+    return path.value ? 'true' : 'false';
+  }
+  if (path.type === 'GlimmerStringLiteral') {
+    return path.value;
+  }
+  if (path.type === 'GlimmerNumberLiteral') {
+    return String(path.value);
+  }
+  return undefined;
+}
+
+module.exports = { getStaticAttrValue };

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

@@ -43,6 +43,24 @@ ruleTester.run('template-no-empty-headings', rule, {
     '<template><h1><this.Heading /></h1></template>',
     '<template><h2><@heading /></h2></template>',
     '<template><h3><ns.Heading /></h3></template>',
+
+    // Explicit "true" exempts the empty-heading check — author has
+    // signalled the heading is intentionally hidden from assistive tech.
+    '<template><h1 aria-hidden={{true}}></h1></template>',
+    '<template><h1 aria-hidden="true">Visible to sighted only</h1></template>',
+    '<template><h1 aria-hidden="TRUE"></h1></template>',
+    '<template><h1 aria-hidden="True"></h1></template>',
+    '<template><h1 aria-hidden={{"TRUE"}}></h1></template>',
+    '<template><h1 aria-hidden={{"True"}}></h1></template>',
+    // Quoted-mustache (GlimmerConcatStatement) forms — `aria-hidden="{{true}}"`
+    // resolves the same as `aria-hidden={{true}}`. Pin these so future
+    // refactors don't regress concat handling.
+    '<template><h1 aria-hidden="{{true}}"></h1></template>',
+    '<template><h1 aria-hidden="{{"true"}}"></h1></template>',
+    // Whitespace normalization — incidental surrounding whitespace should
+    // still resolve to "true".
+    '<template><h1 aria-hidden={{" true "}}></h1></template>',
+    '<template><h1 aria-hidden=" true "></h1></template>',
   ],
   invalid: [
     {
@@ -132,5 +150,54 @@ ruleTester.run('template-no-empty-headings', rule, {
       output: null,
       errors: [{ messageId: 'emptyHeading' }],
     },
+
+    // Explicit falsy aria-hidden does NOT exempt the empty-heading check —
+    // this is the unambiguous opt-out, no ecosystem position disagrees.
+    {
+      code: '<template><h1 aria-hidden="false"></h1></template>',
+      output: null,
+      errors: [{ messageId: 'emptyHeading' }],
+    },
+    {
+      code: '<template><h1 aria-hidden={{false}}></h1></template>',
+      output: null,
+      errors: [{ messageId: 'emptyHeading' }],
+    },
+    {
+      code: '<template><h1 aria-hidden={{"false"}}></h1></template>',
+      output: null,
+      errors: [{ messageId: 'emptyHeading' }],
+    },
+    // Per the WAI-ARIA 1.2 `aria-hidden` value table
+    // (https://www.w3.org/TR/wai-aria-1.2/#aria-hidden): valueless /
+    // empty-string `aria-hidden` resolves to the default `undefined`,
+    // not `true`. Empty headings with these forms still flag.
+    {
+      code: '<template><h1 aria-hidden></h1></template>',
+      output: null,
+      errors: [{ messageId: 'emptyHeading' }],
+    },
+    {
+      code: '<template><h1 aria-hidden=""></h1></template>',
+      output: null,
+      errors: [{ messageId: 'emptyHeading' }],
+    },
+    // Mustache / concat forms that resolve to an empty / whitespace-only
+    // string — same spec-aligned treatment.
+    {
+      code: '<template><h1 aria-hidden={{""}}></h1></template>',
+      output: null,
+      errors: [{ messageId: 'emptyHeading' }],
+    },
+    {
+      code: '<template><h1 aria-hidden="{{""}}"></h1></template>',
+      output: null,
+      errors: [{ messageId: 'emptyHeading' }],
+    },
+    {
+      code: '<template><h1 aria-hidden={{" "}}></h1></template>',
+      output: null,
+      errors: [{ messageId: 'emptyHeading' }],
+    },
   ],
 });

tests/lib/utils/static-attr-value-test.js

@@ -0,0 +1,129 @@
+'use strict';
+
+const { getStaticAttrValue } = require('../../../lib/utils/static-attr-value');
+
+describe('getStaticAttrValue', () => {
+  it('returns empty string for null/undefined (valueless attribute)', () => {
+    expect(getStaticAttrValue(null)).toBe('');
+    expect(getStaticAttrValue(undefined)).toBe('');
+  });
+
+  it('returns chars for GlimmerTextNode', () => {
+    expect(getStaticAttrValue({ type: 'GlimmerTextNode', chars: 'hello' })).toBe('hello');
+    expect(getStaticAttrValue({ type: 'GlimmerTextNode', chars: '' })).toBe('');
+  });
+
+  it('unwraps GlimmerMustacheStatement with BooleanLiteral', () => {
+    expect(
+      getStaticAttrValue({
+        type: 'GlimmerMustacheStatement',
+        path: { type: 'GlimmerBooleanLiteral', value: true },
+      })
+    ).toBe('true');
+    expect(
+      getStaticAttrValue({
+        type: 'GlimmerMustacheStatement',
+        path: { type: 'GlimmerBooleanLiteral', value: false },
+      })
+    ).toBe('false');
+  });
+
+  it('unwraps GlimmerMustacheStatement with StringLiteral', () => {
+    expect(
+      getStaticAttrValue({
+        type: 'GlimmerMustacheStatement',
+        path: { type: 'GlimmerStringLiteral', value: 'foo' },
+      })
+    ).toBe('foo');
+    expect(
+      getStaticAttrValue({
+        type: 'GlimmerMustacheStatement',
+        path: { type: 'GlimmerStringLiteral', value: '' },
+      })
+    ).toBe('');
+  });
+
+  it('unwraps GlimmerMustacheStatement with NumberLiteral', () => {
+    expect(
+      getStaticAttrValue({
+        type: 'GlimmerMustacheStatement',
+        path: { type: 'GlimmerNumberLiteral', value: -1 },
+      })
+    ).toBe('-1');
+    expect(
+      getStaticAttrValue({
+        type: 'GlimmerMustacheStatement',
+        path: { type: 'GlimmerNumberLiteral', value: 0 },
+      })
+    ).toBe('0');
+  });
+
+  it('returns undefined for GlimmerMustacheStatement with a dynamic PathExpression', () => {
+    expect(
+      getStaticAttrValue({
+        type: 'GlimmerMustacheStatement',
+        path: { type: 'GlimmerPathExpression', original: 'this.foo' },
+      })
+    ).toBeUndefined();
+  });
+
+  it('joins GlimmerConcatStatement with only static parts', () => {
+    expect(
+      getStaticAttrValue({
+        type: 'GlimmerConcatStatement',
+        parts: [
+          { type: 'GlimmerTextNode', chars: 'prefix-' },
+          {
+            type: 'GlimmerMustacheStatement',
+            path: { type: 'GlimmerStringLiteral', value: 'mid' },
+          },
+          { type: 'GlimmerTextNode', chars: '-suffix' },
+        ],
+      })
+    ).toBe('prefix-mid-suffix');
+  });
+
+  it('joins concat with boolean and number literal parts', () => {
+    expect(
+      getStaticAttrValue({
+        type: 'GlimmerConcatStatement',
+        parts: [
+          {
+            type: 'GlimmerMustacheStatement',
+            path: { type: 'GlimmerBooleanLiteral', value: true },
+          },
+        ],
+      })
+    ).toBe('true');
+    expect(
+      getStaticAttrValue({
+        type: 'GlimmerConcatStatement',
+        parts: [
+          {
+            type: 'GlimmerMustacheStatement',
+            path: { type: 'GlimmerNumberLiteral', value: -1 },
+          },
+        ],
+      })
+    ).toBe('-1');
+  });
+
+  it('returns undefined for GlimmerConcatStatement with a dynamic part', () => {
+    expect(
+      getStaticAttrValue({
+        type: 'GlimmerConcatStatement',
+        parts: [
+          { type: 'GlimmerTextNode', chars: 'x-' },
+          {
+            type: 'GlimmerMustacheStatement',
+            path: { type: 'GlimmerPathExpression', original: 'this.foo' },
+          },
+        ],
+      })
+    ).toBeUndefined();
+  });
+
+  it('returns undefined for an unknown node type', () => {
+    expect(getStaticAttrValue({ type: 'GlimmerSubExpression' })).toBeUndefined();
+  });
+});