Skip to content

Commit 0da6be9

Browse files
NullVoxPopuliNullVoxPopuli
authored
Merge pull request #2726 from johanrd/fix/implicit-role-attribute-constraints
BUGFIX: template-no-unsupported-role-attributes — honor aria-query attribute constraints
2 parents 970fcc4 + dc93f21 commit 0da6be9

2 files changed

Lines changed: 147 additions & 28 deletions

File tree

‎lib/rules/template-no-unsupported-role-attributes.js‎

Lines changed: 89 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,30 +1,100 @@
11
const { roles, elementRoles } = require('aria-query');
22

3-
function createUnsupportedAttributeErrorMessage(attribute, role, element) {
4-
if (element) {
5-
return `The attribute ${attribute} is not supported by the element ${element} with the implicit role of ${role}`;
3+
function getStaticAttrValue(node, name) {
4+
const attr = node.attributes?.find((a) => a.name === name);
5+
if (!attr) {
6+
return undefined;
67
}
8+
if (!attr.value || attr.value.type !== 'GlimmerTextNode') {
9+
// Presence with dynamic value — treat as "set" but unknown string.
10+
return '';
11+
}
12+
return attr.value.chars.trim();
13+
}
14+
15+
function nodeSatisfiesAttributeConstraint(node, attrSpec) {
16+
const value = getStaticAttrValue(node, attrSpec.name);
17+
const isSet = value !== undefined;
718

8-
return `The attribute ${attribute} is not supported by the role ${role}`;
19+
if (attrSpec.constraints?.includes('set')) {
20+
return isSet;
21+
}
22+
if (attrSpec.constraints?.includes('undefined')) {
23+
return !isSet;
24+
}
25+
if (attrSpec.value !== undefined) {
26+
// HTML enumerated attribute values are ASCII case-insensitive
27+
// (HTML common-microsyntaxes §2.3.3). aria-query's attrSpec.value is
28+
// already lowercase, so lowercase the node's value for comparison.
29+
return isSet && value.toLowerCase() === attrSpec.value;
30+
}
31+
// No constraint listed — just require presence.
32+
return isSet;
933
}
1034

11-
function getImplicitRole(tagName, typeAttribute) {
12-
if (tagName === 'input') {
13-
for (const key of elementRoles.keys()) {
14-
if (key.name === tagName && key.attributes) {
15-
for (const attribute of key.attributes) {
16-
if (attribute.name === 'type' && attribute.value === typeAttribute) {
17-
return elementRoles.get(key)[0];
18-
}
19-
}
20-
}
21-
}
35+
function keyMatchesNode(node, key) {
36+
if (key.name !== node.tag) {
37+
return false;
2238
}
39+
if (!key.attributes || key.attributes.length === 0) {
40+
return true;
41+
}
42+
return key.attributes.every((attrSpec) => nodeSatisfiesAttributeConstraint(node, attrSpec));
43+
}
2344

24-
const key = [...elementRoles.keys()].find((entry) => entry.name === tagName);
25-
const implicitRoles = key && elementRoles.get(key);
45+
// Pre-index elementRoles by tag name at module load. aria-query's Map is
46+
// static data; bucketing by tag turns the per-call scan (~80 keys) into a
47+
// 1–5 key lookup per tag. Benchmarked at ~2.6× speedup on realistic
48+
// 200k-call workloads; parity verified across representative tag/attr
49+
// combinations before landing.
50+
const ELEMENT_ROLES_KEYS_BY_TAG = buildElementRolesIndex();
51+
52+
function buildElementRolesIndex() {
53+
const index = new Map();
54+
for (const key of elementRoles.keys()) {
55+
if (!index.has(key.name)) {
56+
index.set(key.name, []);
57+
}
58+
index.get(key.name).push(key);
59+
}
60+
return index;
61+
}
2662

27-
return implicitRoles && implicitRoles[0];
63+
function getImplicitRole(node) {
64+
// Honor aria-query's attribute constraints when mapping element -> implicit role.
65+
// Each elementRoles entry lists attributes that must match (with optional
66+
// constraints "set" / "undefined"); pick the most specific entry whose
67+
// attribute spec is fully satisfied by the node.
68+
//
69+
// Heuristic: "specificity = attribute-constraint count". aria-query exports
70+
// elementRoles as an unordered Map and does not document how consumers
71+
// should resolve multi-match cases; this count-based tiebreak is an
72+
// inference from the data shape. It resolves the motivating bugs:
73+
// - <input type="text"> without `list` → textbox, not combobox
74+
// (the combobox entry requires `list=set`, a stricter 2-attr match;
75+
// the textbox entry's 1-attr type=text wins when `list` is absent).
76+
// - <input type="password"> → no role (no elementRoles entry matches).
77+
// If aria-query ever publishes a resolution order, switch to that.
78+
const keys = ELEMENT_ROLES_KEYS_BY_TAG.get(node.tag);
79+
if (!keys) {
80+
return undefined;
81+
}
82+
let bestKey;
83+
let bestSpecificity = -1;
84+
for (const key of keys) {
85+
if (!keyMatchesNode(node, key)) {
86+
continue;
87+
}
88+
const specificity = key.attributes?.length ?? 0;
89+
if (specificity > bestSpecificity) {
90+
bestKey = key;
91+
bestSpecificity = specificity;
92+
}
93+
}
94+
if (!bestKey) {
95+
return undefined;
96+
}
97+
return elementRoles.get(bestKey)[0];
2898
}
2999

30100
function getExplicitRole(node) {
@@ -35,14 +105,6 @@ function getExplicitRole(node) {
35105
return null;
36106
}
37107

38-
function getTypeAttribute(node) {
39-
const typeAttr = node.attributes?.find((attr) => attr.name === 'type');
40-
if (typeAttr && typeAttr.value?.type === 'GlimmerTextNode') {
41-
return typeAttr.value.chars.trim();
42-
}
43-
return null;
44-
}
45-
46108
function removeRangeWithAdjacentWhitespace(sourceText, range) {
47109
let [start, end] = range;
48110

@@ -111,8 +173,7 @@ module.exports = {
111173

112174
if (!role) {
113175
element = node.tag;
114-
const typeAttribute = getTypeAttribute(node);
115-
role = getImplicitRole(element, typeAttribute);
176+
role = getImplicitRole(node);
116177
}
117178

118179
if (!role) {

‎tests/lib/rules/template-no-unsupported-role-attributes.js‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,23 @@ const validHbs = [
2121
'<ItemCheckbox @model={{@model}} @checkable={{@checkable}} />',
2222
'<some-custom-element />',
2323
'<input type="password">',
24+
25+
// <input type="password"> has no implicit role per aria-query (it's intentionally
26+
// not mapped so that screen readers don't announce typed content). No role →
27+
// no aria-supported-props check. Pin this with attributes that would be REJECTED
28+
// on a textbox (aria-checked, aria-selected): if the rule mis-classified password
29+
// as a textbox fallback, these would flag.
30+
'<input type="password" aria-describedby="hint" />',
31+
'<input type="password" aria-required="true" />',
32+
'<input type="password" aria-checked="false" />',
33+
'<input type="password" aria-selected="true" />',
34+
35+
// <input type="text"> without a list attribute is a textbox — aria-required,
36+
// aria-readonly, aria-placeholder are all supported.
37+
'<input type="text" aria-required="true" />',
38+
'<input type="email" aria-readonly="true" />',
39+
'<input type="tel" aria-required="true" />',
40+
'<input type="url" aria-placeholder="https://…" />',
2441
];
2542

2643
const invalidHbs = [
@@ -80,8 +97,21 @@ const invalidHbs = [
8097
],
8198
},
8299
{
100+
// <input type="email"> without a `list` attribute → implicit role "textbox"
101+
// (per aria-query / HTML-AAM). With a `list` attribute it would be "combobox".
83102
code: '<input type="email" aria-level={{this.level}} />',
84103
output: '<input type="email" />',
104+
errors: [
105+
{
106+
message:
107+
'The attribute aria-level is not supported by the element input with the implicit role of textbox',
108+
},
109+
],
110+
},
111+
{
112+
// With a `list` attribute, <input type="email"> becomes a combobox.
113+
code: '<input type="email" list="x" aria-level={{this.level}} />',
114+
output: '<input type="email" list="x" />',
85115
errors: [
86116
{
87117
message:
@@ -94,6 +124,34 @@ const invalidHbs = [
94124
output: '{{foo-component role="button"}}',
95125
errors: [{ message: 'The attribute aria-valuetext is not supported by the role button' }],
96126
},
127+
// Documented divergence with jsx-a11y on implicit role for <body>.
128+
// jsx-a11y resolves <body> to role "document"; aria-query (which our rule
129+
// uses) resolves to "generic". aria-expanded is unsupported by either, so
130+
// both plugins flag — only the role-name in the message differs.
131+
{
132+
code: '<body aria-expanded="true"></body>',
133+
output: '<body></body>',
134+
errors: [
135+
{
136+
message:
137+
'The attribute aria-expanded is not supported by the element body with the implicit role of generic',
138+
},
139+
],
140+
},
141+
// <a> without href — implicit role is `generic` per HTML-AAM 1.2 §3.5.3
142+
// (https://www.w3.org/TR/html-aam/#el-a-no-href). aria-checked is not
143+
// supported on `generic`, so we flag. vue-a11y reaches the same conclusion
144+
// (it walks aria-query the same way). jsx-a11y's `getImplicitRoleForAnchor`
145+
// returns `''` for href-less <a>, which makes its role-supports-aria-props
146+
// rule early-return and silently allow any aria-* attribute — its own
147+
// source comments this as "This actually isn't true - should fix in future
148+
// release." We're spec-current; jsx-a11y is spec-stale (legacy ARIA 1.1
149+
// mental model).
150+
{
151+
code: '<a aria-checked />',
152+
output: '<a />',
153+
errors: [{ messageId: 'unsupportedImplicit' }],
154+
},
97155
];
98156

99157
function wrapTemplate(entry) {

0 commit comments

Comments
 (0)