Skip to content

Commit

Permalink
fix: Allow divs as groups in dl (#1076)
Browse files Browse the repository at this point in the history
Allow div elements to group content inside of dl elements.

Closes #262

## 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: Jey
  • Loading branch information
WilcoFiers committed Aug 22, 2018
1 parent 3b05fee commit f4f6df6
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 4 deletions.
21 changes: 17 additions & 4 deletions lib/checks/lists/only-dlitems.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,20 +10,33 @@ const ALLOWED_TAGS = [
];

const ALLOWED_ROLES = ['definition', 'term', 'list'];
const { getRole } = axe.commons.aria;

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

const result = virtualNode.children.reduce((out, childNode) => {
const content = virtualNode.children.reduce((content, child) => {
const { actualNode } = child;
if (
actualNode.nodeName.toUpperCase() === 'DIV' &&
getRole(actualNode) === null
) {
return content.concat(child.children);
}
return content.concat(child);
}, []);

const result = content.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)) {
const explicitRole = getRole(actualNode, { noImplicit: true });

if ((tagName !== 'DT' && tagName !== 'DD') || explicitRole) {
if (!ALLOWED_ROLES.includes(explicitRole)) {
// handle comment - https://github.com/dequelabs/axe-core/pull/518/files#r139284668
out.badNodes.push(actualNode);
}
Expand Down
18 changes: 18 additions & 0 deletions test/checks/lists/only-dlitems.js
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,24 @@ describe('only-dlitems', function() {
);
});

it('should return false if the list has dt and dd inside a div group', function() {
var checkArgs = checkSetup(
'<dl id="target"><div><dt>An item</dt><dd>A list</dd></div></dl>'
);
assert.isFalse(
checks['only-dlitems'].evaluate.apply(checkContext, checkArgs)
);
});

it('should return true if the list has dt and dd inside a div group with a role', function() {
var checkArgs = checkSetup(
'<dl id="target"><div role="listitem"><dt>An item</dt><dd>A list</dd></div></dl>'
);
assert.isTrue(
checks['only-dlitems'].evaluate.apply(checkContext, checkArgs)
);
});

(shadowSupport.v1 ? it : xit)(
'should return false in a shadow DOM pass',
function() {
Expand Down

0 comments on commit f4f6df6

Please sign in to comment.