fix: check for role changes in lists and list items

lib/checks/lists/dlitem.js

@@ -1,2 +1,3 @@
 var parent = axe.commons.dom.getComposedParent(node);
-return parent.nodeName.toUpperCase() === 'DL';
+return parent.nodeName.toUpperCase() === 'DL' && !parent.getAttribute('role');
+

lib/checks/lists/listitem.js

@@ -1,4 +1,11 @@
 var parent = axe.commons.dom.getComposedParent(node);
-return (['UL', 'OL'].includes(parent.nodeName.toUpperCase()) ||
-    (parent.getAttribute('role') || '').toLowerCase() === 'list');
-
+
+var parentRole = (parent.getAttribute('role') || '').toLowerCase();
+
+var isListRole = parentRole === 'list';
+
+return (
+	(['UL', 'OL'].includes(parent.nodeName.toUpperCase()) &&
+		(!parentRole || isListRole)) ||
+	isListRole
+);

lib/checks/lists/only-dlitems.js

@@ -4,8 +4,12 @@ var bad = [],
 
 virtualNode.children.forEach(({ actualNode }) => {
 	var nodeName = actualNode.nodeName.toUpperCase();
-	if (actualNode.nodeType === 1 && nodeName !== 'DT' && nodeName !== 'DD' && permitted.indexOf(nodeName) === -1) {
-		bad.push(actualNode);
+
+	if (actualNode.nodeType === 1 && permitted.indexOf(nodeName) === -1) {
+		var role = (actualNode.getAttribute('role') || '').toLowerCase();
+		if ((nodeName !== 'DT' && nodeName !== 'DD') || role) {
+			bad.push(actualNode);
+		}
 	} else if (actualNode.nodeType === 3 && actualNode.nodeValue.trim() !== '') {
 		hasNonEmptyTextNode = true;
 	}

lib/checks/lists/only-listitems.js

@@ -1,18 +1,41 @@
 var bad = [],
 	permitted = ['STYLE', 'META', 'LINK', 'MAP', 'AREA', 'SCRIPT', 'DATALIST', 'TEMPLATE'],
-	hasNonEmptyTextNode = false;
+	hasNonEmptyTextNode = false,
+	hasListItem = false,
+	liItemsWithRole = 0,
+	isEmpty = true;
 
 virtualNode.children.forEach(({ actualNode }) => {
 	var nodeName = actualNode.nodeName.toUpperCase();
-	if (actualNode.nodeType === 1 && nodeName !== 'LI' && permitted.indexOf(nodeName) === -1) {
-		bad.push(actualNode);
+	var isListItemRole = false;
+
+	if (actualNode.nodeType === 1 && permitted.indexOf(nodeName) === -1) {
+		var role = (actualNode.getAttribute('role') || '').toLowerCase();
+		isListItemRole = role === 'listitem' || (nodeName === 'LI' && !role);
+		hasListItem = hasListItem || (nodeName === 'LI' && isListItemRole) || isListItemRole;
+
+		if (isListItemRole) {
+			isEmpty = false;
+		}
+
+		if (nodeName === 'LI' && !isListItemRole) {
+			liItemsWithRole++;
+		}
+
+		if (nodeName !== 'LI' && !isListItemRole) {
+			bad.push(actualNode);
+		}
+
 	} else if (actualNode.nodeType === 3 && actualNode.nodeValue.trim() !== '') {
 		hasNonEmptyTextNode = true;
 	}
 });
 
+var allLiItemsHaveRole = liItemsWithRole > 0 && virtualNode.children.filter(({ actualNode }) =>
+	actualNode.nodeName === 'LI').length === liItemsWithRole;
+
 if (bad.length) {
 	this.relatedNodes(bad);
 }
 
-return !!bad.length || hasNonEmptyTextNode;
+return !(hasListItem || (isEmpty && !allLiItemsHaveRole)) || !!bad.length || hasNonEmptyTextNode;

test/checks/lists/dlitem.js

@@ -21,6 +21,12 @@ describe('dlitem', function () {
 		assert.isFalse(checks.dlitem.evaluate.apply(null, checkArgs));
 	});
 
+	it('should fail if the dlitem 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>';

test/checks/lists/listitem.js

@@ -33,6 +33,18 @@ describe('listitem', function () {
 		assert.isFalse(checks.listitem.evaluate.apply(null, checkArgs));
 	});
 
+	it('should fail if the listitem has a parent <ol> with changed role', function() {
+		var checkArgs = checkSetup('<ol role="menubar"><li id="target">My list item</li></ol>');
+
+		assert.isFalse(checks.listitem.evaluate.apply(null, checkArgs));
+	});
+
+	it('should fail if the listitem has a parent <ul> with changed role', function() {
+		var checkArgs = checkSetup('<ul role="menubar"><li id="target">My list item</li></ul>');
+
+		assert.isFalse(checks.listitem.evaluate.apply(null, checkArgs));
+	});
+
 	(shadowSupport.v1 ? it : xit)('should return true in a shadow DOM pass', function () {
 		var node = document.createElement('div');
 		node.innerHTML = '<li>My list item </li>';

test/checks/lists/only-dlitems.js

@@ -30,6 +30,18 @@ describe('only-dlitems', function () {
 		assert.deepEqual(checkContext._relatedNodes, [fixture.querySelector('p')]);
 	});
 
+	it('should return true if the list has non-dd content through role change', function(){
+		var checkArgs = checkSetup('<dl id="target"><dd role="menuitem">Not a list</dd></dl>');
+
+		assert.isTrue(checks['only-dlitems'].evaluate.apply(checkContext, checkArgs));
+	});
+
+	it('should return true if the list has non-dt content through role change', function(){
+		var checkArgs = checkSetup('<dl id="target"><dt role="menuitem">Not a list</dt></dl>');
+
+		assert.isTrue(checks['only-dlitems'].evaluate.apply(checkContext, checkArgs));
+	});
+
 	it('should return false if the list has only a dd', function () {
 		var checkArgs = checkSetup('<dl id="target"><dd>A list</dd></dl>');
 
@@ -80,30 +92,60 @@ describe('only-dlitems', function () {
 		assert.isFalse(checks['only-dlitems'].evaluate.apply(checkContext, checkArgs));
 	});
 
+	it('should return true if <link> is used along side dt with its role changed', function () {
+		var checkArgs = checkSetup('<dl id="target"><link rel="stylesheet" href="theme.css"><dt role="menuitem">A list</dt></dl>');
+
+		assert.isTrue(checks['only-dlitems'].evaluate.apply(checkContext, checkArgs));
+	});
+
 	it('should return false if <meta> is used along side dt', function () {
 		var checkArgs = checkSetup('<dl id="target"><meta name="description" content=""><dt>A list</dt></dl>');
 
 		assert.isFalse(checks['only-dlitems'].evaluate.apply(checkContext, checkArgs));
 	});
 
+	it('should return true if <meta> is used along side dt with its role changed', function () {
+		var checkArgs = checkSetup('<dl id="target"><meta name="description" content=""><dt role="menuitem">A list</dt></dl>');
+
+		assert.isTrue(checks['only-dlitems'].evaluate.apply(checkContext, checkArgs));
+	});
+
 	it('should return false if <script> is used along side dt', function () {
 		var checkArgs = checkSetup('<dl id="target"><script src="script.js"></script><dt>A list</dt></dl>');
 
 		assert.isFalse(checks['only-dlitems'].evaluate.apply(checkContext, checkArgs));
 	});
 
+	it('should return true if <script> is used along side dt with its role changed', function () {
+		var checkArgs = checkSetup('<dl id="target"><script src="script.js"></script><dt role="menuitem">A list</dt></dl>');
+
+		assert.isTrue(checks['only-dlitems'].evaluate.apply(checkContext, checkArgs));
+	});
+
 	it('should return false if <style> is used along side dt', function () {
 		var checkArgs = checkSetup('<dl id="target"><style></style><dt>A list</dt></dl>');
 
 		assert.isFalse(checks['only-dlitems'].evaluate.apply(checkContext, checkArgs));
 	});
 
+	it('should return true if <style> is used along side dt with its role changed', function () {
+		var checkArgs = checkSetup('<dl id="target"><style></style><dt role="menuitem">A list</dt></dl>');
+
+		assert.isTrue(checks['only-dlitems'].evaluate.apply(checkContext, checkArgs));
+	});
+
 	it('should return false if <template> is used along side dt', function () {
 		var checkArgs = checkSetup('<dl id="target"><template></template><dt>A list</dt></dl>');
 
 		assert.isFalse(checks['only-dlitems'].evaluate.apply(checkContext, checkArgs));
 	});
 
+	it('should return true if <template> is used along side dt with its role changed', function () {
+		var checkArgs = checkSetup('<dl id="target"><template></template><dt role="menuitem">A list</dt></dl>');
+
+		assert.isTrue(checks['only-dlitems'].evaluate.apply(checkContext, checkArgs));
+	});
+
 	(shadowSupport.v1 ? it : xit)('should return false in a shadow DOM pass', function () {
 		var node = document.createElement('div');
 		node.innerHTML = '<dt>My list item </dt>';

test/checks/lists/only-listitems.js

@@ -23,6 +23,12 @@ describe('only-listitems', function () {
 		assert.isFalse(checks['only-listitems'].evaluate.apply(checkContext, checkArgs));
 	});
 
+	it('should return false if the list has only spaces as content', function () {
+		var checkArgs = checkSetup('<ol id="target">   </ol>');
+
+		assert.isFalse(checks['only-listitems'].evaluate.apply(checkContext, checkArgs));
+	});
+
 	it('should return false if the list has whitespace', function () {
 		var checkArgs = checkSetup('<ol id="target"><li>Item</li>    </ol>');
 
@@ -60,43 +66,97 @@ describe('only-listitems', function () {
 		assert.isFalse(checks['only-listitems'].evaluate.apply(checkContext, checkArgs));
 	});
 
+	it('should return false if the list has only an element with role listitem', function () {
+		var checkArgs = checkSetup('<ol id="target"><div role="listitem">A list</div></ol>');
+
+		assert.isFalse(checks['only-listitems'].evaluate.apply(checkContext, checkArgs));
+	});
+
 	it('should return true if the list has an li with other content', function () {
 		var checkArgs = checkSetup('<ol id="target"><li>A list</li><p>Not a list</p></ol>');
 
 		assert.isTrue(checks['only-listitems'].evaluate.apply(checkContext, checkArgs));
 		assert.deepEqual(checkContext._relatedNodes, [fixture.querySelector('p')]);
 	});
 
+	it('should return true if the list has only li items with their roles changed', function () {
+		var checkArgs = checkSetup('<ol id="target"><li role="menuitem">Not a list item</li><li role="menuitem">Not a list item</li></ol>');
+
+		assert.isTrue(checks['only-listitems'].evaluate.apply(checkContext, checkArgs));
+	});
+
+	it('should return false if the list has at least one li while others have their roles changed', function () {
+		var checkArgs = checkSetup('<ol id="target"><li >A list item</li><li role="menuitem">Not a list item</li></ol>');
+
+		assert.isFalse(checks['only-listitems'].evaluate.apply(checkContext, checkArgs));
+	});
+
 	it('should return false if <link> is used along side li', function () {
 		var checkArgs = checkSetup('<ol id="target"><link rel="stylesheet" href="theme.css"><li>A list</li></ol>');
 
 		assert.isFalse(checks['only-listitems'].evaluate.apply(checkContext, checkArgs));
 	});
 
+	it('should return true if <link> is used along side only li items with their roles changed', function () {
+		var checkArgs = checkSetup('<ol id="target"><link rel="stylesheet" href="theme.css"><li role="menuitem">Not a list item</li></ol>');
+
+		assert.isTrue(checks['only-listitems'].evaluate.apply(checkContext, checkArgs));
+	});
+
 	it('should return false if <meta> is used along side li', function () {
 		var checkArgs = checkSetup('<ol id="target"><meta name="description" content=""><li>A list</li></ol>');
 
 		assert.isFalse(checks['only-listitems'].evaluate.apply(checkContext, checkArgs));
 	});
 
+	it('should return true if <meta> is used along side only li items with their roles changed', function () {
+		var checkArgs = checkSetup('<ol id="target"><meta name="description" content=""><li role="menuitem">Not a list item</li></ol>');
+
+		assert.isTrue(checks['only-listitems'].evaluate.apply(checkContext, checkArgs));
+	});
+
 	it('should return false if <script> is used along side li', function () {
 		var checkArgs = checkSetup('<ol id="target"><script src="script.js"></script><li>A list</li></ol>');
 
 		assert.isFalse(checks['only-listitems'].evaluate.apply(checkContext, checkArgs));
 	});
 
+	it('should return true if <script> is used along side only li items with their roles changed', function () {
+		var checkArgs = checkSetup('<ol id="target"><script src="script.js"></script><li role="menuitem">Not a list item</li></ol>');
+
+		assert.isTrue(checks['only-listitems'].evaluate.apply(checkContext, checkArgs));
+	});
+
 	it('should return false if <style> is used along side li', function () {
 		var checkArgs = checkSetup('<ol id="target"><style></style><li>A list</li></ol>');
 
 		assert.isFalse(checks['only-listitems'].evaluate.apply(checkContext, checkArgs));
 	});
 
+	it('should return true if <style> is used along side only li items with their roles changed', function () {
+		var checkArgs = checkSetup('<ol id="target"><style></style><li role="menuitem">Not a list item</li></ol>');
+
+		assert.isTrue(checks['only-listitems'].evaluate.apply(checkContext, checkArgs));
+	});
+
 	it('should return false if <template> is used along side li', function () {
 		var checkArgs = checkSetup('<ol id="target"><template></template><li>A list</li></ol>');
 
 		assert.isFalse(checks['only-listitems'].evaluate.apply(checkContext, checkArgs));
 	});
 
+	it('should return true if <template> is used along side only li items with their roles changed', function () {
+		var checkArgs = checkSetup('<ol id="target"><template></template><li role="menuitem">Not a list item</li></ol>');
+
+		assert.isTrue(checks['only-listitems'].evaluate.apply(checkContext, checkArgs));
+	});
+
+	it('should return false if the list has only allowed non-li tags as content', function () {
+		var checkArgs = checkSetup('<ol id="target"><template></template><style></style></ol>');
+
+		assert.isFalse(checks['only-listitems'].evaluate.apply(checkContext, checkArgs));
+	});
+
 	(shadowSupport.v1 ? it : xit)('should return false in a shadow DOM pass', function () {
 		var node = document.createElement('div');
 		node.innerHTML = '<li>My list item </li>';

test/integration/rules/definition-list/definition-list.html

@@ -42,3 +42,11 @@
 	</template>
 	<p>para</p>
 </dl>
+<dl id="ddrolechanged">
+	<dd role="menuitem">Thingy</dd>
+	<dt>Thing</dt>
+</dl>
+<dl id="dtrolechanged">
+	<dd>Thingy</dd>
+	<dt role="menuitem">Thing</dt>
+</dl>

test/integration/rules/definition-list/definition-list.json

@@ -1,6 +1,7 @@
 {
 	"description": "definition-list test",
 	"rule": "definition-list",
-	"violations": [["#divdl"], ["#mixeddl"], ["#unordereddl"], ["#mixedscriptdl"]],
+	"violations": [["#divdl"], ["#mixeddl"], ["#unordereddl"], ["#mixedscriptdl"],
+    ["#ddrolechanged"], ["#dtrolechanged"]],
 	"passes": [["#emptydl"], ["#repeatdl"], ["#properdl"], ["#scriptdl"], ["#emptyscriptdl"]]
 }

test/integration/rules/dlitem/dlitem.html

@@ -3,3 +3,7 @@
 <dl><dt id="contained">Does belong to a list.</dt>
 	<dd id="alsocontained">Also belongs to a list.</dd>
 </dl>
+<dl role="menubar">
+	<dt id="uncontainedbyrole">Not part of a list.</dt>
+	<dd id="alsouncontainedbyrole">Also not part of a list.</dd>
+</dl>

test/integration/rules/dlitem/dlitem.json

@@ -1,6 +1,6 @@
 {
 	"description": "dlitem test",
 	"rule": "dlitem",
-	"violations": [["#uncontained"], ["#alsouncontained"]],
+	"violations": [["#uncontained"], ["#alsouncontained"],["#uncontainedbyrole"],["#alsouncontainedbyrole"]],
 	"passes": [["#contained"], ["#alsocontained"]]
 }

test/integration/rules/list/list.html

@@ -17,3 +17,7 @@
 </ol>
 <ol id="properol"><li>One.</li><li>Two.</li></ol>
 <ol id="scriptproperol"><script></script><template></template><li>One.</li><li>Two.</li></ol>
+<ul id="ulrolledallli"><li role="menuitem">One.</li><li role="menuitem">Two.</li></ul>
+<ul id="ulrolledli"><li>One.</li><li role="menuitem">Two</li></ul>
+<ol id="olrolledallli"><li role="menuitem">One.</li><li role="menuitem">Two.</li></ol>
+<ol id="olrolledli"><li>One.</li><li role="menuitem">Two</li></ol>

test/integration/rules/list/list.json

@@ -2,7 +2,7 @@
 	"description": "list test",
 	"rule": "list",
 	"violations": [["#divul"], ["#mixedul"],
-		["#divol"], ["#mixedol"]],
+		["#divol"], ["#mixedol"], ["#ulrolledallli"], ["#olrolledallli"]],
 	"passes": [["#emptyul"], ["#scriptemptyul"], ["#properul"], ["#scriptproperul"],
-		["#emptyol"], ["#scriptemptyol"], ["#properol"], ["#scriptproperol"]]
+		["#emptyol"], ["#scriptemptyol"], ["#properol"], ["#scriptproperol"], ["#ulrolledli"], ["#olrolledli"]]
 }

test/integration/rules/listitem/listitem.html

@@ -1,4 +1,6 @@
 
 <li id="uncontained">Should belong to a list.</li>
 <ul><li id="contained">Does belong to a list.</li></ul>
-<ol><li id="alsocontained">Also belongs toa  list.</li></ol>
+<ol><li id="alsocontained">Also belongs to a  list.</li></ol>
+<ul role="menubar"><li id="ulrolechanged">Also does not belong to a list.</li></ul>
+<ol role="menubar"><li id="olrolechanged">I too do not belong to a list.</li></ol>

test/integration/rules/listitem/listitem.json

@@ -1,6 +1,6 @@
 {
 	"description": "listitem test",
 	"rule": "listitem",
-	"violations": [["#uncontained"]],
+	"violations": [["#uncontained"], ["#ulrolechanged"], ["#olrolechanged"]],
 	"passes": [["#contained"], ["#alsocontained"]]
 }