Skip to content

Commit

Permalink
fix(rule): update for lists, list-items to handle role changes. (#949)
Browse files Browse the repository at this point in the history
This rule updates list(items) to cater to role changes to either parent or child list elements.

This rule is a taken over PR from the community contribution (PR: #518 @AlmeroSteyn - thanks for the initial work).

Closes issue:
- #463

## Reviewer checks

**Required fields, to be filled out by PR reviewer(s)**
- [x] Follows the commit message policy, appropriate for next version
- [x] Has documentation updated, a DU ticket, or requires no documentation change
- [x] Includes new tests, or was unnecessary
- [x] Code is reviewed for security by: @WilcoFiers
  • Loading branch information
jeeyyy authored and WilcoFiers committed Jul 18, 2018
1 parent 1c0408a commit 3a8729b
Show file tree
Hide file tree
Showing 18 changed files with 477 additions and 369 deletions.
24 changes: 22 additions & 2 deletions lib/checks/lists/dlitem.js
Original file line number Diff line number Diff line change
@@ -1,2 +1,22 @@
var parent = axe.commons.dom.getComposedParent(node);
return parent.nodeName.toUpperCase() === 'DL';
const parent = axe.commons.dom.getComposedParent(node);
const parentTagName = parent.nodeName.toUpperCase();

if (parentTagName !== 'DL') {
return false;
}

const parentRole = (parent.getAttribute('role') || '').toLowerCase();

if (!parentRole) {
return true;
}

if (!axe.commons.aria.isValidRole(parentRole)) {
return false;
}

if (parentRole === 'list') {
return true;
}

return false;
19 changes: 16 additions & 3 deletions lib/checks/lists/listitem.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,18 @@
var parent = axe.commons.dom.getComposedParent(node);
const parent = axe.commons.dom.getComposedParent(node);
if (!parent) {
return false;
}

const ALLOWED_TAGS = ['UL', 'OL'];
const parentTagName = parent.nodeName.toUpperCase();

const parentRole = (parent.getAttribute('role') || '').toLowerCase();
if (parentRole && !axe.commons.aria.isValidRole(parentRole)) {
return false;
}

const isListRole = parentRole === 'list';
return (
['UL', 'OL'].includes(parent.nodeName.toUpperCase()) ||
(parent.getAttribute('role') || '').toLowerCase() === 'list'
(ALLOWED_TAGS.includes(parentTagName) && (!parentRole || isListRole)) ||
isListRole
);
62 changes: 35 additions & 27 deletions lib/checks/lists/only-dlitems.js
Original file line number Diff line number Diff line change
@@ -1,33 +1,41 @@
var bad = [],
permitted = [
'STYLE',
'META',
'LINK',
'MAP',
'AREA',
'SCRIPT',
'DATALIST',
'TEMPLATE'
],
hasNonEmptyTextNode = false;
const ALLOWED_TAGS = [
'STYLE',
'META',
'LINK',
'MAP',
'AREA',
'SCRIPT',
'DATALIST',
'TEMPLATE'
];

virtualNode.children.forEach(({ actualNode }) => {
var nodeName = actualNode.nodeName.toUpperCase();
if (
actualNode.nodeType === 1 &&
nodeName !== 'DT' &&
nodeName !== 'DD' &&
permitted.indexOf(nodeName) === -1
) {
bad.push(actualNode);
const ALLOWED_ROLES = ['definition', 'term', 'list'];

let base = {
badNodes: [],
hasNonEmptyTextNode: false
};

const result = virtualNode.children.reduce((out, childNode) => {
const { actualNode } = childNode;
const tagName = actualNode.nodeName.toUpperCase();

if (actualNode.nodeType === 1 && !ALLOWED_TAGS.includes(tagName)) {
const role = (actualNode.getAttribute('role') || '').toLowerCase();
if ((tagName !== 'DT' && tagName !== 'DD') || role) {
if (!ALLOWED_ROLES.includes(role)) {
// handle comment - https://github.com/dequelabs/axe-core/pull/518/files#r139284668
out.badNodes.push(actualNode);
}
}
} else if (actualNode.nodeType === 3 && actualNode.nodeValue.trim() !== '') {
hasNonEmptyTextNode = true;
out.hasNonEmptyTextNode = true;
}
});
return out;
}, base);

if (bad.length) {
this.relatedNodes(bad);
if (result.badNodes.length) {
this.relatedNodes(result.badNodes);
}

var retVal = !!bad.length || hasNonEmptyTextNode;
return retVal;
return !!result.badNodes.length || result.hasNonEmptyTextNode;
107 changes: 80 additions & 27 deletions lib/checks/lists/only-listitems.js
Original file line number Diff line number Diff line change
@@ -1,31 +1,84 @@
var bad = [],
permitted = [
'STYLE',
'META',
'LINK',
'MAP',
'AREA',
'SCRIPT',
'DATALIST',
'TEMPLATE'
],
hasNonEmptyTextNode = false;

virtualNode.children.forEach(({ actualNode }) => {
var nodeName = actualNode.nodeName.toUpperCase();
if (
actualNode.nodeType === 1 &&
nodeName !== 'LI' &&
permitted.indexOf(nodeName) === -1
) {
bad.push(actualNode);
} else if (actualNode.nodeType === 3 && actualNode.nodeValue.trim() !== '') {
hasNonEmptyTextNode = true;
const ALLOWED_TAGS = [
'STYLE',
'META',
'LINK',
'MAP',
'AREA',
'SCRIPT',
'DATALIST',
'TEMPLATE'
];

const getIsListItemRole = (role, tagName) => {
return role === 'listitem' || (tagName === 'LI' && !role);
};

const getHasListItem = (hasListItem, tagName, isListItemRole) => {
return hasListItem || (tagName === 'LI' && isListItemRole) || isListItemRole;
};

let base = {
badNodes: [],
isEmpty: true,
hasNonEmptyTextNode: false,
hasListItem: false,
liItemsWithRole: 0
};

let out = virtualNode.children.reduce((out, { actualNode }) => {
/*eslint
max-statements: ["error", 20]
complexity: ["error", 11]
*/
const tagName = actualNode.nodeName.toUpperCase();

if (actualNode.nodeType === 1) {
if (!ALLOWED_TAGS.includes(tagName)) {
const role = (actualNode.getAttribute('role') || '').toLowerCase();
const isListItemRole = getIsListItemRole(role, tagName);

out.hasListItem = getHasListItem(
out.hasListItem,
tagName,
isListItemRole
);

if (isListItemRole) {
out.isEmpty = false;
}
if (tagName === 'LI' && !isListItemRole) {
out.liItemsWithRole++;
}
if (tagName !== 'LI' && !isListItemRole) {
out.badNodes.push(actualNode);
}
}
}
if (actualNode.nodeType === 3) {
if (actualNode.nodeValue.trim() !== '') {
out.hasNonEmptyTextNode = true;
}
}

return out;
}, base);

const virtualNodeChildrenOfTypeLi = virtualNode.children.filter(
({ actualNode }) => {
return actualNode.nodeName.toUpperCase() === 'LI';
}
});
);

const allLiItemsHaveRole =
out.liItemsWithRole > 0 &&
virtualNodeChildrenOfTypeLi.length === out.liItemsWithRole;

if (bad.length) {
this.relatedNodes(bad);
if (out.badNodes.length) {
this.relatedNodes(out.badNodes);
}

return !!bad.length || hasNonEmptyTextNode;
const isInvalidListItem = !(
out.hasListItem ||
(out.isEmpty && !allLiItemsHaveRole)
);
return isInvalidListItem || !!out.badNodes.length || out.hasNonEmptyTextNode;
5 changes: 4 additions & 1 deletion lib/core/utils/flattened-tree.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,10 @@ axe.utils.getFlattenedTree = function(node, shadowId) {
if (nodeName === 'content') {
realArray = Array.from(node.getDistributedNodes());
return realArray.reduce(reduceShadowDOM, []);
} else if (nodeName === 'slot' && typeof node.assignedNodes === 'function') {
} else if (
nodeName === 'slot' &&
typeof node.assignedNodes === 'function'
) {
realArray = Array.from(node.assignedNodes());
if (!realArray.length) {
// fallback content
Expand Down
69 changes: 40 additions & 29 deletions test/checks/lists/dlitem.js
Original file line number Diff line number Diff line change
@@ -1,49 +1,60 @@
describe('dlitem', function() {
describe('dlitem', function () {
'use strict';

var fixture = document.getElementById('fixture');
var checkSetup = axe.testUtils.checkSetup;
var shadowSupport = axe.testUtils.shadowSupport;

afterEach(function() {
afterEach(function () {
fixture.innerHTML = '';
});

it('should pass if the dlitem has a parent <dl>', function() {
it('should pass if the dlitem has a parent <dl>', function () {
var checkArgs = checkSetup('<dl><dt id="target">My list item</dl>');

assert.isTrue(checks.dlitem.evaluate.apply(null, checkArgs));
});

it('should fail if the dlitem has an incorrect parent', function() {
it('should fail if the dt element has an incorrect parent', function () {
var checkArgs = checkSetup('<video><dt id="target">My list item</video>');

assert.isFalse(checks.dlitem.evaluate.apply(null, checkArgs));
});

(shadowSupport.v1 ? it : xit)(
'should return true in a shadow DOM pass',
function() {
var node = document.createElement('div');
node.innerHTML = '<dt>My list item </dt>';
var shadow = node.attachShadow({ mode: 'open' });
shadow.innerHTML = '<dl><slot></slot></dl>';

var checkArgs = checkSetup(node, 'dt');
assert.isTrue(checks.dlitem.evaluate.apply(null, checkArgs));
}
);

(shadowSupport.v1 ? it : xit)(
'should return false in a shadow DOM fail',
function() {
var node = document.createElement('div');
node.innerHTML = '<dt>My list item </dt>';
var shadow = node.attachShadow({ mode: 'open' });
shadow.innerHTML = '<div><slot></slot></div>';

var checkArgs = checkSetup(node, 'dt');
assert.isFalse(checks.dlitem.evaluate.apply(null, checkArgs));
}
);
it('should pass if the dt element has a parent <dl> with role="list"', function () {
var checkArgs = checkSetup('<dl role="list"><dt id="target">My list item</dl>');
assert.isTrue(checks.dlitem.evaluate.apply(null, checkArgs));
})

it('should fail if the dt element has a parent <dl> with role="presentation"', function () {
var checkArgs = checkSetup('<dl role="presentation"><dt id="target">My list item</dl>');
assert.isFalse(checks.dlitem.evaluate.apply(null, checkArgs));
});

it('should fail if the dt element has a parent <dl> with a changed role', function(){
var checkArgs = checkSetup('<dl role="menubar"><dt id="target">My list item</dl>');
assert.isFalse(checks.dlitem.evaluate.apply(null, checkArgs));
});

(shadowSupport.v1 ? it : xit)('should return true in a shadow DOM pass', function () {
var node = document.createElement('div');
node.innerHTML = '<dt>My list item </dt>';
var shadow = node.attachShadow({ mode: 'open' });
shadow.innerHTML = '<dl><slot></slot></dl>';

var checkArgs = checkSetup(node, 'dt');
assert.isTrue(checks.dlitem.evaluate.apply(null, checkArgs));
});

(shadowSupport.v1 ? it : xit)('should return false in a shadow DOM fail', function () {
var node = document.createElement('div');
node.innerHTML = '<dt>My list item </dt>';
var shadow = node.attachShadow({ mode: 'open' });
shadow.innerHTML = '<div><slot></slot></div>';

var checkArgs = checkSetup(node, 'dt');
assert.isFalse(checks.dlitem.evaluate.apply(null, checkArgs));
});
});


Loading

0 comments on commit 3a8729b

Please sign in to comment.