From b0acffe2709da10266b37bd9b5f0c8805f31ef0c Mon Sep 17 00:00:00 2001
From: Wilco Fiers <wilco.fiers@deque.com>
Date: Wed, 19 Dec 2018 12:08:23 +0100
Subject: [PATCH 1/2] fix: Allow div groups for dlitem rule

---
 lib/checks/lists/dlitem.js                | 21 +++---
 test/checks/lists/dlitem.js               | 87 ++++++++++++++++++++---
 test/integration/rules/dlitem/dlitem.html | 10 ++-
 test/integration/rules/dlitem/dlitem.json |  6 +-
 4 files changed, 104 insertions(+), 20 deletions(-)

diff --git a/lib/checks/lists/dlitem.js b/lib/checks/lists/dlitem.js
index a750334627..55bde622f0 100644
--- a/lib/checks/lists/dlitem.js
+++ b/lib/checks/lists/dlitem.js
@@ -1,17 +1,22 @@
-const parent = axe.commons.dom.getComposedParent(node);
-const parentTagName = parent.nodeName.toUpperCase();
+let parent = axe.commons.dom.getComposedParent(node);
+let parentTagName = parent.nodeName.toUpperCase();
+let parentRole = axe.commons.aria.getRole(parent, { noImplicit: true });
+
+if (
+	parentTagName === 'DIV' &&
+	['presentation', 'none', null].includes(parentRole)
+) {
+	parent = axe.commons.dom.getComposedParent(parent);
+	parentTagName = parent.nodeName.toUpperCase();
+	parentRole = axe.commons.aria.getRole(parent, { noImplicit: true });
+}
 
 // Unlike with UL|OL+LI, DT|DD must be in a DL
 if (parentTagName !== 'DL') {
 	return false;
 }
 
-const parentRole = (parent.getAttribute('role') || '').toLowerCase();
-if (!parentRole || !axe.commons.aria.isValidRole(parentRole)) {
-	return true;
-}
-
-if (parentRole === 'list') {
+if (!parentRole || parentRole === 'list') {
 	return true;
 }
 
diff --git a/test/checks/lists/dlitem.js b/test/checks/lists/dlitem.js
index 615df2f2c5..0c5aff043a 100644
--- a/test/checks/lists/dlitem.js
+++ b/test/checks/lists/dlitem.js
@@ -10,60 +10,116 @@ describe('dlitem', function() {
 	});
 
 	it('should pass if the dlitem has a parent <dl>', function() {
-		var checkArgs = checkSetup('<dl><dt id="target">My list item</dl>');
+		var checkArgs = checkSetup('<dl><dt id="target">My list item</dt></dl>');
 
 		assert.isTrue(checks.dlitem.evaluate.apply(null, checkArgs));
 	});
 
 	it('should fail if the dt element has an incorrect parent', function() {
-		var checkArgs = checkSetup('<video><dt id="target">My list item</video>');
+		var checkArgs = checkSetup(
+			'<video><dt id="target">My list item</dt></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>'
+			'<dl role="list"><dt id="target">My list item</dt></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>'
+			'<dl role="presentation"><dt id="target">My list item</dt></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>'
+			'<dl role="menubar"><dt id="target">My list item<</dt>/dl>'
 		);
 		assert.isFalse(checks.dlitem.evaluate.apply(null, checkArgs));
 	});
 
 	it('should pass if the dt element has a parent <dl> with an abstract role', function() {
 		var checkArgs = checkSetup(
-			'<dl role="section"><dt id="target">My list item</dl>'
+			'<dl role="section"><dt id="target">My list item</dt></dl>'
 		);
 		assert.isTrue(checks.dlitem.evaluate.apply(null, checkArgs));
 	});
 
 	it('should pass if the dt element has a parent <dl> with an invalid role', function() {
 		var checkArgs = checkSetup(
-			'<dl role="invalid-role"><dt id="target">My list item</dl>'
+			'<dl role="invalid-role"><dt id="target">My list item</dt></dl>'
 		);
 		assert.isTrue(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>'
+			'<dl role="menubar"><dt id="target">My list item</dt></dl>'
 		);
 		assert.isFalse(checks.dlitem.evaluate.apply(null, checkArgs));
 	});
 
-	(shadowSupport.v1 ? it : xit)(
+	it('returns true if the dd/dt is in a div with a dl as grandparent', function() {
+		const nodeNames = ['dd', 'dt'];
+		nodeNames.forEach(function(nodeName) {
+			var checkArgs = checkSetup(
+				'<dl><div><' +
+					nodeName +
+					' id="target">My list item</' +
+					nodeName +
+					'></div></dl>'
+			);
+			assert.isTrue(checks.dlitem.evaluate.apply(null, checkArgs));
+		});
+	});
+
+	it('returns false if the dd/dt is in a div with a role with a dl as grandparent', function() {
+		const nodeNames = ['dd', 'dt'];
+		nodeNames.forEach(function(nodeName) {
+			var checkArgs = checkSetup(
+				'<dl><div role="list"><' +
+					nodeName +
+					' id="target">My list item</' +
+					nodeName +
+					'></div></dl>'
+			);
+			assert.isFalse(checks.dlitem.evaluate.apply(null, checkArgs));
+		});
+	});
+
+	it('returns false if the dd/dt is in a div[role=presentation] with a dl as grandparent', function() {
+		const nodeNames = ['dd', 'dt'];
+		nodeNames.forEach(function(nodeName) {
+			var checkArgs = checkSetup(
+				'<dl><div role="presentation"><' +
+					nodeName +
+					' id="target">My list item</' +
+					nodeName +
+					'></div></dl>'
+			);
+			assert.isTrue(checks.dlitem.evaluate.apply(null, checkArgs));
+		});
+	});
+
+	it('returns false if the dd/dt is in a div[role=none] with a dl as grandparent', function() {
+		const nodeNames = ['dd', 'dt'];
+		nodeNames.forEach(function(nodeName) {
+			var checkArgs = checkSetup(
+				'<dl><div role="none"><' +
+					nodeName +
+					' id="target">My list item</' +
+					nodeName +
+					'></div></dl>'
+			);
+			assert.isTrue(checks.dlitem.evaluate.apply(null, checkArgs));
+		});
+	})(shadowSupport.v1 ? it : xit)(
 		'should return true in a shadow DOM pass',
 		function() {
 			var node = document.createElement('div');
@@ -88,4 +144,17 @@ describe('dlitem', function() {
 			assert.isFalse(checks.dlitem.evaluate.apply(null, checkArgs));
 		}
 	);
+
+	(shadowSupport.v1 ? it : xit)(
+		'should return truewhen the item is grouped in dl > div',
+		function() {
+			var node = document.createElement('div');
+			node.innerHTML = '<dt>My list item </dt>';
+			var shadow = node.attachShadow({ mode: 'open' });
+			shadow.innerHTML = '<dl><div><slot></slot></div></dl>';
+
+			var checkArgs = checkSetup(node, 'dt');
+			assert.isTrue(checks.dlitem.evaluate.apply(null, checkArgs));
+		}
+	);
 });
diff --git a/test/integration/rules/dlitem/dlitem.html b/test/integration/rules/dlitem/dlitem.html
index 3c92602827..429bfa0d6b 100644
--- a/test/integration/rules/dlitem/dlitem.html
+++ b/test/integration/rules/dlitem/dlitem.html
@@ -3,8 +3,16 @@
 <dl>
 	<dt id="contained">Does belong to a list.</dt>
 	<dd id="alsocontained">Also belongs to a list.</dd>
+	<div>
+		<dt id="grouped">Grouped in a div.</dt>
+		<dd id="alsogrouped">Also grouped in a div.</dd>
+	</div>
+	<div role="main">
+		<dt id="incorrectlygrouped">Grouped in a div with role.</dt>
+		<dd id="alsoincorrectlygrouped">Also grouped in a div with role.</dd>
+	</div>
 </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
+</dl>
diff --git a/test/integration/rules/dlitem/dlitem.json b/test/integration/rules/dlitem/dlitem.json
index 072c342503..d7f5c7ce6d 100644
--- a/test/integration/rules/dlitem/dlitem.json
+++ b/test/integration/rules/dlitem/dlitem.json
@@ -5,7 +5,9 @@
 		["#uncontained"],
 		["#alsouncontained"],
 		["#uncontainedbyrole"],
-		["#alsouncontainedbyrole"]
+		["#alsouncontainedbyrole"],
+		["#incorrectlygrouped"],
+		["#alsoincorrectlygrouped"]
 	],
-	"passes": [["#contained"], ["#alsocontained"]]
+	"passes": [["#contained"], ["#alsocontained"], ["#grouped"], ["#alsogrouped"]]
 }

From f6f1fc6e6abd6efda1c312da1d14aa93f7b559fe Mon Sep 17 00:00:00 2001
From: Wilco Fiers <WilcoFiers@users.noreply.github.com>
Date: Sun, 30 Dec 2018 11:29:21 +0100
Subject: [PATCH 2/2] chore: Update test descriptions

---
 test/checks/lists/dlitem.js | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/test/checks/lists/dlitem.js b/test/checks/lists/dlitem.js
index 0c5aff043a..38a3ccceb0 100644
--- a/test/checks/lists/dlitem.js
+++ b/test/checks/lists/dlitem.js
@@ -79,7 +79,7 @@ describe('dlitem', function() {
 		});
 	});
 
-	it('returns false if the dd/dt is in a div with a role with a dl as grandparent', function() {
+	it('returns false if the dd/dt is in a div with a role with a dl as grandparent with a list role', function() {
 		const nodeNames = ['dd', 'dt'];
 		nodeNames.forEach(function(nodeName) {
 			var checkArgs = checkSetup(
@@ -146,7 +146,7 @@ describe('dlitem', function() {
 	);
 
 	(shadowSupport.v1 ? it : xit)(
-		'should return truewhen the item is grouped in dl > div',
+		'should return true when the item is grouped in dl > div in a shadow DOM',
 		function() {
 			var node = document.createElement('div');
 			node.innerHTML = '<dt>My list item </dt>';