Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(aria-required-children): allow comboboxes with more popup roles #1950

Merged
merged 5 commits into from
Jan 6, 2020
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
63 changes: 37 additions & 26 deletions lib/checks/aria/required-children.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ function owns(node, virtualTree, role, ariaOwned) {
if (node === null) {
return false;
}
var implicit = implicitNodes(role),
selector = ['[role="' + role + '"]'];
const implicit = implicitNodes(role);
let selector = ['[role="' + role + '"]'];

if (implicit) {
selector = selector.concat(
Expand All @@ -27,28 +27,25 @@ function owns(node, virtualTree, role, ariaOwned) {
}

function ariaOwns(nodes, role) {
var index, length;

for (index = 0, length = nodes.length; index < length; index++) {
if (nodes[index] === null) {
for (let index = 0; index < nodes.length; index++) {
const node = nodes[index];
if (node === null) {
continue;
}
const virtualTree = axe.utils.getNodeFromTree(nodes[index]);
if (owns(nodes[index], virtualTree, role, true)) {
const virtualTree = axe.utils.getNodeFromTree(node);
if (owns(node, virtualTree, role, true)) {
return true;
}
}
return false;
}

function missingRequiredChildren(node, childRoles, all, role) {
var index,
length = childRoles.length,
missing = [],
const missing = [],
ownedElements = idrefs(node, 'aria-owns');

for (index = 0; index < length; index++) {
var childRole = childRoles[index];
for (let index = 0; index < childRoles.length; index++) {
const childRole = childRoles[index];
if (
owns(node, virtualNode, childRole) ||
ariaOwns(ownedElements, childRole)
Expand All @@ -66,8 +63,8 @@ function missingRequiredChildren(node, childRoles, all, role) {
// combobox exceptions
if (role === 'combobox') {
// remove 'textbox' from missing roles if combobox is a native text-type input or owns a 'searchbox'
var textboxIndex = missing.indexOf('textbox');
var textTypeInputs = ['text', 'search', 'email', 'url', 'tel'];
const textboxIndex = missing.indexOf('textbox');
const textTypeInputs = ['text', 'search', 'email', 'url', 'tel'];
if (
(textboxIndex >= 0 &&
(node.nodeName.toUpperCase() === 'INPUT' &&
Expand All @@ -78,11 +75,25 @@ function missingRequiredChildren(node, childRoles, all, role) {
missing.splice(textboxIndex, 1);
}

// remove 'listbox' from missing roles if combobox is collapsed
var listboxIndex = missing.indexOf('listbox');
var expanded = node.getAttribute('aria-expanded');
if (listboxIndex >= 0 && (!expanded || expanded === 'false')) {
missing.splice(listboxIndex, 1);
const expandedChildRoles = ['listbox', 'tree', 'grid', 'dialog'];
const expandedValue = node.getAttribute('aria-expanded');
const expanded = expandedValue && expandedValue !== 'false';
const popupRole = (
node.getAttribute('aria-haspopup') || 'listbox'
).toLowerCase();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please test this too.


for (let index = 0; index < expandedChildRoles.length; index++) {
const expandedChildRole = expandedChildRoles[index];
// keep the specified popup type required if expanded
if (expanded && expandedChildRole === popupRole) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should probably make this case insensitive.

continue;
}

// remove 'listbox' and company from missing roles if combobox is collapsed
const missingIndex = missing.indexOf(expandedChildRole);
if (missingIndex >= 0) {
missing.splice(missingIndex, 1);
}
}
}

Expand All @@ -108,21 +119,21 @@ function hasDecendantWithRole(node) {
);
}

var role = node.getAttribute('role');
var required = requiredOwned(role);
const role = node.getAttribute('role');
const required = requiredOwned(role);

if (!required) {
return true;
}

var all = false;
var childRoles = required.one;
let all = false;
let childRoles = required.one;
if (!childRoles) {
var all = true;
all = true;
childRoles = required.all;
}

var missing = missingRequiredChildren(node, childRoles, all, role);
const missing = missingRequiredChildren(node, childRoles, all, role);

if (!missing) {
return true;
Expand Down
2 changes: 1 addition & 1 deletion lib/commons/aria/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -433,7 +433,7 @@ lookupTable.role = {
required: ['aria-expanded']
},
owned: {
all: ['listbox', 'textbox']
all: ['listbox', 'tree', 'grid', 'dialog', 'textbox']
},
nameFrom: ['author'],
context: null,
Expand Down
31 changes: 31 additions & 0 deletions test/checks/aria/required-children.js
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,37 @@ describe('aria-required-children', function() {
);
});

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(
checks['aria-required-children'].evaluate.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(
checks['aria-required-children'].evaluate.apply(checkContext, params)
);

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

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(
checks['aria-required-children'].evaluate.apply(checkContext, params)
);

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

it('should pass one indirectly aria-owned child when one required', function() {
var params = checkSetup(
'<div role="grid" id="target" aria-owns="r"></div><div id="r"><div role="row">Nothing here.</div></div>'
Expand Down