From 5f21fe889592d4b92e8a0bb99fcb8f65fa81e89b Mon Sep 17 00:00:00 2001
From: Jey <jey.nandakumar@gmail.com>
Date: Fri, 8 Jun 2018 18:10:18 +0100
Subject: [PATCH 1/5] fix: list and list items fixes for role changes.

---
 lib/checks/lists/dlitem.js                    | 23 ++++-
 lib/checks/lists/listitem.js                  | 18 +++-
 lib/checks/lists/only-dlitems.js              | 52 +++++++---
 lib/checks/lists/only-listitems.js            | 98 ++++++++++++++++---
 test/checks/lists/dlitem.js                   | 19 +++-
 test/checks/lists/listitem.js                 | 10 +-
 test/checks/lists/only-dlitems.js             | 17 ++++
 test/checks/lists/only-listitems.js           | 35 +++++++
 .../definition-list/definition-list.html      | 15 ++-
 .../definition-list/definition-list.json      |  2 +-
 test/integration/rules/dlitem/dlitem.html     |  7 +-
 test/integration/rules/dlitem/dlitem.json     |  2 +-
 test/integration/rules/list/list.html         | 63 +++++++++---
 test/integration/rules/list/list.json         |  6 +-
 test/integration/rules/listitem/listitem.html | 21 +++-
 test/integration/rules/listitem/listitem.json |  2 +-
 16 files changed, 321 insertions(+), 69 deletions(-)

diff --git a/lib/checks/lists/dlitem.js b/lib/checks/lists/dlitem.js
index 80a6608fc7..e3b9bbd97e 100644
--- a/lib/checks/lists/dlitem.js
+++ b/lib/checks/lists/dlitem.js
@@ -1,2 +1,21 @@
-var parent = axe.commons.dom.getComposedParent(node);
-return parent.nodeName.toUpperCase() === 'DL';
+const parent = axe.commons.dom.getComposedParent(node);
+
+if (!parent) {
+	return false;
+}
+
+const ALLOWED_ROLES = ['list'];
+const parentTagName = parent.nodeName.toLowerCase();
+const parentRole = (parent.getAttribute('role') || '').toLowerCase();
+
+if (parentTagName === 'dl') {
+	if (parentRole) {
+		if (ALLOWED_ROLES.includes(parentRole)) {
+			return true;
+		}
+		return false;
+	}
+	return true;
+}
+
+return false;
diff --git a/lib/checks/lists/listitem.js b/lib/checks/lists/listitem.js
index 1a7abcb40d..758282d876 100644
--- a/lib/checks/lists/listitem.js
+++ b/lib/checks/lists/listitem.js
@@ -1,4 +1,14 @@
-var parent = axe.commons.dom.getComposedParent(node);
-return (['UL', 'OL'].includes(parent.nodeName.toUpperCase()) ||
-    (parent.getAttribute('role') || '').toLowerCase() === 'list');
-  
\ No newline at end of file
+const parent = axe.commons.dom.getComposedParent(node);
+
+if (!parent) {
+	return false;
+}
+
+const ALLOWED_TAGS = ['ul', 'ol'];
+const ALLOWED_ROLES = ['list'];
+
+const parentTagName = parent.nodeName.toLowerCase();
+const parentRole = (parent.getAttribute('role') || '').toLowerCase();
+const isListRole = (ALLOWED_ROLES.includes(parentRole));
+
+return ((ALLOWED_TAGS.includes(parentTagName) && (!parentRole || isListRole)) ||isListRole);
diff --git a/lib/checks/lists/only-dlitems.js b/lib/checks/lists/only-dlitems.js
index ff3a20d62d..537c8d7588 100644
--- a/lib/checks/lists/only-dlitems.js
+++ b/lib/checks/lists/only-dlitems.js
@@ -1,19 +1,41 @@
-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 !== 'DT' && nodeName !== 'DD' && 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'
+];
 
-if (bad.length) {
-	this.relatedNodes(bad);
+const ALLOWED_ROLES = ['definition', 'term', 'list'];
+
+let base = {
+	badNodes: [],
+	hasNonEmptyTextNode: false
+};
+
+const out = virtualNode.children
+	.reduce((out, { actualNode }) => {
+		const tagName = actualNode.nodeName.toLowerCase();
+		if (actualNode.nodeType === 1 && ALLOWED_TAGS.indexOf(tagName) === -1) {
+			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() !== '') {
+			out.hasNonEmptyTextNode = true;
+		}
+		return out;
+	}, base);
+
+if (out.badNodes.length) {
+	this.relatedNodes(out.badNodes);
 }
 
-var retVal = !!bad.length || hasNonEmptyTextNode;
-return retVal;
+return !!out.badNodes.length || out.hasNonEmptyTextNode;
\ No newline at end of file
diff --git a/lib/checks/lists/only-listitems.js b/lib/checks/lists/only-listitems.js
index bdb53064e5..2f7333d529 100644
--- a/lib/checks/lists/only-listitems.js
+++ b/lib/checks/lists/only-listitems.js
@@ -1,18 +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;
-	}
-});
-
-if (bad.length) {
-	this.relatedNodes(bad);
+const ALLOWED_TAGS = [
+	'style',
+	'meta',
+	'link',
+	'map',
+	'area',
+	'script',
+	'datalist',
+	'template'
+];
+
+const getIsListItemRole = (r, t) => {
+	let out = false;
+	out = (r === 'listitem' || (t === 'li' && !r));
+	return out;
+};
+
+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 
+		complexity: ["error", 11] 
+		*/
+		const tagName = actualNode.nodeName.toLowerCase();
+
+		switch (actualNode.nodeType) {
+		case 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);
+				}
+			}
+			break;
+		case 3:
+			if (actualNode.nodeValue.trim() !== '') {
+				out.hasNonEmptyTextNode = true;
+			}
+			break;
+		default:
+			break;
+		}
+		return out;
+	}, base);
+
+const virtualNodeChildrenOfTypeLi = virtualNode.children
+	.filter(({ actualNode }) => {
+		return actualNode.nodeName.toLowerCase() === 'li'
+	});
+
+const allLiItemsHaveRole = out.liItemsWithRole > 0 &&
+	virtualNodeChildrenOfTypeLi.length === out.liItemsWithRole;
+
+if (out.badNodes.length) {
+	this.relatedNodes(out.badNodes);
 }
 
-return !!bad.length || hasNonEmptyTextNode;
+return (
+	!(out.hasListItem ||
+		(out.isEmpty && !allLiItemsHaveRole)) ||
+	!!out.badNodes.length ||
+	out.hasNonEmptyTextNode
+);
diff --git a/test/checks/lists/dlitem.js b/test/checks/lists/dlitem.js
index 56f7596b36..77e7f144b8 100644
--- a/test/checks/lists/dlitem.js
+++ b/test/checks/lists/dlitem.js
@@ -15,12 +15,27 @@ describe('dlitem', function () {
 		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));
 	});
 
+	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>';
@@ -41,3 +56,5 @@ describe('dlitem', function () {
 		assert.isFalse(checks.dlitem.evaluate.apply(null, checkArgs));
 	});
 });
+
+
diff --git a/test/checks/lists/listitem.js b/test/checks/lists/listitem.js
index e543634883..751bd6b6e9 100644
--- a/test/checks/lists/listitem.js
+++ b/test/checks/lists/listitem.js
@@ -11,25 +11,26 @@ describe('listitem', function () {
 
 	it('should pass if the listitem has a parent <ol>', function () {
 		var checkArgs = checkSetup('<ol><li id="target">My list item</li></ol>');
-
 		assert.isTrue(checks.listitem.evaluate.apply(null, checkArgs));
 	});
 
 	it('should pass if the listitem has a parent <ul>', function () {
 		var checkArgs = checkSetup('<ul><li id="target">My list item</li></ul>');
-
 		assert.isTrue(checks.listitem.evaluate.apply(null, checkArgs));
 	});
 
 	it('should pass if the listitem has a parent role=list', function () {
 		var checkArgs = checkSetup('<div role="list"><li id="target">My list item</li></div>');
-
 		assert.isTrue(checks.listitem.evaluate.apply(null, checkArgs));
 	});
 
 	it('should fail if the listitem has an incorrect parent', function () {
 		var checkArgs = checkSetup('<div><li id="target">My list item</li></div>');
+		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));
 	});
 
@@ -38,7 +39,6 @@ describe('listitem', function () {
 		node.innerHTML = '<li>My list item </li>';
 		var shadow = node.attachShadow({ mode: 'open' });
 		shadow.innerHTML = '<ul><slot></slot></ul>';
-
 		var checkArgs = checkSetup(node, 'li');
 		assert.isTrue(checks.listitem.evaluate.apply(null, checkArgs));
 	});
@@ -48,8 +48,8 @@ describe('listitem', function () {
 		node.innerHTML = '<li>My list item </li>';
 		var shadow = node.attachShadow({ mode: 'open' });
 		shadow.innerHTML = '<div><slot></slot></div>';
-
 		var checkArgs = checkSetup(node, 'li');
 		assert.isFalse(checks.listitem.evaluate.apply(null, checkArgs));
 	});
+
 });
diff --git a/test/checks/lists/only-dlitems.js b/test/checks/lists/only-dlitems.js
index 71fa350dd2..624624d204 100644
--- a/test/checks/lists/only-dlitems.js
+++ b/test/checks/lists/only-dlitems.js
@@ -25,12 +25,29 @@ 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>');
 
 		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 the list has only a dt', function () {
 		var checkArgs = checkSetup('<dl id="target"><dt>A list</dt></dl>');
 
diff --git a/test/checks/lists/only-listitems.js b/test/checks/lists/only-listitems.js
index 5ff9108829..ef7b864e14 100644
--- a/test/checks/lists/only-listitems.js
+++ b/test/checks/lists/only-listitems.js
@@ -18,12 +18,30 @@ 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>');
 
 		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 false if the list has only multiple mixed elements with role listitem', function () {
+		var checkArgs = checkSetup('<ol id="target"><div role="listitem">list</div><li role="listitem">list</li><div role="listitem">list</div></ol>');
+
+		assert.isFalse(checks['only-listitems'].evaluate.apply(checkContext, checkArgs));
+	});
+
 	it('should return false if the list has non-li comments', function () {
 		var checkArgs = checkSetup('<ol id="target"><li>Item</li><!--comment--></ol>');
 
@@ -62,6 +80,23 @@ describe('only-listitems', function () {
 		assert.deepEqual(checkContext._relatedNodes, [fixture.querySelector('p')]);
 	});
 
+	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 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 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 <link> is used along side li', function () {
 		var checkArgs = checkSetup('<ol id="target"><link rel="stylesheet" href="theme.css"><li>A list</li></ol>');
 
diff --git a/test/integration/rules/definition-list/definition-list.html b/test/integration/rules/definition-list/definition-list.html
index 8581722c20..e486ab404a 100644
--- a/test/integration/rules/definition-list/definition-list.html
+++ b/test/integration/rules/definition-list/definition-list.html
@@ -1,6 +1,7 @@
-
-<dl id="emptydl">  </dl>
-<dl id="divdl"><div>This shouldn't be here.</div></dl>
+<dl id="emptydl"> </dl>
+<dl id="divdl">
+	<div>This shouldn't be here.</div>
+</dl>
 <dl id="mixeddl">
 	<dt>Correct.</dt>
 	<dd>Correct.</dd>
@@ -42,3 +43,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>
\ No newline at end of file
diff --git a/test/integration/rules/definition-list/definition-list.json b/test/integration/rules/definition-list/definition-list.json
index 6f3825225e..f7a8612fc3 100644
--- a/test/integration/rules/definition-list/definition-list.json
+++ b/test/integration/rules/definition-list/definition-list.json
@@ -1,6 +1,6 @@
 {
 	"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"]]
 }
diff --git a/test/integration/rules/dlitem/dlitem.html b/test/integration/rules/dlitem/dlitem.html
index d20c8216b2..3c92602827 100644
--- a/test/integration/rules/dlitem/dlitem.html
+++ b/test/integration/rules/dlitem/dlitem.html
@@ -1,5 +1,10 @@
 <dd id="uncontained">Should belong to a list.</dd>
 <dt id="alsouncontained">Should belong to a list.</dt>
-<dl><dt id="contained">Does belong to a list.</dt>
+<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>
\ No newline at end of file
diff --git a/test/integration/rules/dlitem/dlitem.json b/test/integration/rules/dlitem/dlitem.json
index 077ae72432..ff8b0a06fe 100644
--- a/test/integration/rules/dlitem/dlitem.json
+++ b/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"]]
 }
diff --git a/test/integration/rules/list/list.html b/test/integration/rules/list/list.html
index ae252124c4..b1e2762e82 100644
--- a/test/integration/rules/list/list.html
+++ b/test/integration/rules/list/list.html
@@ -1,19 +1,60 @@
-
-<ul id="emptyul">  </ul>
-<ul id="scriptemptyul"><script></script><template></template>  </ul>
-<ul id="divul"><div>This shouldn't be here.</div></ul>
+<ul id="emptyul"> </ul>
+<ul id="scriptemptyul">
+	<script></script>
+	<template></template>
+</ul>
+<ul id="divul">
+	<div>This shouldn't be here.</div>
+</ul>
 <ul id="mixedul">
 	<li>Correct.</li>
 	<div>Wrong.</div>
 </ul>
-<ul id="properul"><li>One.</li><li>Two.</li></ul>
-<ul id="scriptproperul"><script></script><template></template><li>One.</li><li>Two.</li></ul>
-<ol id="emptyol">  </ol>
-<ol id="scriptemptyol"><script></script><template></template>  </ol>
-<ol id="divol"><div>This shouldn't be here.</div></ol>
+<ul id="properul">
+	<li>One.</li>
+	<li>Two.</li>
+</ul>
+<ul id="scriptproperul">
+	<script></script>
+	<template></template>
+	<li>One.</li>
+	<li>Two.</li>
+</ul>
+<ol id="emptyol"> </ol>
+<ol id="scriptemptyol">
+	<script></script>
+	<template></template>
+</ol>
+<ol id="divol">
+	<div>This shouldn't be here.</div>
+</ol>
 <ol id="mixedol">
 	<li>Correct.</li>
 	<div>Wrong.</div>
 </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>
+<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>
\ No newline at end of file
diff --git a/test/integration/rules/list/list.json b/test/integration/rules/list/list.json
index 53e4645890..b0cfc24114 100644
--- a/test/integration/rules/list/list.json
+++ b/test/integration/rules/list/list.json
@@ -1,8 +1,6 @@
 {
 	"description": "list test",
 	"rule": "list",
-	"violations": [["#divul"], ["#mixedul"],
-		["#divol"], ["#mixedol"]],
-	"passes": [["#emptyul"], ["#scriptemptyul"], ["#properul"], ["#scriptproperul"],
-		["#emptyol"], ["#scriptemptyol"], ["#properol"], ["#scriptproperol"]]
+	"violations": [["#divul"], ["#mixedul"], ["#divol"], ["#mixedol"], ["#ulrolledallli"], ["#olrolledallli"]],
+	"passes": [["#emptyul"], ["#scriptemptyul"], ["#properul"], ["#scriptproperul"], ["#emptyol"], ["#scriptemptyol"], ["#properol"], ["#scriptproperol"], ["#ulrolledli"], ["#olrolledli"]]
 }
diff --git a/test/integration/rules/listitem/listitem.html b/test/integration/rules/listitem/listitem.html
index 06ebb7d11e..0186df462f 100644
--- a/test/integration/rules/listitem/listitem.html
+++ b/test/integration/rules/listitem/listitem.html
@@ -1,4 +1,17 @@
-
-<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>
+<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>
\ No newline at end of file
diff --git a/test/integration/rules/listitem/listitem.json b/test/integration/rules/listitem/listitem.json
index 2cf761638f..036d4dc42e 100644
--- a/test/integration/rules/listitem/listitem.json
+++ b/test/integration/rules/listitem/listitem.json
@@ -1,6 +1,6 @@
 {
 	"description": "listitem test",
 	"rule": "listitem",
-	"violations": [["#uncontained"]],
+	"violations": [["#uncontained"], ["#ulrolechanged"], ["#olrolechanged"]],
 	"passes": [["#contained"], ["#alsocontained"]]
 }

From 0bf986394ef89797eea9fa3d583062e00c1fb0ea Mon Sep 17 00:00:00 2001
From: Jey <jey.nandakumar@gmail.com>
Date: Fri, 8 Jun 2018 18:17:40 +0100
Subject: [PATCH 2/5] fix: integration test for listitem

---
 test/integration/rules/listitem/listitem.html | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/test/integration/rules/listitem/listitem.html b/test/integration/rules/listitem/listitem.html
index 0186df462f..8550b9fb0f 100644
--- a/test/integration/rules/listitem/listitem.html
+++ b/test/integration/rules/listitem/listitem.html
@@ -3,9 +3,6 @@
 <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>

From fbbb1788cdfb2fa169045a92d8e9ddeba36d5c18 Mon Sep 17 00:00:00 2001
From: Jey <jey.nandakumar@gmail.com>
Date: Tue, 3 Jul 2018 20:51:14 +0100
Subject: [PATCH 3/5] fix: tackle comments on PR

---
 lib/checks/lists/dlitem.js         |  27 +++--
 lib/checks/lists/listitem.js       |  16 +--
 lib/checks/lists/only-dlitems.js   |  23 ++--
 lib/checks/lists/only-listitems.js | 164 +++++++++++++++++++++--------
 4 files changed, 161 insertions(+), 69 deletions(-)

diff --git a/lib/checks/lists/dlitem.js b/lib/checks/lists/dlitem.js
index e3b9bbd97e..b07e047c08 100644
--- a/lib/checks/lists/dlitem.js
+++ b/lib/checks/lists/dlitem.js
@@ -1,21 +1,26 @@
 const parent = axe.commons.dom.getComposedParent(node);
-
 if (!parent) {
 	return false;
 }
 
-const ALLOWED_ROLES = ['list'];
-const parentTagName = parent.nodeName.toLowerCase();
+const parentTagName = parent.nodeName.toUpperCase();
+if (parentTagName !== 'DL') {
+	return false;
+}
+
 const parentRole = (parent.getAttribute('role') || '').toLowerCase();
 
-if (parentTagName === 'dl') {
-	if (parentRole) {
-		if (ALLOWED_ROLES.includes(parentRole)) {
-			return true;
-		}
-		return false;
-	}
+if (!parentRole) {
 	return true;
 }
 
-return false;
+if (!axe.commons.aria.isValidRole(parentRole)) {
+	return false;
+}
+
+if (parentRole) {
+	if (parentRole === 'list') {
+		return true;
+	}
+	return false;
+}
diff --git a/lib/checks/lists/listitem.js b/lib/checks/lists/listitem.js
index 758282d876..3270c8b4e3 100644
--- a/lib/checks/lists/listitem.js
+++ b/lib/checks/lists/listitem.js
@@ -1,14 +1,18 @@
 const parent = axe.commons.dom.getComposedParent(node);
-
 if (!parent) {
 	return false;
 }
 
-const ALLOWED_TAGS = ['ul', 'ol'];
-const ALLOWED_ROLES = ['list'];
+const ALLOWED_TAGS = ['UL', 'OL'];
+const parentTagName = parent.nodeName.toUpperCase();
 
-const parentTagName = parent.nodeName.toLowerCase();
 const parentRole = (parent.getAttribute('role') || '').toLowerCase();
-const isListRole = (ALLOWED_ROLES.includes(parentRole));
+if (parentRole && !axe.commons.aria.isValidRole(parentRole)) {
+	return false;
+}
 
-return ((ALLOWED_TAGS.includes(parentTagName) && (!parentRole || isListRole)) ||isListRole);
+const isListRole = parentRole === 'list';
+return (
+	(ALLOWED_TAGS.includes(parentTagName) && (!parentRole || isListRole)) ||
+	isListRole
+);
diff --git a/lib/checks/lists/only-dlitems.js b/lib/checks/lists/only-dlitems.js
index 98465ff7f2..c0e6bc36f4 100644
--- a/lib/checks/lists/only-dlitems.js
+++ b/lib/checks/lists/only-dlitems.js
@@ -1,12 +1,12 @@
 const ALLOWED_TAGS = [
-	'style',
-	'meta',
-	'link',
-	'map',
-	'area',
-	'script',
-	'datalist',
-	'template'
+	'STYLE',
+	'META',
+	'LINK',
+	'MAP',
+	'AREA',
+	'SCRIPT',
+	'DATALIST',
+	'TEMPLATE'
 ];
 
 const ALLOWED_ROLES = ['definition', 'term', 'list'];
@@ -17,10 +17,11 @@ let base = {
 };
 
 const out = virtualNode.children.reduce((out, { actualNode }) => {
-	const tagName = actualNode.nodeName.toLowerCase();
-	if (actualNode.nodeType === 1 && ALLOWED_TAGS.indexOf(tagName) === -1) {
+	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 ((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);
diff --git a/lib/checks/lists/only-listitems.js b/lib/checks/lists/only-listitems.js
index 2f7333d529..9939921825 100644
--- a/lib/checks/lists/only-listitems.js
+++ b/lib/checks/lists/only-listitems.js
@@ -1,3 +1,89 @@
+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 
+		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 (out.badNodes.length) {
+	this.relatedNodes(out.badNodes);
+}
+
+const isInvalidListItem = !(
+	out.hasListItem ||
+	(out.isEmpty && !allLiItemsHaveRole)
+);
+return isInvalidListItem || !!out.badNodes.length || out.hasNonEmptyTextNode;
+
+/** 
+
 const ALLOWED_TAGS = [
 	'style',
 	'meta',
@@ -9,14 +95,12 @@ const ALLOWED_TAGS = [
 	'template'
 ];
 
-const getIsListItemRole = (r, t) => {
-	let out = false;
-	out = (r === 'listitem' || (t === 'li' && !r));
-	return out;
+const getIsListItemRole = (role, tagName) => {
+	return (role === 'listitem' || (tagName === 'LI' && !role));
 };
 
 const getHasListItem = (hasListItem, tagName, isListItemRole) => {
-	return hasListItem || (tagName === 'li' && isListItemRole) || isListItemRole;
+	return (hasListItem || (tagName === 'LI' && isListItemRole) || isListItemRole);
 }
 
 let base = {
@@ -29,56 +113,54 @@ let base = {
 
 let out = virtualNode.children
 	.reduce((out, { actualNode }) => {
-		/*eslint 
-		complexity: ["error", 11] 
-		*/
-		const tagName = actualNode.nodeName.toLowerCase();
-
-		switch (actualNode.nodeType) {
-		case 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);
-				}
+	
+		const tagName = actualNode.nodeName.toUpperCase();
+
+		if(actualNode.nodeType === 1) {
+			if (ALLOWED_TAGS.includes(tagName)) {
+				return;
+			}
+
+			const role = (actualNode.getAttribute('role') || '').toLowerCase();
+			const isListItemRole = getIsListItemRole(role, tagName);
+			
+			out.hasListItem = getHasListItem(out.hasListItem, tagName, isListItemRole);
+
+			if (isListItemRole) {
+				out.isEmpty = false;
 			}
-			break;
-		case 3:
+			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;
 			}
-			break;
-		default:
-			break;
 		}
+
 		return out;
 	}, base);
 
 const virtualNodeChildrenOfTypeLi = virtualNode.children
 	.filter(({ actualNode }) => {
-		return actualNode.nodeName.toLowerCase() === 'li'
+		return actualNode.nodeName.toUpperCase() === 'LI';
 	});
 
-const allLiItemsHaveRole = out.liItemsWithRole > 0 &&
-	virtualNodeChildrenOfTypeLi.length === out.liItemsWithRole;
+const allLiItemsHaveRole = (out.liItemsWithRole > 0 && 
+	virtualNodeChildrenOfTypeLi.length === out.liItemsWithRole);
 
 if (out.badNodes.length) {
 	this.relatedNodes(out.badNodes);
 }
 
-return (
-	!(out.hasListItem ||
-		(out.isEmpty && !allLiItemsHaveRole)) ||
-	!!out.badNodes.length ||
-	out.hasNonEmptyTextNode
-);
+const notListItem = !(out.hasListItem || (out.isEmpty && !allLiItemsHaveRole));
+
+return (notListItem || !!out.badNodes.length || out.hasNonEmptyTextNode);
+
+*/

From e3c224463eea1ed3a29d6364d542b8d15ac881a8 Mon Sep 17 00:00:00 2001
From: Jey <jey.nandakumar@gmail.com>
Date: Tue, 3 Jul 2018 21:06:04 +0100
Subject: [PATCH 4/5] style: update eslint max-statements allowed

---
 lib/checks/lists/only-listitems.js | 84 +-----------------------------
 1 file changed, 1 insertion(+), 83 deletions(-)

diff --git a/lib/checks/lists/only-listitems.js b/lib/checks/lists/only-listitems.js
index 9939921825..5a66a4b0e2 100644
--- a/lib/checks/lists/only-listitems.js
+++ b/lib/checks/lists/only-listitems.js
@@ -27,6 +27,7 @@ let base = {
 
 let out = virtualNode.children.reduce((out, { actualNode }) => {
 	/*eslint 
+		max-statements: ["error", 20]
 		complexity: ["error", 11] 
 		*/
 	const tagName = actualNode.nodeName.toUpperCase();
@@ -81,86 +82,3 @@ const isInvalidListItem = !(
 	(out.isEmpty && !allLiItemsHaveRole)
 );
 return isInvalidListItem || !!out.badNodes.length || out.hasNonEmptyTextNode;
-
-/** 
-
-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 }) => {
-	
-		const tagName = actualNode.nodeName.toUpperCase();
-
-		if(actualNode.nodeType === 1) {
-			if (ALLOWED_TAGS.includes(tagName)) {
-				return;
-			}
-
-			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 (out.badNodes.length) {
-	this.relatedNodes(out.badNodes);
-}
-
-const notListItem = !(out.hasListItem || (out.isEmpty && !allLiItemsHaveRole));
-
-return (notListItem || !!out.badNodes.length || out.hasNonEmptyTextNode);
-
-*/

From 6821aaba3495b136bd046f9cfaa6f47178c134fe Mon Sep 17 00:00:00 2001
From: Jey <jey.nandakumar@gmail.com>
Date: Wed, 18 Jul 2018 09:55:47 +0100
Subject: [PATCH 5/5] refactor: changes based on code review

---
 lib/checks/lists/dlitem.js       | 14 +++++---------
 lib/checks/lists/only-dlitems.js |  9 +++++----
 2 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/lib/checks/lists/dlitem.js b/lib/checks/lists/dlitem.js
index b07e047c08..b4f11e5315 100644
--- a/lib/checks/lists/dlitem.js
+++ b/lib/checks/lists/dlitem.js
@@ -1,9 +1,6 @@
 const parent = axe.commons.dom.getComposedParent(node);
-if (!parent) {
-	return false;
-}
-
 const parentTagName = parent.nodeName.toUpperCase();
+
 if (parentTagName !== 'DL') {
 	return false;
 }
@@ -18,9 +15,8 @@ if (!axe.commons.aria.isValidRole(parentRole)) {
 	return false;
 }
 
-if (parentRole) {
-	if (parentRole === 'list') {
-		return true;
-	}
-	return false;
+if (parentRole === 'list') {
+	return true;
 }
+
+return false;
diff --git a/lib/checks/lists/only-dlitems.js b/lib/checks/lists/only-dlitems.js
index c0e6bc36f4..5227fb60de 100644
--- a/lib/checks/lists/only-dlitems.js
+++ b/lib/checks/lists/only-dlitems.js
@@ -16,7 +16,8 @@ let base = {
 	hasNonEmptyTextNode: false
 };
 
-const out = virtualNode.children.reduce((out, { actualNode }) => {
+const result = virtualNode.children.reduce((out, childNode) => {
+	const { actualNode } = childNode;
 	const tagName = actualNode.nodeName.toUpperCase();
 
 	if (actualNode.nodeType === 1 && !ALLOWED_TAGS.includes(tagName)) {
@@ -33,8 +34,8 @@ const out = virtualNode.children.reduce((out, { actualNode }) => {
 	return out;
 }, base);
 
-if (out.badNodes.length) {
-	this.relatedNodes(out.badNodes);
+if (result.badNodes.length) {
+	this.relatedNodes(result.badNodes);
 }
 
-return !!out.badNodes.length || out.hasNonEmptyTextNode;
+return !!result.badNodes.length || result.hasNonEmptyTextNode;