Skip to content

Commit 930ff88

Browse files
johanrdjohanrd
committed
refactor: adopt isComponentInvocation + isNativeInteractive utils
Migrate template-no-aria-hidden-on-focusable to the shared utility helpers introduced by PR #31 (isComponentInvocation) and PR #37 (isNativeInteractive): - isFocusable() now delegates the native-focusable-tag check to isNativeInteractive(node, getTextAttrValue). The local INHERENTLY_FOCUSABLE_TAGS set and the inline a[href] branch are removed. - hasFocusableDescendant()'s opaque-tag skip (added in G5.1) now uses isComponentInvocation(child) in place of the inline isOpaqueTag predicate; the local isOpaqueTag helper is removed. Behavior delta (spec-correct FN fix): - Previously <video controls> and <audio controls> were absent from the local INHERENTLY_FOCUSABLE_TAGS, so <div aria-hidden="true"><video controls></video></div> was VALID. - isNativeInteractive returns true for audio[controls] / video[controls] (browsers only render focusable media UI when controls is present). Such patterns are now FLAGGED under noAriaHiddenOnAncestorOfFocusable, and the element directly (<video controls aria-hidden="true">) is FLAGGED under noAriaHiddenOnFocusable. - Audio/video without controls remain VALID (no native focusable UI). Tests: new invalid cases for audio/video with controls directly aria-hidden and as descendants of aria-hidden wrappers, in both gts and hbs suites. New valid cases for audio/video without controls to pin the conditional behavior.
1 parent b8c282a commit 930ff88

6 files changed

Lines changed: 402 additions & 51 deletions

‎lib/rules/template-no-aria-hidden-on-focusable.js‎

Lines changed: 12 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,14 @@
1-
// Native interactive elements that are focusable by default.
2-
const INHERENTLY_FOCUSABLE_TAGS = new Set([
3-
'button',
4-
'details',
5-
'embed',
6-
'iframe',
7-
'input',
8-
'select',
9-
'summary',
10-
'textarea',
11-
]);
1+
'use strict';
2+
3+
const { isComponentInvocation } = require('../utils/is-component-invocation');
4+
const { isNativeInteractive } = require('../utils/native-interactive-elements');
125

136
function findAttr(node, name) {
147
return node.attributes?.find((a) => a.name === name);
158
}
169

17-
function getTextAttrValue(attr) {
10+
function getTextAttrValue(node, name) {
11+
const attr = findAttr(node, name);
1812
if (attr?.value?.type === 'GlimmerTextNode') {
1913
return attr.value.chars;
2014
}
@@ -61,42 +55,11 @@ function isFocusable(node) {
6155
return true;
6256
}
6357

64-
if (INHERENTLY_FOCUSABLE_TAGS.has(tag)) {
65-
// <input type="hidden"> is not focusable.
66-
if (tag === 'input') {
67-
const type = getTextAttrValue(findAttr(node, 'type'));
68-
if (type === 'hidden') {
69-
return false;
70-
}
71-
}
72-
return true;
73-
}
74-
75-
if (tag === 'a' && findAttr(node, 'href')) {
76-
return true;
77-
}
78-
79-
return false;
80-
}
81-
82-
// A tag name is "opaque" if we cannot statically know what element it renders.
83-
// This covers component invocations (PascalCase), argument/this/path-based
84-
// dynamic tags, and namespace-pathed tags. Per WAI-ARIA 1.2 §aria-hidden, we
85-
// conservatively do not descend into these branches (bias toward no-FP).
86-
function isOpaqueTag(tag) {
87-
if (!tag) {
88-
return true;
89-
}
90-
if (/^[A-Z]/.test(tag)) {
91-
return true;
92-
}
93-
if (tag.startsWith('@') || tag.startsWith('this.')) {
94-
return true;
95-
}
96-
if (tag.includes('.') || tag.includes('::')) {
97-
return true;
98-
}
99-
return false;
58+
// Delegate native-focusable classification to the shared util. It handles
59+
// button/select/textarea/iframe/embed/summary/details/option/datalist/
60+
// object/canvas, input (non-hidden), a[href]/area[href], and
61+
// audio[controls]/video[controls].
62+
return isNativeInteractive(node, getTextAttrValue);
10063
}
10164

10265
// Per WAI-ARIA 1.2 §aria-hidden: "Authors SHOULD NOT use aria-hidden='true' on
@@ -115,7 +78,7 @@ function hasFocusableDescendant(node) {
11578
// expressions, and anything else whose rendered element we can't inspect.
11679
continue;
11780
}
118-
if (isOpaqueTag(child.tag)) {
81+
if (isComponentInvocation(child)) {
11982
// Component / dynamic tag — opaque. Don't recurse.
12083
continue;
12184
}

‎lib/utils/is-component-invocation.js‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
'use strict';
2+
3+
/**
4+
* Returns true if the Glimmer element node is a component invocation
5+
* rather than a native HTML element. Excludes:
6+
* - PascalCase tags (<Button>, <MyWidget>)
7+
* - Named-arg invocations (<@heading>, <@tag.foo>)
8+
* - This-path invocations (<this.myComponent>, <this.comp.sub>)
9+
* - Dot-path invocations (<foo.bar>)
10+
* - Named-block syntax (<foo::bar>)
11+
*/
12+
module.exports.isComponentInvocation = function isComponentInvocation(node) {
13+
const tag = node?.tag;
14+
if (typeof tag !== 'string') {
15+
return false;
16+
}
17+
return (
18+
/^[A-Z]/.test(tag) ||
19+
tag.startsWith('@') ||
20+
tag.startsWith('this.') ||
21+
tag.includes('.') ||
22+
tag.includes('::')
23+
);
24+
};

‎lib/utils/native-interactive-elements.js‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
'use strict';
2+
3+
/**
4+
* Native-interactive HTML element classification, shared across rules that need to
5+
* ask "does this HTML tag natively expose interactive UI to keyboard / AT users?".
6+
*
7+
* The set is hand-curated rather than derived from a single authority because
8+
* aria-query, axobject-query, HTML-AAM, WAI-ARIA, and browser reality disagree on
9+
* several rows. Decision rationale is documented per-tag:
10+
*
11+
* | Element | Behavior | Rationale |
12+
* |-------------------------------------------------|----------------------|-----------|
13+
* | button, select, textarea, iframe, embed, | Interactive | aria-query/axobject-query widget + universally-accepted |
14+
* | summary, details | | |
15+
* | input (except type=hidden) | Interactive | Same as above, minus hidden |
16+
* | option, datalist | Interactive | aria-query roles option/listbox; axobject widget; HTML-AAM |
17+
* | a[href], area[href] | Interactive (cond.) | HTML-AAM: anchor interactivity requires href |
18+
* | audio[controls], video[controls] | Interactive | Browsers only render focusable UI with `controls` |
19+
* | audio, video (no controls) | NOT interactive | No keyboard semantics without controls; browsers agree |
20+
* | object | Interactive | axobject-query EmbeddedObjectRole |
21+
* | canvas | Interactive | axobject-query CanvasRole widget; bias toward no-FP |
22+
* | input[type=hidden] | NOT interactive | HTML spec: no UI, no focus, no AT exposure |
23+
* | menuitem | NOT interactive | Deprecated; no longer rendered in Chrome/Edge/Safari/FF |
24+
* | label | NOT interactive | axobject-query LabelRole is structure, not widget |
25+
*/
26+
27+
// Unconditionally-interactive HTML tags (no attribute dependencies).
28+
const UNCONDITIONAL_INTERACTIVE_TAGS = new Set([
29+
'button',
30+
'select',
31+
'textarea',
32+
'iframe',
33+
'embed',
34+
'summary',
35+
'details',
36+
'option',
37+
'datalist',
38+
'object',
39+
'canvas',
40+
]);
41+
42+
/**
43+
* Determine whether a Glimmer element node represents a natively-interactive
44+
* HTML element.
45+
*
46+
* @param {object} node Glimmer `ElementNode` (has a string `tag`).
47+
* @param {Function} getTextAttrValue Helper (node, attrName) -> string | undefined
48+
* that returns the text value of a static
49+
* attribute, or undefined for dynamic / missing.
50+
* @returns {boolean} True if the element is natively interactive.
51+
*/
52+
function isNativeInteractive(node, getTextAttrValue) {
53+
const rawTag = node && node.tag;
54+
if (typeof rawTag !== 'string' || rawTag.length === 0) {
55+
return false;
56+
}
57+
const tag = rawTag.toLowerCase();
58+
59+
// Unconditional interactive tags.
60+
if (UNCONDITIONAL_INTERACTIVE_TAGS.has(tag)) {
61+
return true;
62+
}
63+
64+
// <input> is interactive unless type="hidden" (HTML spec: no UI/focus/AT exposure).
65+
if (tag === 'input') {
66+
const type = getTextAttrValue(node, 'type');
67+
return type !== 'hidden';
68+
}
69+
70+
// <a> and <area> are interactive only when an href is present (HTML-AAM).
71+
if (tag === 'a' || tag === 'area') {
72+
return hasAttribute(node, 'href');
73+
}
74+
75+
// <audio>/<video> are only interactive when `controls` is present.
76+
if (tag === 'audio' || tag === 'video') {
77+
return hasAttribute(node, 'controls');
78+
}
79+
80+
return false;
81+
}
82+
83+
function hasAttribute(node, name) {
84+
return Boolean(node.attributes && node.attributes.some((a) => a.name === name));
85+
}
86+
87+
module.exports = {
88+
isNativeInteractive,
89+
};

‎tests/lib/rules/template-no-aria-hidden-on-focusable.js‎

Lines changed: 42 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,12 @@ ruleTester.run('template-no-aria-hidden-on-focusable', rule, {
3232
// Components — we don't know if they render a focusable element.
3333
'<template><CustomBtn aria-hidden="true" /></template>',
3434

35+
// <audio> / <video> without `controls` are not interactive — no focusable UI.
36+
'<template><video aria-hidden="true"></video></template>',
37+
'<template><audio aria-hidden="true"></audio></template>',
38+
'<template><div aria-hidden="true"><video></video></div></template>',
39+
'<template><div aria-hidden="true"><audio></audio></div></template>',
40+
3541
// Descendant-focusable check — valid cases.
3642
// No focusable descendant.
3743
'<template><div aria-hidden="true"><span>Just text</span></div></template>',
@@ -133,8 +139,6 @@ ruleTester.run('template-no-aria-hidden-on-focusable', rule, {
133139
},
134140
{
135141
// Depth check — focusable descendant two levels deep.
136-
// (Our isFocusable doesn't treat <video controls> as focusable because
137-
// it's not in INHERENTLY_FOCUSABLE_TAGS; use <textarea> instead.)
138142
code: '<template><section aria-hidden="true"><div><textarea></textarea></div></section></template>',
139143
output: null,
140144
errors: [{ messageId: 'noAriaHiddenOnAncestorOfFocusable' }],
@@ -145,6 +149,30 @@ ruleTester.run('template-no-aria-hidden-on-focusable', rule, {
145149
output: null,
146150
errors: [{ messageId: 'noAriaHiddenOnAncestorOfFocusable' }],
147151
},
152+
153+
// <audio controls> / <video controls> expose focusable UI; flag directly.
154+
{
155+
code: '<template><video controls aria-hidden="true"></video></template>',
156+
output: null,
157+
errors: [{ messageId: 'noAriaHiddenOnFocusable' }],
158+
},
159+
{
160+
code: '<template><audio controls aria-hidden="true"></audio></template>',
161+
output: null,
162+
errors: [{ messageId: 'noAriaHiddenOnFocusable' }],
163+
},
164+
// <audio controls> / <video controls> as focusable descendants of an
165+
// aria-hidden ancestor.
166+
{
167+
code: '<template><div aria-hidden="true"><video controls></video></div></template>',
168+
output: null,
169+
errors: [{ messageId: 'noAriaHiddenOnAncestorOfFocusable' }],
170+
},
171+
{
172+
code: '<template><div aria-hidden="true"><audio controls></audio></div></template>',
173+
output: null,
174+
errors: [{ messageId: 'noAriaHiddenOnAncestorOfFocusable' }],
175+
},
148176
],
149177
});
150178

@@ -186,5 +214,17 @@ hbsRuleTester.run('template-no-aria-hidden-on-focusable', rule, {
186214
output: null,
187215
errors: [{ messageId: 'noAriaHiddenOnAncestorOfFocusable' }],
188216
},
217+
// <audio controls> / <video controls> — directly focusable.
218+
{
219+
code: '<video controls aria-hidden="true"></video>',
220+
output: null,
221+
errors: [{ messageId: 'noAriaHiddenOnFocusable' }],
222+
},
223+
// <audio controls> / <video controls> — focusable descendant.
224+
{
225+
code: '<div aria-hidden="true"><audio controls></audio></div>',
226+
output: null,
227+
errors: [{ messageId: 'noAriaHiddenOnAncestorOfFocusable' }],
228+
},
189229
],
190230
});

‎tests/lib/utils/is-component-invocation-test.js‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
'use strict';
2+
3+
const { isComponentInvocation } = require('../../../lib/utils/is-component-invocation');
4+
5+
describe('isComponentInvocation', () => {
6+
it('returns true for PascalCase tags', () => {
7+
expect(isComponentInvocation({ tag: 'Button' })).toBe(true);
8+
expect(isComponentInvocation({ tag: 'MyWidget' })).toBe(true);
9+
// PascalCase tags that match a native HTML element name — the core bug case
10+
expect(isComponentInvocation({ tag: 'Article' })).toBe(true);
11+
expect(isComponentInvocation({ tag: 'Form' })).toBe(true);
12+
expect(isComponentInvocation({ tag: 'Main' })).toBe(true);
13+
expect(isComponentInvocation({ tag: 'Nav' })).toBe(true);
14+
expect(isComponentInvocation({ tag: 'Ul' })).toBe(true);
15+
expect(isComponentInvocation({ tag: 'Li' })).toBe(true);
16+
expect(isComponentInvocation({ tag: 'Aside' })).toBe(true);
17+
expect(isComponentInvocation({ tag: 'Section' })).toBe(true);
18+
expect(isComponentInvocation({ tag: 'Table' })).toBe(true);
19+
});
20+
21+
it('returns false for lowercase native HTML tags', () => {
22+
expect(isComponentInvocation({ tag: 'div' })).toBe(false);
23+
expect(isComponentInvocation({ tag: 'article' })).toBe(false);
24+
expect(isComponentInvocation({ tag: 'form' })).toBe(false);
25+
expect(isComponentInvocation({ tag: 'h1' })).toBe(false);
26+
expect(isComponentInvocation({ tag: 'button' })).toBe(false);
27+
});
28+
29+
it('returns true for named-arg invocations', () => {
30+
expect(isComponentInvocation({ tag: '@heading' })).toBe(true);
31+
expect(isComponentInvocation({ tag: '@tag.foo' })).toBe(true);
32+
});
33+
34+
it('returns true for this-path invocations', () => {
35+
expect(isComponentInvocation({ tag: 'this.myComponent' })).toBe(true);
36+
expect(isComponentInvocation({ tag: 'this.comp.sub' })).toBe(true);
37+
});
38+
39+
it('returns true for dot-path invocations', () => {
40+
expect(isComponentInvocation({ tag: 'foo.bar' })).toBe(true);
41+
expect(isComponentInvocation({ tag: 'ns.widget' })).toBe(true);
42+
});
43+
44+
it('returns true for named-block / namespaced invocations', () => {
45+
expect(isComponentInvocation({ tag: 'foo::bar' })).toBe(true);
46+
expect(isComponentInvocation({ tag: 'Foo::Bar' })).toBe(true);
47+
});
48+
49+
it('returns false for empty-string tag', () => {
50+
expect(isComponentInvocation({ tag: '' })).toBe(false);
51+
});
52+
53+
it('returns false for undefined node', () => {
54+
expect(isComponentInvocation()).toBe(false);
55+
expect(isComponentInvocation(undefined)).toBe(false);
56+
expect(isComponentInvocation(null)).toBe(false);
57+
});
58+
59+
it('returns false for node with undefined tag', () => {
60+
expect(isComponentInvocation({})).toBe(false);
61+
expect(isComponentInvocation({ tag: undefined })).toBe(false);
62+
});
63+
64+
it('returns false for node with non-string tag', () => {
65+
expect(isComponentInvocation({ tag: 123 })).toBe(false);
66+
expect(isComponentInvocation({ tag: null })).toBe(false);
67+
});
68+
});

0 commit comments

Comments
 (0)