Skip to content

Commit 4d28308

Browse files
johanrdjohanrd
committed
refactor: adopt isComponentInvocation + isNativeInteractive utils
Fixes audit B8 PascalCase-component FP.
1 parent 0e9acf8 commit 4d28308

7 files changed

Lines changed: 400 additions & 81 deletions

File tree

‎lib/rules/template-no-noninteractive-tabindex.js‎

Lines changed: 14 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,6 @@
11
const { dom, roles } = require('aria-query');
2-
3-
// Native elements with default interactive semantics — tabindex here is fine.
4-
const INHERENTLY_INTERACTIVE_TAGS = new Set([
5-
'button',
6-
'details',
7-
'embed',
8-
'iframe',
9-
'input',
10-
'select',
11-
'summary',
12-
'textarea',
13-
]);
2+
const { isComponentInvocation } = require('../utils/is-component-invocation');
3+
const { isNativeInteractive } = require('../utils/native-interactive-elements');
144

155
// Interactive ARIA roles (widget/command/composite/input/range subtypes) —
166
// tabindex is required for widget keyboard access, so allow it when present.
@@ -68,41 +58,14 @@ function getStaticTabindexValue(attr) {
6858
return undefined;
6959
}
7060

71-
function getTextAttrValue(attr) {
61+
function getTextAttrValue(node, name) {
62+
const attr = findAttr(node, name);
7263
if (attr?.value?.type === 'GlimmerTextNode') {
7364
return attr.value.chars;
7465
}
7566
return undefined;
7667
}
7768

78-
function isInteractiveElement(node) {
79-
const tag = node.tag?.toLowerCase();
80-
if (INHERENTLY_INTERACTIVE_TAGS.has(tag)) {
81-
if (tag === 'input') {
82-
const type = getTextAttrValue(findAttr(node, 'type'));
83-
if (type === 'hidden') {
84-
return false;
85-
}
86-
}
87-
return true;
88-
}
89-
if (tag === 'a' && findAttr(node, 'href')) {
90-
return true;
91-
}
92-
// <audio>/<video> expose interactive UI only when `controls` is present.
93-
// Matches template-no-invalid-interactive.
94-
if ((tag === 'audio' || tag === 'video') && findAttr(node, 'controls')) {
95-
return true;
96-
}
97-
// <object> maps to an embedded-widget role (axobject-query treats embedded
98-
// content as a widget); authors legitimately put tabindex on it to include
99-
// plugin content in the tab order.
100-
if (tag === 'object') {
101-
return true;
102-
}
103-
return false;
104-
}
105-
10669
// Returns "interactive", "non-interactive", or "unknown" (dynamic value).
10770
function roleStatus(node) {
10871
const attr = findAttr(node, 'role');
@@ -144,17 +107,25 @@ module.exports = {
144107
return;
145108
}
146109

110+
// Skip component invocations (PascalCase, named-arg, this-path,
111+
// dot-path, named-block). Fixes the <Article tabindex={{0}}> FP where
112+
// a PascalCase component name collided with a native tag after
113+
// lowercasing.
114+
if (isComponentInvocation(node)) {
115+
return;
116+
}
117+
147118
const tag = node.tag?.toLowerCase();
148119
if (!tag) {
149120
return;
150121
}
151122

152-
// Skip components and custom elements (not in aria-query's dom map).
123+
// Skip custom elements (not in aria-query's dom map).
153124
if (!dom.has(tag)) {
154125
return;
155126
}
156127

157-
if (isInteractiveElement(node)) {
128+
if (isNativeInteractive(node, getTextAttrValue)) {
158129
return;
159130
}
160131

‎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/audit/no-noninteractive-tabindex/peer-parity.js‎

Lines changed: 22 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -64,24 +64,19 @@ ruleTester.run('audit:no-noninteractive-tabindex (gts)', rule, {
6464
// Non-interactive native element + tabindex="-1" — valid via -1 exemption.
6565
'<template><article tabindex="-1"></article></template>',
6666

67-
// === DIVERGENCE — components whose name lowercases to a native tag ===
68-
// jsx-a11y `components` setting maps `<Article>` → `article`, then treats
69-
// it like the native tag. Without such a setting jsx-a11y would treat
70-
// `<Article>` as an opaque component and skip.
71-
// Our rule lowercases `node.tag` before the `dom.has(tag)` check, so
72-
// `<Article>` becomes `article` and IS validated against the dom map.
73-
// This has two effects:
74-
// (a) `<Article tabindex="-1" />` passes via the tabindex=-1 exemption
75-
// (valid in jsx-a11y too, so no visible divergence here).
76-
// (b) `<Article tabindex={{0}} />` is FLAGGED by our rule (see invalid
77-
// section below). jsx-a11y without `components` setting: VALID (opaque
78-
// component skip). jsx-a11y with `components: {Article: 'article'}`
79-
// setting: INVALID. Our rule: INVALID regardless — false positive
80-
// for the no-settings case.
81-
// Components whose name does NOT collide with a native tag (e.g.
82-
// `<MyButton>` → `mybutton` which is not in the dom map) are correctly
83-
// skipped.
67+
// === Upstream parity — PascalCase component skip ===
68+
// Components whose name lowercases to a native tag (e.g. `<Article>` →
69+
// `article`) are correctly skipped by `isComponentInvocation`, which
70+
// classifies the invocation BEFORE the `dom.has(tag)` check. Parity with
71+
// jsx-a11y's no-settings default (opaque component skip).
72+
//
73+
// Previously this was a FALSE POSITIVE (audit B8): `<Article tabindex={{0}} />`
74+
// was flagged because the rule lowercased `node.tag` before the
75+
// `dom.has(tag)` check, matching `article` in the dom map and validating
76+
// the component like the native tag. Adopting `isComponentInvocation`
77+
// fixes the FP.
8478
'<template><Article tabindex="-1" /></template>',
79+
'<template><Article tabindex={{0}} /></template>',
8580

8681
// === Upstream parity (jsx-a11y recommended valid) ===
8782

@@ -154,23 +149,11 @@ ruleTester.run('audit:no-noninteractive-tabindex (gts)', rule, {
154149
// assertion here; captured as a comment only. Our behavior matches
155150
// jsx-a11y RECOMMENDED, not strict.
156151

157-
// === DIVERGENCE — component name collides with a native tag ===
158-
// `<Article tabIndex={0} />` — classifications:
159-
// jsx-a11y without `components` setting: VALID (opaque component skip).
160-
// jsx-a11y with `components: {Article: 'article'}`: INVALID (article
161-
// is non-interactive).
162-
// Our rule: INVALID unconditionally. We lowercase `node.tag` before the
163-
// `dom.has(tag)` check, so `Article` → `article` is found in the dom
164-
// map and validated like the native tag. This is a FALSE POSITIVE
165-
// relative to jsx-a11y's no-settings default, and accidental parity
166-
// with jsx-a11y's components-configured mode.
167-
// Components whose lowercased name doesn't collide with a native tag
168-
// (e.g. `<MyButton>`) are correctly skipped.
169-
{
170-
code: '<template><Article tabindex={{0}} /></template>',
171-
output: null,
172-
errors: [{ messageId: 'noNonInteractiveTabindex' }],
173-
},
152+
// === Resolved — component name collides with a native tag ===
153+
// Audit B8 previously flagged `<Article tabindex={{0}} />` as a FALSE
154+
// POSITIVE. After adopting `isComponentInvocation`, PascalCase components
155+
// (including those whose lowercased name collides with a native tag) are
156+
// correctly skipped. See the valid section for the parity assertion.
174157
],
175158
});
176159

@@ -189,11 +172,12 @@ hbsRuleTester.run('audit:no-noninteractive-tabindex (hbs)', rule, {
189172
'<article tabindex="-1"></article>',
190173
// Dynamic role — parity with jsx-a11y recommended (allowExpressionValues: true).
191174
'<div role={{this.role}} tabindex="0"></div>',
192-
// Non-tag-colliding component — skipped (not in aria-query dom map).
193-
// Parity with jsx-a11y's no-settings default.
194-
// (Components whose lowercased name collides with a native tag diverge;
195-
// see `<Article tabindex={{0}} />` in the gts invalid section.)
175+
// Component invocation — skipped via isComponentInvocation. Parity with
176+
// jsx-a11y's no-settings default. Holds for both non-colliding names
177+
// (e.g. `<MyButton>`) and names that lowercase to a native tag
178+
// (e.g. `<Article>` → `article`).
196179
'<MyButton tabindex="0" />',
180+
'<Article tabindex="0"></Article>',
197181
],
198182
invalid: [
199183
// Parity — neverValid cases in hbs form.

‎tests/lib/rules/template-no-noninteractive-tabindex.js‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,22 @@ ruleTester.run('template-no-noninteractive-tabindex', rule, {
3737
'<template><CustomWidget tabindex="0" /></template>',
3838
'<template><my-widget tabindex="0"></my-widget></template>',
3939

40+
// PascalCase component whose name lowercases to a native tag — rule skips.
41+
// Before adopting isComponentInvocation, `<Article>` collided with
42+
// `article` in aria-query's dom map and produced a false positive
43+
// (audit B8). isComponentInvocation correctly classifies it as a
44+
// component invocation before the dom-map check.
45+
'<template><Article tabindex={{0}} /></template>',
46+
'<template><Article tabindex="0" /></template>',
47+
'<template><Form tabindex={{0}} /></template>',
48+
'<template><Section tabindex="0" /></template>',
49+
50+
// Named-arg, this-path, dot-path, and named-block invocations — rule skips.
51+
'<template><@heading tabindex="0" /></template>',
52+
'<template><this.myWidget tabindex="0" /></template>',
53+
'<template><foo.bar tabindex="0" /></template>',
54+
'<template><Foo::Bar tabindex="0" /></template>',
55+
4056
// Dynamic role — rule conservatively skips.
4157
'<template><div role={{this.role}} tabindex="0"></div></template>',
4258

‎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)