Skip to content

Commit

Permalink
fix(combobox): support aria 1.2 pattern for comboboxes (#3033)
Browse files Browse the repository at this point in the history 
* remove textbox from aria-required-children for combobox

* aria-required-attr changes for combobox

* address PR comments

* hardcode combobox owned roles

* fix another test

* use ariaAttrs.has-popup.values instead of hardcoded  list

* add comment

* add aria-owns attribute

* fix test
  • Loading branch information
@clottman
clottman authored Jul 6, 2021
1 parent 988d536 commit 5ab026d
Show file tree
Hide file tree
Showing 8 changed files with 83 additions and 242 deletions.
15 changes: 12 additions & 3 deletions lib/checks/aria/aria-required-attr-evaluate.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,8 @@ import { uniqueArray } from '../../core/utils';
function ariaRequiredAttrEvaluate(node, options = {}, virtualNode) {
const missing = [];
const attrs = virtualNode.attrNames;

const role = getExplicitRole(virtualNode);
if (attrs.length) {
const role = getExplicitRole(virtualNode);
let required = requiredAttr(role);
const elmSpec = getElementSpec(virtualNode);

Expand All @@ -56,11 +55,21 @@ function ariaRequiredAttrEvaluate(node, options = {}, virtualNode) {
}
}

// aria 1.2 combobox requires aria-controls, but aria-owns is acceptable instead in earlier versions of the guidelines
// https://github.com/dequelabs/axe-core/issues/2505#issuecomment-788703942
if (
missing.length === 1 &&
role === 'combobox' &&
missing[0] === 'aria-controls' &&
virtualNode.attr('aria-owns')
) {
return true;
}

if (missing.length) {
this.data(missing);
return false;
}

return true;
}

Expand Down
37 changes: 1 addition & 36 deletions lib/checks/aria/aria-required-children-evaluate.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,47 +38,12 @@ function getOwnedRoles(virtualNode, required) {
* Get missing children roles
*/
function missingRequiredChildren(virtualNode, role, required, ownedRoles) {
const isCombobox = role === 'combobox';

// combobox exceptions
if (isCombobox) {
// remove 'textbox' from missing roles if combobox is a native
// text-type input or owns a 'searchbox'
const textTypeInputs = ['text', 'search', 'email', 'url', 'tel'];
if (
(virtualNode.props.nodeName === 'input' &&
textTypeInputs.includes(virtualNode.props.type)) ||
ownedRoles.includes('searchbox')
) {
required = required.filter(requiredRole => requiredRole !== 'textbox');
}

// combobox only needs one of [listbox, tree, grid, dialog] and
// only the type that matches the aria-popup value. remove
// all the other popup roles from the list of required
const expandedChildRoles = ['listbox', 'tree', 'grid', 'dialog'];
const expandedValue = virtualNode.attr('aria-expanded');
const expanded = expandedValue && expandedValue.toLowerCase() !== 'false';
const popupRole = (
virtualNode.attr('aria-haspopup') || 'listbox'
).toLowerCase();
required = required.filter(
requiredRole =>
!expandedChildRoles.includes(requiredRole) ||
(expanded && requiredRole === popupRole)
);
}

for (let i = 0; i < ownedRoles.length; i++) {
var ownedRole = ownedRoles[i];

if (required.includes(ownedRole)) {
required = required.filter(requiredRole => requiredRole !== ownedRole);

// combobox requires all the roles not just any one of them
if (!isCombobox) {
return null;
}
return null;
}
}

Expand Down
4 changes: 2 additions & 2 deletions lib/rules/scrollable-region-focusable-matches.js
View file
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import { hasContentVirtual } from '../commons/dom';
import { getExplicitRole } from '../commons/aria';
import standards from '../standards';
import {
querySelectorAll,
getScroll,
closest,
getRootNode,
tokenList
} from '../core/utils';
import ariaAttrs from '../standards/aria-attrs';

function scrollableRegionFocusableMatches(node, virtualNode) {
/**
Expand All @@ -29,7 +29,7 @@ function scrollableRegionFocusableMatches(node, virtualNode) {
* @see https://github.com/dequelabs/axe-core/issues/1763
*/
const role = getExplicitRole(virtualNode);
if (standards.ariaRoles.combobox.requiredOwned.includes(role)) {
if (ariaAttrs['aria-haspopup'].values.includes(role)) {
// in ARIA 1.1 the container has role=combobox
if (closest(virtualNode, '[role~="combobox"]')) {
return false;
Expand Down
8 changes: 2 additions & 6 deletions lib/standards/aria-roles.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,13 +117,9 @@ const ariaRoles = {
},
combobox: {
type: 'composite',
requiredOwned: ['listbox', 'tree', 'grid', 'dialog', 'textbox'],
requiredAttrs: ['aria-expanded'],
// Note: because aria-controls is not well supported we will not
// make it a required attribute even though it is required in the
// spec
requiredAttrs: ['aria-expanded', 'aria-controls'],
allowedAttrs: [
'aria-controls',
'aria-owns',
'aria-autocomplete',
'aria-readonly',
'aria-required',
Expand Down
48 changes: 48 additions & 0 deletions test/checks/aria/aria-required-attr.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,54 @@ describe('aria-required-attr', function() {
);
});

describe('combobox special case', function() {
it('should pass comboboxes that have aria-owns and aria-expanded', function() {
var vNode = queryFixture(
'<div id="target" role="combobox" aria-expanded="false" aria-owns="ownedchild"></div>'
);

assert.isTrue(
axe.testUtils
.getCheckEvaluate('aria-required-attr')
.call(checkContext, vNode.actualNode, options, vNode)
);
});

it('should pass comboboxes that have aria-controls and aria-expanded', function() {
var vNode = queryFixture(
'<div id="target" role="combobox" aria-expanded="false" aria-controls="test"></div>'
);

assert.isTrue(
axe.testUtils
.getCheckEvaluate('aria-required-attr')
.call(checkContext, vNode.actualNode, options, vNode)
);
});

it('should fail comboboxes that have aria-expanded only', function() {
var vNode = queryFixture(
'<div id="target" role="combobox" aria-expanded="false"></div>'
);

assert.isFalse(
axe.testUtils
.getCheckEvaluate('aria-required-attr')
.call(checkContext, vNode.actualNode, options, vNode)
);
});

it('should fail comboboxes that have no required attributes', function() {
var vNode = queryFixture('<div id="target" role="combobox"></div>');

assert.isFalse(
axe.testUtils
.getCheckEvaluate('aria-required-attr')
.call(checkContext, vNode.actualNode, options, vNode)
);
});
});

describe('options', function() {
it('should require provided attribute names for a role', function() {
axe.configure({
Expand Down
193 changes: 0 additions & 193 deletions test/checks/aria/required-children.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,41 +82,6 @@ describe('aria-required-children', function() {
}
);

it('should detect multiple missing required children when all required', function() {
var params = checkSetup(
'<div role="combobox" id="target" aria-expanded="true"><p>Nothing here.</p></div>'
);
assert.isFalse(
axe.testUtils
.getCheckEvaluate('aria-required-children')
.apply(checkContext, params)
);
assert.deepEqual(checkContext._data, ['listbox', 'textbox']);
});

it('should detect single missing required child when all required', function() {
var params = checkSetup(
'<div role="combobox" id="target" aria-expanded="true"><p role="listbox">Nothing here.</p></div>'
);
assert.isFalse(
axe.testUtils
.getCheckEvaluate('aria-required-children')
.apply(checkContext, params)
);
assert.deepEqual(checkContext._data, ['textbox']);
});

it('should pass all existing required children when all required', function() {
var params = checkSetup(
'<div role="combobox" id="target"><p role="listbox">Nothing here.</p><p role="textbox">Textbox</p></div>'
);
assert.isTrue(
axe.testUtils
.getCheckEvaluate('aria-required-children')
.apply(checkContext, params)
);
});

it('should pass all existing required children when all required', function() {
var params = checkSetup(
'<div id="target" role="menu"><li role="none"></li><li role="menuitem">Item 1</li><div role="menuitemradio">Item 2</div><div role="menuitemcheckbox">Item 3</div></div>'
Expand Down Expand Up @@ -163,142 +128,6 @@ describe('aria-required-children', function() {
);
});

(shadowSupported ? it : xit)(
'should pass all existing required children in shadow tree when all required',
function() {
fixture.innerHTML = '<div role="combobox" id="target"></div>';

var target = document.querySelector('#target');
var shadowRoot = target.attachShadow({ mode: 'open' });
shadowRoot.innerHTML =
'<p role="listbox">Nothing here.</p><p role="textbox">Textbox</p>';

axe.testUtils.flatTreeSetup(fixture);
var virtualTarget = axe.utils.getNodeFromTree(target);

var params = [target, undefined, virtualTarget];
assert.isTrue(
axe.testUtils
.getCheckEvaluate('aria-required-children')
.apply(checkContext, params)
);
}
);

it('should pass a native "text" type input with role comboxbox when missing child is role textbox', function() {
var params = checkSetup(
'<input type="text" role="combobox" aria-owns="listbox" id="target"><p role="listbox" id="listbox">Nothing here.</p>'
);
assert.isTrue(
axe.testUtils
.getCheckEvaluate('aria-required-children')
.apply(checkContext, params)
);
});

it('should pass a native "search" type input with role comboxbox when missing child is role textbox', function() {
var params = checkSetup(
'<input type="search" role="combobox" aria-owns="listbox1" id="target"><p role="listbox" id="listbox1">Nothing here.</p>'
);
assert.isTrue(
axe.testUtils
.getCheckEvaluate('aria-required-children')
.apply(checkContext, params)
);
});

it('should pass a native "email" type input with role comboxbox when missing child is role textbox', function() {
var params = checkSetup(
'<input type="email" role="combobox" aria-owns="listbox" id="target"><p role="listbox" id="listbox">Nothing here.</p>'
);
assert.isTrue(
axe.testUtils
.getCheckEvaluate('aria-required-children')
.apply(checkContext, params)
);
});

it('should pass a native "url" type input with role comboxbox when missing child is role textbox', function() {
var params = checkSetup(
'<input type="url" role="combobox" aria-owns="listbox" id="target"><p role="listbox" id="listbox">Nothing here.</p>'
);
assert.isTrue(
axe.testUtils
.getCheckEvaluate('aria-required-children')
.apply(checkContext, params)
);
});

it('should pass a native "tel" type input with role comboxbox when missing child is role textbox', function() {
var params = checkSetup(
'<input type="tel" role="combobox" aria-owns="listbox" id="target"><p role="listbox" id="listbox">Nothing here.</p>'
);
assert.isTrue(
axe.testUtils
.getCheckEvaluate('aria-required-children')
.apply(checkContext, params)
);
});

it('should pass a collapsed comboxbox when missing child is role listbox', function() {
var params = checkSetup(
'<div role="combobox" id="target"><p role="textbox">Textbox</p></div>'
);
assert.isTrue(
axe.testUtils
.getCheckEvaluate('aria-required-children')
.apply(checkContext, params)
);
});

it('should pass an expanded combobox when the required popup role matches', function() {
var params = checkSetup(
'<div role="combobox" aria-haspopup="grid" aria-expanded="true" id="target"><p role="textbox">Textbox</p><div role="grid"></div></div>'
);
assert.isTrue(
axe.testUtils
.getCheckEvaluate('aria-required-children')
.apply(checkContext, params)
);
});

it('should fail an expanded combobox when the required role is missing on children', function() {
var params = checkSetup(
'<div role="combobox" aria-haspopup="grid" aria-expanded="true" id="target"><p role="textbox">Textbox</p><div role="listbox"></div></div>'
);
assert.isFalse(
axe.testUtils
.getCheckEvaluate('aria-required-children')
.apply(checkContext, params)
);

assert.deepEqual(checkContext._data, ['grid']);
});

it('should pass an expanded combobox when the required popup role matches regarless of case', function() {
var params = checkSetup(
'<div role="combobox" aria-haspopup="gRiD" aria-expanded="true" id="target"><p role="textbox">Textbox</p><div role="grid"></div></div>'
);
assert.isTrue(
axe.testUtils
.getCheckEvaluate('aria-required-children')
.apply(checkContext, params)
);
});

it('should fail when combobox child isnt default listbox', function() {
var params = checkSetup(
'<div role="combobox" aria-expanded="true" id="target"><p role="textbox">Textbox</p><div role="grid"></div></div>'
);
assert.isFalse(
axe.testUtils
.getCheckEvaluate('aria-required-children')
.apply(checkContext, params)
);

assert.deepEqual(checkContext._data, ['listbox']);
});

it('should fail when list does not have required children listitem', function() {
var params = checkSetup(
'<div id="target" role="list"><span>Item 1</span></div>'
Expand Down Expand Up @@ -470,28 +299,6 @@ describe('aria-required-children', function() {
);
});

it('should pass role comboxbox when child is native "search" input type', function() {
var params = checkSetup(
'<div role="combobox" id="target"><input type="search"><p role="listbox">Textbox</p></div>'
);
assert.isTrue(
axe.testUtils
.getCheckEvaluate('aria-required-children')
.apply(checkContext, params)
);
});

it('should not accept implicit nodes with a different role', function() {
var params = checkSetup(
'<div role="combobox" id="target"><input type="search" role="spinbutton"><p role="listbox">Textbox</p></div>'
);
assert.isFalse(
axe.testUtils
.getCheckEvaluate('aria-required-children')
.apply(checkContext, params)
);
});

it('should pass when role allows group and group has required child', function() {
var params = checkSetup(
'<div role="menu" id="target"><ul role="group"><li role="menuitem">Menuitem</li></ul></div>'
Expand Down
Loading

0 comments on commit 5ab026d

Please sign in to comment.