Skip to content

Commit 6bcfd5a

Browse files
committed
refactor: adopt isComponentInvocation + isNativeInteractive utils
Replace the rule's inline INHERENTLY_FOCUSABLE_TAGS set and ad-hoc tag checks with the two shared utils: - lib/utils/is-component-invocation.js (from PR #31) - lib/utils/native-interactive-elements.js (from PR #37) Both files (and their tests) are copied bit-identically from their source branches so parity is preserved while those PRs remain open. Behavior deltas introduced by the util swap ------------------------------------------- The prior inline set was {button, details, embed, iframe, input, select, summary, textarea}. The shared util covers the same set plus several additional native-interactive tags that were previously false negatives: - option, datalist, object, canvas — now recognized as native-interactive - area[href] — now recognized (symmetric with a[href]) - audio[controls], video[controls] — now recognized (per HTML-AAM / browser reality; keyboard-operable transport controls) Net effect: `role="presentation"` / `role="none"` on any of the above is now flagged where it wasn't before. All of these are spec-correct FN fixes (WAI-ARIA 1.2 §4.6 conflict resolution applies the same way once the element is acknowledged as focusable). Tests added for representative new cases: - <video controls role="presentation"> — flags (gts + hbs) - <audio controls role="none"> — flags (gts) - <area href="/x" role="presentation"> — flags (gts) - <video role="presentation"> (no controls) — valid (still not focusable) No deltas for <label>: it was not in the prior INHERENTLY_FOCUSABLE_TAGS set and it is not in the shared util either, so behavior is unchanged. Component-invocation handling is now an explicit early-return via isComponentInvocation(node), which also excludes named-arg (<@slot>), this-path (<this.widget>), and dot-path (<foo.bar>) invocations that were previously only excluded incidentally by the tag-lowercase lookup.
1 parent 94f153e commit 6bcfd5a

6 files changed

Lines changed: 396 additions & 30 deletions

lib/rules/template-no-role-presentation-on-focusable.js

Lines changed: 13 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -13,22 +13,17 @@
1313
// recurses into descendants. Vue's recursion is uncommented in their source and
1414
// appears to be a copy-paste from their aria-hidden rule.
1515

16-
const INHERENTLY_FOCUSABLE_TAGS = new Set([
17-
'button',
18-
'details',
19-
'embed',
20-
'iframe',
21-
'input',
22-
'select',
23-
'summary',
24-
'textarea',
25-
]);
16+
'use strict';
17+
18+
const { isComponentInvocation } = require('../utils/is-component-invocation');
19+
const { isNativeInteractive } = require('../utils/native-interactive-elements');
2620

2721
function findAttr(node, name) {
2822
return node.attributes?.find((a) => a.name === name);
2923
}
3024

31-
function getTextAttrValue(attr) {
25+
function getTextAttrValue(node, name) {
26+
const attr = findAttr(node, name);
3227
if (attr?.value?.type === 'GlimmerTextNode') {
3328
return attr.value.chars;
3429
}
@@ -48,27 +43,13 @@ function hasPresentationRole(node) {
4843
}
4944

5045
function isFocusable(node) {
51-
const tag = node.tag?.toLowerCase();
52-
if (!tag) {
53-
return false;
54-
}
55-
46+
// tabindex makes any element focusable (including tabindex="-1" — still
47+
// programmatically focusable; see audit notes for the divergence from
48+
// vue-a11y's treatment of tabindex="-1").
5649
if (findAttr(node, 'tabindex')) {
5750
return true;
5851
}
59-
if (INHERENTLY_FOCUSABLE_TAGS.has(tag)) {
60-
if (tag === 'input') {
61-
const type = getTextAttrValue(findAttr(node, 'type'));
62-
if (type === 'hidden') {
63-
return false;
64-
}
65-
}
66-
return true;
67-
}
68-
if (tag === 'a' && findAttr(node, 'href')) {
69-
return true;
70-
}
71-
return false;
52+
return isNativeInteractive(node, getTextAttrValue);
7253
}
7354

7455
/** @type {import('eslint').Rule.RuleModule} */
@@ -92,6 +73,9 @@ module.exports = {
9273
create(context) {
9374
return {
9475
GlimmerElementNode(node) {
76+
if (isComponentInvocation(node)) {
77+
return;
78+
}
9579
if (!hasPresentationRole(node)) {
9680
return;
9781
}
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+
};
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-role-presentation-on-focusable.js

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,15 @@ ruleTester.run('template-no-role-presentation-on-focusable', rule, {
2626
// <a> without href isn't focusable.
2727
'<template><a role="presentation">Not a link</a></template>',
2828

29-
// Components — rule skips.
29+
// <audio>/<video> without `controls` aren't focusable — no keyboard UI.
30+
'<template><video role="presentation"></video></template>',
31+
'<template><audio role="presentation"></audio></template>',
32+
33+
// Components — rule skips (isComponentInvocation).
3034
'<template><CustomBtn role="presentation" /></template>',
35+
'<template><@slot role="presentation" /></template>',
36+
'<template><this.widget role="presentation" /></template>',
37+
'<template><foo.bar role="presentation" /></template>',
3138

3239
// No role at all.
3340
'<template><button></button></template>',
@@ -64,6 +71,26 @@ ruleTester.run('template-no-role-presentation-on-focusable', rule, {
6471
output: null,
6572
errors: [{ messageId: 'invalidPresentation' }],
6673
},
74+
// <video controls> / <audio controls> — focusable per HTML-AAM / browser
75+
// reality (keyboard-operable transport controls), so role="presentation"
76+
// on them is a semantic conflict. Added after migrating to the shared
77+
// isNativeInteractive util (previously not flagged).
78+
{
79+
code: '<template><video controls role="presentation"></video></template>',
80+
output: null,
81+
errors: [{ messageId: 'invalidPresentation' }],
82+
},
83+
{
84+
code: '<template><audio controls role="none"></audio></template>',
85+
output: null,
86+
errors: [{ messageId: 'invalidPresentation' }],
87+
},
88+
// <area href> — same conditional-interactive rule as <a href>.
89+
{
90+
code: '<template><area href="/x" role="presentation" /></template>',
91+
output: null,
92+
errors: [{ messageId: 'invalidPresentation' }],
93+
},
6794
],
6895
});
6996

@@ -77,6 +104,8 @@ hbsRuleTester.run('template-no-role-presentation-on-focusable', rule, {
77104
'<div role="presentation"></div>',
78105
'<input type="hidden" role="presentation" />',
79106
'<CustomBtn role="presentation" />',
107+
// <video> / <audio> without controls aren't focusable.
108+
'<video role="presentation"></video>',
80109
],
81110
invalid: [
82111
{
@@ -89,5 +118,10 @@ hbsRuleTester.run('template-no-role-presentation-on-focusable', rule, {
89118
output: null,
90119
errors: [{ messageId: 'invalidPresentation' }],
91120
},
121+
{
122+
code: '<video controls role="presentation"></video>',
123+
output: null,
124+
errors: [{ messageId: 'invalidPresentation' }],
125+
},
92126
],
93127
});
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)