Skip to content

Commit

Permalink
fix(listitem): allow as child of menu (#3286)
Browse files Browse the repository at this point in the history
  • Loading branch information
WilcoFiers authored Nov 24, 2021
1 parent f23d8c8 commit 4bf7d35
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 50 deletions.
16 changes: 6 additions & 10 deletions lib/checks/lists/listitem-evaluate.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
import { getComposedParent } from '../../commons/dom';
import { isValidRole } from '../../commons/aria';
import { isValidRole, getExplicitRole } from '../../commons/aria';

function listitemEvaluate(node) {
const parent = getComposedParent(node);
export default function listitemEvaluate(node, options, virtualNode) {
const { parent } = virtualNode;
if (!parent) {
// Can only happen with detached DOM nodes and roots:
return undefined;
}

const parentTagName = parent.nodeName.toUpperCase();
const parentRole = (parent.getAttribute('role') || '').toLowerCase();
const parentNodeName = parent.props.nodeName;
const parentRole = getExplicitRole(parent);

if (['presentation', 'none', 'list'].includes(parentRole)) {
return true;
Expand All @@ -21,8 +20,5 @@ function listitemEvaluate(node) {
});
return false;
}

return ['UL', 'OL'].includes(parentTagName);
return ['ul', 'ol', 'menu'].includes(parentNodeName);
}

export default listitemEvaluate;
89 changes: 50 additions & 39 deletions test/checks/lists/listitem.js
Original file line number Diff line number Diff line change
@@ -1,74 +1,81 @@
describe('listitem', function() {
'use strict';

var fixture = document.getElementById('fixture');
var shadowSupport = axe.testUtils.shadowSupport;
var checkContext = axe.testUtils.MockCheckContext();
var checkSetup = axe.testUtils.checkSetup;
var fixtureSetup = axe.testUtils.fixtureSetup;
var checkEvaluate = axe.testUtils.getCheckEvaluate('listitem');

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

it('should pass if the listitem has a parent <ol>', function() {
fixture.innerHTML = '<ol><li id="target">My list item</li></ol>';
var target = fixture.querySelector('#target');
assert.isTrue(checks.listitem.evaluate.call(checkContext, target));
var params = checkSetup('<ol><li id="target">My list item</li></ol>');
var result = checkEvaluate.apply(checkContext, params)
assert.isTrue(result);
});

it('should pass if the listitem has a parent <ul>', function() {
fixture.innerHTML = '<ul><li id="target">My list item</li></ul>';
var target = fixture.querySelector('#target');
assert.isTrue(checks.listitem.evaluate.call(checkContext, target));
var params = checkSetup('<ul><li id="target">My list item</li></ul>');
var result = checkEvaluate.apply(checkContext, params)
assert.isTrue(result);
});

it('should pass if the listitem has a parent role=list', function() {
fixture.innerHTML =
'<div role="list"><li id="target">My list item</li></div>';
var target = fixture.querySelector('#target');
assert.isTrue(checks.listitem.evaluate.call(checkContext, target));
var params = checkSetup(
'<div role="list"><li id="target">My list item</li></div>'
);
var result = checkEvaluate.apply(checkContext, params)
assert.isTrue(result);
});

it('should pass if the listitem has a parent role=none', function() {
fixture.innerHTML =
'<ul role="none"><li id="target">My list item</li></ul>';
var target = fixture.querySelector('#target');
assert.isTrue(checks.listitem.evaluate.call(checkContext, target));
var params = checkSetup(
'<ul role="none"><li id="target">My list item</li></ul>'
);
var result = checkEvaluate.apply(checkContext, params)
assert.isTrue(result);
});

it('should pass if the listitem has a parent role=presentation', function() {
fixture.innerHTML =
'<ul role="presentation"><li id="target">My list item</li></ul>';
var target = fixture.querySelector('#target');
assert.isTrue(checks.listitem.evaluate.call(checkContext, target));
var params = checkSetup(
'<ul role="presentation"><li id="target">My list item</li></ul>'
);
var result = checkEvaluate.apply(checkContext, params)
assert.isTrue(result);
});

it('should fail if the listitem has an incorrect parent', function() {
fixture.innerHTML = '<div><li id="target">My list item</li></div>';
var target = fixture.querySelector('#target');
assert.isFalse(checks.listitem.evaluate.call(checkContext, target));
var params = checkSetup('<div><li id="target">My list item</li></div>');
var result = checkEvaluate.apply(checkContext, params)
assert.isFalse(result);
});

it('should fail if the listitem has a parent <ol> with changed role', function() {
fixture.innerHTML =
'<ol role="menubar"><li id="target">My list item</li></ol>';
var target = fixture.querySelector('#target');
assert.isFalse(checks.listitem.evaluate.call(checkContext, target));
var params = checkSetup(
'<ol role="menubar"><li id="target">My list item</li></ol>'
);
var result = checkEvaluate.apply(checkContext, params)
assert.isFalse(result);
assert.equal(checkContext._data.messageKey, 'roleNotValid');
});

it('should pass if the listitem has a parent <ol> with an invalid role', function() {
fixture.innerHTML =
'<ol role="invalid-role"><li id="target">My list item</li></ol>';
var target = fixture.querySelector('#target');
assert.isTrue(checks.listitem.evaluate.call(checkContext, target));
var params = checkSetup(
'<ol role="invalid-role"><li id="target">My list item</li></ol>'
);
var result = checkEvaluate.apply(checkContext, params)
assert.isTrue(result);
});

it('should pass if the listitem has a parent <ol> with an abstract role', function() {
fixture.innerHTML =
'<ol role="section"><li id="target">My list item</li></ol>';
var target = fixture.querySelector('#target');
assert.isTrue(checks.listitem.evaluate.call(checkContext, target));
var params = checkSetup(
'<ol role="section"><li id="target">My list item</li></ol>'
);
var result = checkEvaluate.apply(checkContext, params)
assert.isTrue(result);
});

(shadowSupport.v1 ? it : xit)(
Expand All @@ -78,9 +85,11 @@ describe('listitem', function() {
node.innerHTML = '<li id="target">My list item </li>';
var shadow = node.attachShadow({ mode: 'open' });
shadow.innerHTML = '<ul><slot></slot></ul>';
fixture.appendChild(node);
fixtureSetup(node);
var target = node.querySelector('#target');
assert.isTrue(checks.listitem.evaluate.call(checkContext, target));
var virtualTarget = axe.utils.getNodeFromTree(target);
var result = checkEvaluate.apply(checkContext, [target, {}, virtualTarget])
assert.isTrue(result);
}
);

Expand All @@ -91,9 +100,11 @@ describe('listitem', function() {
node.innerHTML = '<li id="target">My list item </li>';
var shadow = node.attachShadow({ mode: 'open' });
shadow.innerHTML = '<div><slot></slot></div>';
fixture.appendChild(node);
fixtureSetup(node);
var target = node.querySelector('#target');
assert.isFalse(checks.listitem.evaluate.call(checkContext, target));
var virtualTarget = axe.utils.getNodeFromTree(target);
var result = checkEvaluate.apply(checkContext, [target, {}, virtualTarget])
assert.isFalse(result);
}
);
});
4 changes: 4 additions & 0 deletions test/integration/rules/listitem/listitem.html
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,7 @@
<ol role="menubar">
<li id="olrolechanged">I too do not belong to a list.</li>
</ol>

<menu>
<li id="menuitem"><button>Copy</button></li>
</menu>
2 changes: 1 addition & 1 deletion test/integration/rules/listitem/listitem.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@
"description": "listitem test",
"rule": "listitem",
"violations": [["#uncontained"], ["#ulrolechanged"], ["#olrolechanged"]],
"passes": [["#contained"], ["#alsocontained"], ["#presentation"], ["#none"]]
"passes": [["#contained"], ["#alsocontained"], ["#presentation"], ["#none"], ["#menuitem"]]
}

0 comments on commit 4bf7d35

Please sign in to comment.