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.

Sign up for GitHub

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

Jump to bottom

fix(required-children): consider overriding descendant role(s) when validating required child role(s) #2131

Merged
merged 6 commits into from
Apr 7, 2020
Merged

fix(required-children): consider overriding descendant role(s) when validating required child role(s) #2131

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
de9ba3a
fix(required-children): handle descendant role when checking for requ…
jeeyyy Mar 25, 2020
00829e3
test: add integration tests
jeeyyy Mar 25, 2020
c424736
update check and test
jeeyyy Apr 1, 2020
ad395e1
update for ie
jeeyyy Apr 1, 2020
d4ff9e1
remove usage of spread operator
jeeyyy Apr 2, 2020
ef9a849
update helper fn
jeeyyy Apr 2, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
196 changes: 119 additions & 77 deletions lib/checks/aria/required-children.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,58 +1,77 @@
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 missing = missingRequiredChildren(node, childRoles, all, role);
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) &&
!getRolesOfDescendant(node).length &&
idrefs(node, 'aria-owns').length === 0
) {
return undefined;
}

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) {
const missing = [],
ownedElements = idrefs(node, 'aria-owns');
let missing = [];
const ownedElements = idrefs(node, 'aria-owns');
const descendantRoles = getRolesOfDescendant(node);

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(ownedElements, 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 (
!ariaOwnsRole &&
role !== 'combobox' &&
!descendantRoles.includes(childRole)
) {
missing.push(childRole);
}
} else {
if (all) {
missing.push(childRole);
Expand Down Expand Up @@ -106,49 +125,72 @@ 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 roles of all descendants for a given node
* @param {HTMLElement} node node
* @returns {String[]}
*/
function getRolesOfDescendant(node) {
if (!node || !node.children) {
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;
return Array.from(node.children).reduce((out, child) => {
const role = getRole(child);
if (['presentation', 'none', null].includes(role)) {
out.push(...getRolesOfDescendant(child));
} else {
out.push(role);
}
return out;
}, []);
}
60 changes: 60 additions & 0 deletions test/checks/aria/required-children.js
View file
Open in desktop
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,24 @@ describe('aria-required-children', function() {
);
});

it('should pass one existing aria-owned child when one required', 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 one existing aria-owned child when one required', function() {
jeeyyy marked this conversation as resolved.
Show resolved Hide resolved
var params = checkSetup(
'<table id="target" role="grid"><tr role="row"><span role="cell">Item 1</span></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
35 changes: 31 additions & 4 deletions test/integration/rules/aria-required-children/aria-required-children.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,20 @@
<div role="list" id="pass1"><div role="listitem" id="pass2">Item 1</div></div>
<div role="list" id="pass1">
<div role="listitem" id="pass2">Item 1</div>
</div>
<div role="list" id="pass3" aria-owns="pass4"></div>
<div role="listitem" id="pass4"></div>
<div role="list" id="fail1"><div role="menuitem" id="pass5"></div></div>
<div role="list" id="fail1">
<div role="menuitem" id="pass5"></div>
</div>
<div role="list" id="fail2" aria-owns="pass6"></div>
<div role="menu" id="fail3"></div>
<div role="menubar" id="fail4"></div>
<div role="row" id="fail5"></div>
<div role="menuitem" id="pass6"></div>
<div role="list" id="pass7" aria-owns="parent"></div>
<div id="parent"><div role="listitem" id="pass8"></div></div>
<div id="parent">
<div role="listitem" id="pass8"></div>
</div>
<div role="grid" id="incomplete1"></div>
<div role="list" id="incomplete2"></div>
<div role="listbox" id="incomplete3"></div>
Expand All @@ -19,4 +25,25 @@
<div role="rowgroup" id="incomplete8"></div>
<div role="doc-bibliography" id="incomplete9"></div>
<div role="doc-endnotes" id="incomplete10"></div>
<div role="listbox" id="incomplete11"><div></div></div>
<div role="listbox" id="incomplete11">
<div></div>
</div>
<div role="list" id="fail6">
<div role="tabpanel" id="pass9">
<div role="listitem" id="pass10">List item 1</div>
</div>
</div>
<div role="grid" id="pass11">
<div role="row" id="fail7">
<span>Item 1</span>
</div>
</div>
<div role="grid" id="pass12">
<div role="presentation" id="pass13">
<div role="row" id="pass14">
<div role="none" id="pass15">
<span role="cell" id="pass16">Item 1</span>
</div>
</div>
</div>
</div>
20 changes: 18 additions & 2 deletions test/integration/rules/aria-required-children/aria-required-children.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,15 @@
{
"description": "aria-required-children test",
"rule": "aria-required-children",
"violations": [["#fail1"], ["#fail2"], ["#fail3"], ["#fail4"], ["#fail5"]],
"violations": [
["#fail1"],
["#fail2"],
["#fail3"],
["#fail4"],
["#fail5"],
["#fail6"],
["#fail7"]
],
"passes": [
["#pass1"],
["#pass2"],
Expand All @@ -10,7 +18,15 @@
["#pass5"],
["#pass6"],
["#pass7"],
["#pass8"]
["#pass8"],
["#pass9"],
["#pass10"],
["#pass11"],
["#pass12"],
["#pass13"],
["#pass14"],
["#pass15"],
["#pass16"]
],
"incomplete": [
["#incomplete1"],
Expand Down