Skip to content

Commit ca16804

Browse files
Merge pull request #2727 from johanrd/fix/no-redundant-role-case-and-select
BUGFIX: template-no-redundant-role — case-insensitive match + <select>→combobox
2 parents 086b8b5 + 5bc47f9 commit ca16804

2 files changed

Lines changed: 169 additions & 1 deletion

File tree

lib/rules/template-no-redundant-role.js

Lines changed: 77 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
const { roles } = require('aria-query');
2+
13
const DEFAULT_CONFIG = {
24
checkAllHTMLElements: true,
35
};
@@ -37,6 +39,48 @@ const ALLOWED_ELEMENT_ROLES = [
3739
{ name: 'input', role: 'combobox' },
3840
];
3941

42+
// Per HTML-AAM, <select> maps to "combobox" only when neither `multiple` nor
43+
// `size > 1` is set; otherwise it maps to "listbox". Mirrors jsx-a11y's
44+
// src/util/implicitRoles/select.js.
45+
//
46+
// Returns 'combobox' / 'listbox' for static cases, or 'unknown' when a
47+
// dynamic `multiple` or `size` value blocks a decision. Callers should skip
48+
// flagging on 'unknown' to avoid false positives.
49+
function getSelectImplicitRole(node) {
50+
const attrs = node.attributes || [];
51+
const multipleAttr = attrs.find((a) => a.name === 'multiple');
52+
if (multipleAttr) {
53+
// Valueless `multiple` or static string value — statically present.
54+
if (!multipleAttr.value || multipleAttr.value.type === 'GlimmerTextNode') {
55+
return 'listbox';
56+
}
57+
// Dynamic `multiple={{...}}` — Ember omits bound boolean attributes at
58+
// runtime when the value is falsy, so we can't tell statically whether
59+
// the implicit role is combobox or listbox.
60+
return 'unknown';
61+
}
62+
const sizeAttr = attrs.find((a) => a.name === 'size');
63+
if (sizeAttr) {
64+
// Valueless `size` (e.g. `<select size>`) — per HTML boolean-attr
65+
// semantics the attribute value is an empty string, which Number()
66+
// parses as 0. Per HTML's default size (>1 → listbox), 0 leaves the
67+
// implicit role as combobox. Treat the same as the static-0 case.
68+
if (!sizeAttr.value) {
69+
return 'combobox';
70+
}
71+
if (sizeAttr.value.type !== 'GlimmerTextNode') {
72+
// Dynamic `size={{...}}` / concat — can't tell whether the runtime
73+
// value is >1 or not, so bail out instead of risking a false positive.
74+
return 'unknown';
75+
}
76+
const sizeValue = Number(sizeAttr.value.chars);
77+
if (Number.isFinite(sizeValue) && sizeValue > 1) {
78+
return 'listbox';
79+
}
80+
}
81+
return 'combobox';
82+
}
83+
4084
// Mapping of roles to their corresponding HTML elements
4185
// From https://www.w3.org/TR/html-aria/
4286
const ROLE_TO_ELEMENTS = {
@@ -45,6 +89,10 @@ const ROLE_TO_ELEMENTS = {
4589
button: ['button'],
4690
cell: ['td'],
4791
checkbox: ['input'],
92+
// <select> is a combobox by default per HTML-AAM (section 4). When
93+
// `multiple` is present or `size > 1`, it maps to "listbox" instead;
94+
// that case is handled at the call site via getSelectImplicitRole.
95+
combobox: ['select'],
4896
columnheader: ['th'],
4997
complementary: ['aside'],
5098
contentinfo: ['footer'],
@@ -125,7 +173,19 @@ module.exports = {
125173

126174
let roleValue;
127175
if (roleAttr.value && roleAttr.value.type === 'GlimmerTextNode') {
128-
roleValue = roleAttr.value.chars || '';
176+
// ARIA role tokens are compared ASCII-case-insensitively, and the
177+
// attribute is a space-separated fallback list. Per WAI-ARIA §4.1,
178+
// UAs walk tokens for the first role they RECOGNISE — unknown
179+
// leading tokens are skipped, subsequent tokens are author-provided
180+
// fallbacks. `role="xxyxyz button"` resolves to `button`;
181+
// `role="tab button"` resolves to `tab` (recognised first, even
182+
// though no implicit mapping — this rule then has nothing to flag).
183+
const tokens = (roleAttr.value.chars || '').trim().toLowerCase().split(/\s+/u);
184+
const firstRecognised = tokens.find((t) => t && roles.has(t));
185+
if (!firstRecognised) {
186+
return;
187+
}
188+
roleValue = firstRecognised;
129189
} else {
130190
// Skip dynamic role values
131191
return;
@@ -138,9 +198,25 @@ module.exports = {
138198

139199
const elementsWithRole = ROLE_TO_ELEMENTS[roleValue];
140200
if (!elementsWithRole) {
201+
// Role is recognised by ARIA but has no implicit-element mapping
202+
// in this table — nothing can be redundant.
141203
return;
142204
}
143205

206+
// <select>'s implicit role depends on attributes (HTML-AAM):
207+
// - default (no `multiple`, `size` absent or <= 1) → "combobox"
208+
// - `multiple` or `size` > 1 → "listbox"
209+
// A role attribute is only redundant when it matches the element's
210+
// computed implicit role. Guard both combobox and listbox against
211+
// the opposite configuration, and bail when `size` is dynamic
212+
// ('unknown') rather than risk a false positive.
213+
if (node.tag === 'select' && (roleValue === 'combobox' || roleValue === 'listbox')) {
214+
const implicit = getSelectImplicitRole(node);
215+
if (implicit !== roleValue) {
216+
return;
217+
}
218+
}
219+
144220
const isRedundant =
145221
elementsWithRole.includes(node.tag) &&
146222
!ALLOWED_ELEMENT_ROLES.some((e) => e.name === node.tag && e.role === roleValue);

tests/lib/rules/template-no-redundant-role.js

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,24 @@ ruleTester.run('template-no-redundant-role', rule, {
4848
options: [{ checkAllHTMLElements: false }],
4949
},
5050
'<template><input role="combobox"></template>',
51+
// <select multiple> has implicit role listbox, so combobox is not redundant.
52+
'<template><select role="combobox" multiple></select></template>',
53+
// <select size="5"> (size > 1) has implicit role listbox.
54+
'<template><select role="combobox" size="5"></select></template>',
55+
// Default <select> (no `multiple`, `size` absent or <= 1) has implicit
56+
// role "combobox" — an explicit `role="listbox"` overrides to listbox
57+
// and is NOT redundant.
58+
'<template><select role="listbox"></select></template>',
59+
'<template><select role="listbox" size="1"></select></template>',
60+
// Dynamic `multiple={{...}}` — can't determine implicit role statically,
61+
// so neither `role="combobox"` nor `role="listbox"` is flagged.
62+
'<template><select role="combobox" multiple={{this.isMulti}}></select></template>',
63+
'<template><select role="listbox" multiple={{this.isMulti}}></select></template>',
64+
65+
// Role-fallback: first recognised token wins. `role="tab button"` on
66+
// <button> resolves to `tab` (non-redundant — button's implicit is
67+
// `button`, not `tab`). WAI-ARIA §4.1 fallback-list semantics.
68+
'<template><button role="tab button"></button></template>',
5169
],
5270
invalid: [
5371
{
@@ -69,6 +87,45 @@ ruleTester.run('template-no-redundant-role', rule, {
6987
},
7088
],
7189
},
90+
// Non-landmark same-role redundancy — covered by jsx-a11y / vue-a11y too.
91+
{
92+
code: '<template><button role="button"></button></template>',
93+
output: '<template><button></button></template>',
94+
errors: [{ message: 'Use of redundant or invalid role: button on <button> detected.' }],
95+
},
96+
{
97+
code: '<template><img role="img" /></template>',
98+
output: '<template><img /></template>',
99+
errors: [{ message: 'Use of redundant or invalid role: img on <img> detected.' }],
100+
},
101+
{
102+
// Valueless `<select size>` — per HTML boolean-attr semantics, the
103+
// attribute value is an empty string; Number('') is 0; 0 is NOT > 1,
104+
// so the implicit role stays combobox. `role="combobox"` is therefore
105+
// redundant and must be flagged.
106+
code: '<template><select role="combobox" size></select></template>',
107+
output: '<template><select size></select></template>',
108+
errors: [
109+
{
110+
message: 'Use of redundant or invalid role: combobox on <select> detected.',
111+
},
112+
],
113+
},
114+
{
115+
// Role-fallback: unknown leading token is skipped per ARIA §4.1.
116+
// `role="xxyxyz button"` resolves to `button`, which IS redundant on
117+
// <button>. Autofix drops the whole role attribute — the implicit
118+
// `button` role is preserved natively, so runtime semantics are
119+
// unchanged. Authors who wanted the `xxyxyz` fallback for some
120+
// reason can opt out via eslint-disable.
121+
code: '<template><button role="xxyxyz button"></button></template>',
122+
output: '<template><button></button></template>',
123+
errors: [
124+
{
125+
message: 'Use of redundant or invalid role: button on <button> detected.',
126+
},
127+
],
128+
},
72129
{
73130
code: '<template><main role="main"></main></template>',
74131
output: '<template><main></main></template>',
@@ -155,6 +212,17 @@ hbsRuleTester.run('template-no-redundant-role', rule, {
155212
options: [{ checkAllHTMLElements: false }],
156213
},
157214
'<ul class="list" role="combobox"></ul>',
215+
// <select> with `multiple` has implicit role "listbox", so role="combobox"
216+
// is not redundant (it disagrees with the implicit role, but that is for
217+
// other rules to catch — this rule only flags redundancy).
218+
'<select role="combobox" multiple></select>',
219+
// <select size="5"> (size > 1) has implicit role "listbox", same reasoning.
220+
'<select role="combobox" size="5"></select>',
221+
// Default <select> (no `multiple`, `size` absent or <= 1) has implicit
222+
// role "combobox" — explicit role="listbox" overrides to listbox and is
223+
// NOT redundant.
224+
'<select role="listbox"></select>',
225+
'<select role="listbox" size="1"></select>',
158226
],
159227
invalid: [
160228
{
@@ -243,6 +311,30 @@ hbsRuleTester.run('template-no-redundant-role', rule, {
243311
'<select name="color" id="color" multiple><option value="default-color">black</option></select>',
244312
errors: [{ message: 'Use of redundant or invalid role: listbox on <select> detected.' }],
245313
},
314+
{
315+
// <select> without `multiple` or `size` defaults to role "combobox".
316+
code: '<select role="combobox"></select>',
317+
output: '<select></select>',
318+
errors: [{ message: 'Use of redundant or invalid role: combobox on <select> detected.' }],
319+
},
320+
{
321+
// size="1" still defaults to combobox (only size > 1 flips to listbox).
322+
code: '<select role="combobox" size="1"></select>',
323+
output: '<select size="1"></select>',
324+
errors: [{ message: 'Use of redundant or invalid role: combobox on <select> detected.' }],
325+
},
326+
{
327+
// Case-insensitive match on <select>, combined with the implicit-role check.
328+
code: '<select role="COMBOBOX"></select>',
329+
output: '<select></select>',
330+
errors: [{ message: 'Use of redundant or invalid role: combobox on <select> detected.' }],
331+
},
332+
{
333+
// Case-insensitive matching — ARIA role tokens compare as ASCII-case-insensitive.
334+
code: '<body role="DOCUMENT"></body>',
335+
output: '<body></body>',
336+
errors: [{ message: 'Use of redundant or invalid role: document on <body> detected.' }],
337+
},
246338
{
247339
code: '<main role="main"></main>',
248340
output: '<main></main>',

0 commit comments

Comments
 (0)