Skip to content

Commit

Permalink
fix: Allow only-dlitem / only-listitem to have any hidden content (#1098
Browse files Browse the repository at this point in the history
)

This change allows any hidden element in ul, ol and dl elements.

Closes #1021 

## Reviewer checks

**Required fields, to be filled out by PR reviewer(s)**
- [ ] Follows the commit message policy, appropriate for next version
- [ ] Has documentation updated, a DU ticket, or requires no documentation change
- [ ] Includes new tests, or was unnecessary
- [ ] Code is reviewed for security by: << Name here >>
  • Loading branch information
WilcoFiers authored Aug 27, 2018
1 parent 24dd39c commit 6034aae
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 46 deletions.
20 changes: 4 additions & 16 deletions lib/checks/lists/only-dlitems.js
Original file line number Diff line number Diff line change
@@ -1,17 +1,5 @@
const ALLOWED_TAGS = [
'STYLE',
'META',
'LINK',
'MAP',
'AREA',
'SCRIPT',
'DATALIST',
'TEMPLATE'
];

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

let base = {
badNodes: [],
hasNonEmptyTextNode: false
Expand All @@ -21,7 +9,7 @@ const content = virtualNode.children.reduce((content, child) => {
const { actualNode } = child;
if (
actualNode.nodeName.toUpperCase() === 'DIV' &&
getRole(actualNode) === null
aria.getRole(actualNode) === null
) {
return content.concat(child.children);
}
Expand All @@ -32,8 +20,8 @@ const result = content.reduce((out, childNode) => {
const { actualNode } = childNode;
const tagName = actualNode.nodeName.toUpperCase();

if (actualNode.nodeType === 1 && !ALLOWED_TAGS.includes(tagName)) {
const explicitRole = getRole(actualNode, { noImplicit: true });
if (actualNode.nodeType === 1 && dom.isVisible(actualNode, true, false)) {
const explicitRole = aria.getRole(actualNode, { noImplicit: true });

if ((tagName !== 'DT' && tagName !== 'DD') || explicitRole) {
if (!ALLOWED_ROLES.includes(explicitRole)) {
Expand Down
44 changes: 14 additions & 30 deletions lib/checks/lists/only-listitems.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,4 @@
const ALLOWED_TAGS = [
'STYLE',
'META',
'LINK',
'MAP',
'AREA',
'SCRIPT',
'DATALIST',
'TEMPLATE'
];

const { dom } = axe.commons;
const getIsListItemRole = (role, tagName) => {
return role === 'listitem' || (tagName === 'LI' && !role);
};
Expand All @@ -28,30 +18,24 @@ let base = {
let out = virtualNode.children.reduce((out, { actualNode }) => {
/*eslint
max-statements: ["error", 20]
complexity: ["error", 11]
complexity: ["error", 12]
*/
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);
if (actualNode.nodeType === 1 && dom.isVisible(actualNode, true, false)) {
const role = (actualNode.getAttribute('role') || '').toLowerCase();
const isListItemRole = getIsListItemRole(role, tagName);

out.hasListItem = getHasListItem(
out.hasListItem,
tagName,
isListItemRole
);
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 (isListItemRole) {
out.isEmpty = false;
}
if (tagName === 'LI' && !isListItemRole) {
out.liItemsWithRole++;
}
if (tagName !== 'LI' && !isListItemRole) {
out.badNodes.push(actualNode);
}
}
if (actualNode.nodeType === 3) {
Expand Down
36 changes: 36 additions & 0 deletions test/checks/lists/only-dlitems.js
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,42 @@ describe('only-dlitems', function() {
);
});

it('returns false if there are display:none elements that normally would not be allowed', function() {
var checkArgs = checkSetup(
'<dl id="target"> <dt>An item</dt> <dd>A list</dd> <h1 style="display:none">heading</h1> </dl>'
);
assert.isFalse(
checks['only-dlitems'].evaluate.apply(checkContext, checkArgs)
);
});

it('returns false if there are visibility:hidden elements that normally would not be allowed', function() {
var checkArgs = checkSetup(
'<dl id="target"> <dt>An item</dt> <dd>A list</dd> <h1 style="visibility:hidden">heading</h1> </dl>'
);
assert.isFalse(
checks['only-dlitems'].evaluate.apply(checkContext, checkArgs)
);
});

it('returns false if there are aria-hidden=true elements that normally would not be allowed', function() {
var checkArgs = checkSetup(
'<dl id="target"> <dt>An item</dt> <dd>A list</dd> <h1 aria-hidden="true">heading</h1> </dl>'
);
assert.isFalse(
checks['only-dlitems'].evaluate.apply(checkContext, checkArgs)
);
});

it('returns true if there are aria-hidden=false elements that normally would not be allowed', function() {
var checkArgs = checkSetup(
'<dl id="target"> <dt>An item</dt> <dd>A list</dd> <h1 aria-hidden="false">heading</h1> </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
36 changes: 36 additions & 0 deletions test/checks/lists/only-listitems.js
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,42 @@ describe('only-listitems', function() {
);
});

it('returns false if there are display:none elements that normally would not be allowed', function() {
var checkArgs = checkSetup(
'<ul id="target"> <li>An item</li> <h1 style="display:none">heading</h1> </ul>'
);
assert.isFalse(
checks['only-listitems'].evaluate.apply(checkContext, checkArgs)
);
});

it('returns false if there are visibility:hidden elements that normally would not be allowed', function() {
var checkArgs = checkSetup(
'<ul id="target"> <li>An item</li> <h1 style="visibility:hidden">heading</h1> </ul>'
);
assert.isFalse(
checks['only-listitems'].evaluate.apply(checkContext, checkArgs)
);
});

it('returns false if there are aria-hidden=true elements that normally would not be allowed', function() {
var checkArgs = checkSetup(
'<ul id="target"> <li>An item</li> <h1 aria-hidden="true">heading</h1> </ul>'
);
assert.isFalse(
checks['only-listitems'].evaluate.apply(checkContext, checkArgs)
);
});

it('returns true if there are aria-hidden=false elements that normally would not be allowed', function() {
var checkArgs = checkSetup(
'<ul id="target"> <li>An item</li> <h1 aria-hidden="false">heading</h1> </ul>'
);
assert.isTrue(
checks['only-listitems'].evaluate.apply(checkContext, checkArgs)
);
});

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

0 comments on commit 6034aae

Please sign in to comment.