Skip to content

Commit 802c738

Browse files
committed
fix: condition select→combobox implicit role on multiple/size
Per HTML-AAM, <select>'s implicit role is "combobox" only when neither `multiple` nor `size > 1` is present; otherwise it is "listbox". The previous commit added `combobox: ['select']` unconditionally, which caused false positives for <select role="combobox" multiple> and <select role="combobox" size="5"> (where combobox disagrees with the implicit listbox role and therefore is not redundant). Add a selectHasComboboxImplicitRole helper mirroring jsx-a11y's src/util/implicitRoles/select.js, and short-circuit the redundancy check for <select role="combobox"> when the implicit role is actually listbox. Update the code comment on the `combobox: ['select']` entry to reflect this (and drop the inaccurate "§4.1" reference — the <select> mapping lives in the HTML-AAM main conformance table, section 4, without a numbered subsection). Tests: - valid: <select role="combobox" multiple></select> - valid: <select role="combobox" size="5"></select> - invalid: <select role="combobox" size="1"></select> (size=1 → combobox) - invalid: <select role="COMBOBOX"></select> (case + implicit)
1 parent 8fc6f6d commit 802c738

2 files changed

Lines changed: 56 additions & 2 deletions

File tree

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

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,25 @@ const ALLOWED_ELEMENT_ROLES = [
3737
{ name: 'input', role: 'combobox' },
3838
];
3939

40+
// Per HTML-AAM, <select> maps to "combobox" only when neither `multiple` nor
41+
// `size > 1` is set; otherwise it maps to "listbox". Mirrors jsx-a11y's
42+
// src/util/implicitRoles/select.js.
43+
function selectHasComboboxImplicitRole(node) {
44+
const attrs = node.attributes || [];
45+
const hasMultiple = attrs.some((a) => a.name === 'multiple');
46+
if (hasMultiple) {
47+
return false;
48+
}
49+
const sizeAttr = attrs.find((a) => a.name === 'size');
50+
if (sizeAttr && sizeAttr.value && sizeAttr.value.type === 'GlimmerTextNode') {
51+
const sizeValue = Number(sizeAttr.value.chars);
52+
if (Number.isFinite(sizeValue) && sizeValue > 1) {
53+
return false;
54+
}
55+
}
56+
return true;
57+
}
58+
4059
// Mapping of roles to their corresponding HTML elements
4160
// From https://www.w3.org/TR/html-aria/
4261
const ROLE_TO_ELEMENTS = {
@@ -45,8 +64,9 @@ const ROLE_TO_ELEMENTS = {
4564
button: ['button'],
4665
cell: ['td'],
4766
checkbox: ['input'],
48-
// <select> is a combobox by default (per HTML-AAM §4.1 — unless `multiple` or
49-
// a `size > 1` is set, in which case it's a listbox; both mappings are listed).
67+
// <select> is a combobox by default per HTML-AAM (section 4). When
68+
// `multiple` is present or `size > 1`, it maps to "listbox" instead;
69+
// that case is handled at the call site via selectHasComboboxImplicitRole.
5070
combobox: ['select'],
5171
columnheader: ['th'],
5272
complementary: ['aside'],
@@ -145,6 +165,18 @@ module.exports = {
145165
return;
146166
}
147167

168+
// <select role="combobox"> is only redundant when <select>'s implicit
169+
// role actually is "combobox" (no `multiple`, and `size` absent or <= 1).
170+
// Otherwise the implicit role is "listbox", so the explicit "combobox"
171+
// is not redundant and this rule should not flag it.
172+
if (
173+
node.tag === 'select' &&
174+
roleValue === 'combobox' &&
175+
!selectHasComboboxImplicitRole(node)
176+
) {
177+
return;
178+
}
179+
148180
const isRedundant =
149181
elementsWithRole.includes(node.tag) &&
150182
!ALLOWED_ELEMENT_ROLES.some((e) => e.name === node.tag && e.role === roleValue);

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

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,10 @@ 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>',
5155
],
5256
invalid: [
5357
{
@@ -155,6 +159,12 @@ hbsRuleTester.run('template-no-redundant-role', rule, {
155159
options: [{ checkAllHTMLElements: false }],
156160
},
157161
'<ul class="list" role="combobox"></ul>',
162+
// <select> with `multiple` has implicit role "listbox", so role="combobox"
163+
// is not redundant (it disagrees with the implicit role, but that is for
164+
// other rules to catch — this rule only flags redundancy).
165+
'<select role="combobox" multiple></select>',
166+
// <select size="5"> (size > 1) has implicit role "listbox", same reasoning.
167+
'<select role="combobox" size="5"></select>',
158168
],
159169
invalid: [
160170
{
@@ -249,6 +259,18 @@ hbsRuleTester.run('template-no-redundant-role', rule, {
249259
output: '<select></select>',
250260
errors: [{ message: 'Use of redundant or invalid role: combobox on <select> detected.' }],
251261
},
262+
{
263+
// size="1" still defaults to combobox (only size > 1 flips to listbox).
264+
code: '<select role="combobox" size="1"></select>',
265+
output: '<select size="1"></select>',
266+
errors: [{ message: 'Use of redundant or invalid role: combobox on <select> detected.' }],
267+
},
268+
{
269+
// Case-insensitive match on <select>, combined with the implicit-role check.
270+
code: '<select role="COMBOBOX"></select>',
271+
output: '<select></select>',
272+
errors: [{ message: 'Use of redundant or invalid role: combobox on <select> detected.' }],
273+
},
252274
{
253275
// Case-insensitive matching — ARIA role tokens compare as ASCII-case-insensitive.
254276
code: '<body role="DOCUMENT"></body>',

0 commit comments

Comments
 (0)