Skip to content

Commit

Permalink
fix(required-children): consider overriding descendant role(s)… (dequ…
Browse files Browse the repository at this point in the history 
…elabs#2131)
  • Loading branch information
@jeeyyy @paulirish
jeeyyy authored and paulirish committed Jun 5, 2020
1 parent 02b6c13 commit b6d24ab
Show file tree
Hide file tree
Showing 5 changed files with 263 additions and 86 deletions.
2 changes: 1 addition & 1 deletion lib/checks/.eslintrc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
"strict": 0,
"max-params": [
2,
5
6
],
"max-depth": [
2,
Expand Down
223 changes: 144 additions & 79 deletions lib/checks/aria/required-children.js
View file
Original file line number Diff line number Diff line change
@@ -1,58 +1,87 @@
const requiredOwned = axe.commons.aria.requiredOwned;
const implicitNodes = axe.commons.aria.implicitNodes;
const matchesSelector = axe.utils.matchesSelector;
const idrefs = axe.commons.dom.idrefs;
const hasContentVirtual = axe.commons.dom.hasContentVirtual;
const { aria, dom } = axe.commons;
const { requiredOwned, implicitNodes, getRole } = aria;
const { hasContentVirtual, idrefs } = dom;
const { matchesSelector, querySelectorAll, getNodeFromTree } = axe.utils;

const reviewEmpty =
options && Array.isArray(options.reviewEmpty) ? options.reviewEmpty : [];
const role = node.getAttribute('role');
const required = requiredOwned(role);
if (!required) {
return true;
}

function owns(node, virtualTree, role, ariaOwned) {
if (node === null) {
return false;
}
const implicit = implicitNodes(role);
let selector = ['[role="' + role + '"]'];

if (implicit) {
selector = selector.concat(
implicit.map(implicitSelector => implicitSelector + ':not([role])')
);
}
let all = false;
let childRoles = required.one;
if (!childRoles) {
all = true;
childRoles = required.all;
}

selector = selector.join(',');
return ariaOwned
? matchesSelector(node, selector) ||
!!axe.utils.querySelectorAll(virtualTree, selector)[0]
: !!axe.utils.querySelectorAll(virtualTree, selector)[0];
const ownedElements = idrefs(node, 'aria-owns');
const descendantRole = getDescendantRole(node, ownedElements);
const missing = missingRequiredChildren(
node,
childRoles,
all,
role,
ownedElements,
descendantRole
);
if (!missing) {
return true;
}

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

// Only review empty nodes when a node is both empty and does not have an aria-owns relationship
if (
reviewEmpty.includes(role) &&
!hasContentVirtual(virtualNode, false, true) &&
!descendantRole.length &&
idrefs(node, 'aria-owns').length === 0
) {
return undefined;
}

function missingRequiredChildren(node, childRoles, all, role) {
const missing = [],
ownedElements = idrefs(node, 'aria-owns');
return false;

/**
* Get missing children roles
* @param {HTMLElement} node node
* @param {String[]} childRoles expected children roles
* @param {Boolean} all should all child roles be present?
* @param {String} role role of given node
*/
function missingRequiredChildren(
node,
childRoles,
all,
role,
ownedEls,
descRole
) {
let missing = [];

for (let index = 0; index < childRoles.length; index++) {
const childRole = childRoles[index];
if (
owns(node, virtualNode, childRole) ||
ariaOwns(ownedElements, childRole)
) {
const ownsRole = owns(node, virtualNode, childRole);
const ariaOwnsRole = ariaOwns(ownedEls, childRole);
if (ownsRole || ariaOwnsRole) {
if (!all) {
return null;
}

/**
* Verify if descendants contain one of the requested child roles & that a requested child role is not nested within an overriding role
* Only handle when role is not `combobox`, given there is an exception/ different path for `combobox`
* Eg:
* `<div role="list"><div role="tabpanel"><div role="listitem">List item 1</div></div></div>`
* should fail because `listitem` role not under `list` but has `tabpanel` between them, so although `listitem` is owned by `list` this is a failure.
*/
if (role !== 'combobox' && !descRole.includes(childRole)) {
missing.push(childRole);
}
} else {
if (all) {
missing.push(childRole);
Expand All @@ -70,7 +99,7 @@ function missingRequiredChildren(node, childRoles, all, role) {
node.nodeName.toUpperCase() === 'INPUT' &&
textTypeInputs.includes(node.type)) ||
owns(node, virtualNode, 'searchbox') ||
ariaOwns(ownedElements, 'searchbox')
ariaOwns(ownedEls, 'searchbox')
) {
missing.splice(textboxIndex, 1);
}
Expand Down Expand Up @@ -106,49 +135,85 @@ function missingRequiredChildren(node, childRoles, all, role) {
return null;
}

function hasDecendantWithRole(node) {
return (
node.children &&
node.children.some(child => {
const role = axe.commons.aria.getRole(child);
return (
!['presentation', 'none', null].includes(role) ||
hasDecendantWithRole(child)
);
})
);
}

const role = node.getAttribute('role');
const required = requiredOwned(role);
/**
* Helper to check if a given node owns an element with a given role
* @param {HTMLElement} node node
* @param {Object} virtualTree virtual node
* @param {String} role role
* @param {Boolean} ariaOwned
* @returns {Boolean}
*/
function owns(node, virtualTree, role, ariaOwned) {
if (node === null) {
return false;
}
const implicit = implicitNodes(role);
let selector = ['[role="' + role + '"]'];

if (!required) {
return true;
}
if (implicit) {
selector = selector.concat(
implicit.map(implicitSelector => implicitSelector + ':not([role])')
);
}

let all = false;
let childRoles = required.one;
if (!childRoles) {
all = true;
childRoles = required.all;
selector = selector.join(',');
return ariaOwned
? matchesSelector(node, selector) ||
!!querySelectorAll(virtualTree, selector)[0]
: !!querySelectorAll(virtualTree, selector)[0];
}

const missing = missingRequiredChildren(node, childRoles, all, role);
/**
* Helper to check if a given node is `aria-owns` an element with a given role
* @param {HTMLElement[]} nodes nodes
* @param {String} role role
* @returns {Boolean}
*/
function ariaOwns(nodes, role) {
for (let index = 0; index < nodes.length; index++) {
const node = nodes[index];
if (node === null) {
continue;
}

if (!missing) {
return true;
const virtualTree = getNodeFromTree(node);
if (owns(node, virtualTree, role, true)) {
return true;
}
}
return false;
}

this.data(missing);
/**
* Get role (that is not presentation or none) of descendant
* @param {HTMLElement} node node
* @returns {String[]}
*/
function getDescendantRole(node, ownedEls) {
const isOwns = ownedEls && ownedEls.length > 0;
const el = isOwns ? ownedEls[0] : node;

if (!el) {
return [];
}

// Only review empty nodes when a node is both empty and does not have an aria-owns relationship
if (
reviewEmpty.includes(role) &&
!hasContentVirtual(virtualNode, false, true) &&
!hasDecendantWithRole(virtualNode) &&
idrefs(node, 'aria-owns').length === 0
) {
return undefined;
} else {
return false;
const items = isOwns
? Array.from(el.children).reduce(
(out, child) => {
out.push(child);
return out;
},
[el]
)
: Array.from(el.children);

return items.reduce((out, child) => {
const role = getRole(child);
if (['presentation', 'none', null].includes(role)) {
out = out.concat(getDescendantRole(child));
} else {
out.push(role);
}
return out;
}, []);
}
69 changes: 69 additions & 0 deletions test/checks/aria/required-children.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,15 @@ describe('aria-required-children', function() {
);
});

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

it('should return undefined when element is empty and is in reviewEmpty options', function() {
var params = checkSetup('<div role="list" id="target"></div>', {
reviewEmpty: ['list']
Expand Down Expand Up @@ -246,6 +255,39 @@ describe('aria-required-children', function() {
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>'
);
assert.isFalse(
checks['aria-required-children'].evaluate.apply(checkContext, params)
);

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

it('should fail when list has intermediate child with role that is not a required role', function() {
var params = checkSetup(
'<div id="target" role="list"><div role="tabpanel"><div role="listitem">List item 1</div></div></div>'
);
assert.isFalse(
checks['aria-required-children'].evaluate.apply(checkContext, params)
);

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

it('should fail when nested child with role row does not have required child role cell', function() {
var params = checkSetup(
'<div role="grid"><div role="row" id="target"><span>Item 1</span></div></div>'
);
assert.isFalse(
checks['aria-required-children'].evaluate.apply(checkContext, params)
);

assert.includeMembers(checkContext._data, ['cell']);
});

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 Expand Up @@ -273,6 +315,33 @@ describe('aria-required-children', function() {
);
});

it('should fail one existing aria-owned child when an intermediate child with role that is not a required role exists', function() {
var params = checkSetup(
'<div id="target" role="list" aria-owns="list"></div><div id="list"><div role="tabpanel"><div role="listitem"></div></div></div>'
);
assert.isFalse(
checks['aria-required-children'].evaluate.apply(checkContext, params)
);
});

it('should pass one existing required child when one required (has explicit role of tab)', function() {
var params = checkSetup(
'<ul id="target" role="tablist"><li role="tab">Tab 1</li><li role="tab">Tab 2</li></ul>'
);
assert.isTrue(
checks['aria-required-children'].evaluate.apply(checkContext, params)
);
});

it('should pass required child roles (grid contains row, which contains cell)', function() {
var params = checkSetup(
'<table id="target" role="grid"><tr role="row"><td role="cell">Item 1</td></tr></table>'
);
assert.isTrue(
checks['aria-required-children'].evaluate.apply(checkContext, params)
);
});

it('should pass one existing required child when one required', function() {
var params = checkSetup(
'<div role="grid" id="target"><p role="row">Nothing here.</p></div>'
Expand Down
Loading

0 comments on commit b6d24ab

Please sign in to comment.