fix: Allow div groups for dlitem rule (#1284)

* fix: Allow div groups for dlitem rule

* chore: Update test descriptions

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;
 }
 

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 with a list role', 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 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>';
+			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));
+		}
+	);
 });

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>
+</dl>

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"]]
 }