From 8fa0964f5fd15fabac988d5a135735e0bc481944 Mon Sep 17 00:00:00 2001
From: Steven Lambert <steven.lambert@deque.com>
Date: Thu, 14 Nov 2019 08:14:49 -0700
Subject: [PATCH] fix(listitem): clarify that li elements must be contained in
 a list or role=list (#1894)

* fix(listitem): clarify that li elements must be contained in a list or role=list

* add message to remediation

* remove comment
---
 lib/checks/lists/listitem.js   |  1 +
 lib/checks/lists/listitem.json |  2 +-
 test/checks/lists/listitem.js  | 65 +++++++++++++++++++---------------
 3 files changed, 38 insertions(+), 30 deletions(-)

diff --git a/lib/checks/lists/listitem.js b/lib/checks/lists/listitem.js
index a53deec328..d6fd712275 100644
--- a/lib/checks/lists/listitem.js
+++ b/lib/checks/lists/listitem.js
@@ -12,6 +12,7 @@ if (parentRole === 'list') {
 }
 
 if (parentRole && axe.commons.aria.isValidRole(parentRole)) {
+	this.data('roleNotValid');
 	return false;
 }
 
diff --git a/lib/checks/lists/listitem.json b/lib/checks/lists/listitem.json
index 1f7057a0aa..b1cd6fcce4 100644
--- a/lib/checks/lists/listitem.json
+++ b/lib/checks/lists/listitem.json
@@ -5,7 +5,7 @@
 		"impact": "serious",
 		"messages": {
 			"pass": "List item has a <ul>, <ol> or role=\"list\" parent element",
-			"fail": "List item does not have a <ul>, <ol> or role=\"list\" parent element"
+			"fail": "List item does not have a <ul>, <ol>{{? it.data === 'roleNotValid'}} without a role, or a role=\"list\"{{?}} parent element"
 		}
 	}
 }
diff --git a/test/checks/lists/listitem.js b/test/checks/lists/listitem.js
index 72c3e52f64..670748e707 100644
--- a/test/checks/lists/listitem.js
+++ b/test/checks/lists/listitem.js
@@ -2,65 +2,71 @@ describe('listitem', function() {
 	'use strict';
 
 	var fixture = document.getElementById('fixture');
-	var checkSetup = axe.testUtils.checkSetup;
 	var shadowSupport = axe.testUtils.shadowSupport;
+	var checkContext = axe.testUtils.MockCheckContext();
 
 	afterEach(function() {
 		fixture.innerHTML = '';
+		checkContext.reset();
 	});
 
 	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));
+		fixture.innerHTML = '<ol><li id="target">My list item</li></ol>';
+		var target = fixture.querySelector('#target');
+		assert.isTrue(checks.listitem.evaluate.call(checkContext, target));
 	});
 
 	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));
+		fixture.innerHTML = '<ul><li id="target">My list item</li></ul>';
+		var target = fixture.querySelector('#target');
+		assert.isTrue(checks.listitem.evaluate.call(checkContext, target));
 	});
 
 	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));
+		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));
 	});
 
 	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));
+		fixture.innerHTML = '<div><li id="target">My list item</li></div>';
+		var target = fixture.querySelector('#target');
+		assert.isFalse(checks.listitem.evaluate.call(checkContext, target));
 	});
 
 	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));
+		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));
+		assert.equal(checkContext._data, 'roleNotValid');
 	});
 
 	it('should pass if the listitem has a parent <ol> with an invalid role', function() {
-		var checkArgs = checkSetup(
-			'<ol role="invalid-role"><li id="target">My list item</li></ol>'
-		);
-		assert.isTrue(checks.listitem.evaluate.apply(null, checkArgs));
+		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));
 	});
 
 	it('should pass if the listitem has a parent <ol> with an abstract role', function() {
-		var checkArgs = checkSetup(
-			'<ol role="section"><li id="target">My list item</li></ol>'
-		);
-		assert.isTrue(checks.listitem.evaluate.apply(null, checkArgs));
+		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));
 	});
 
 	(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>';
+			node.innerHTML = '<li id="target">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));
+			fixture.appendChild(node);
+			var target = node.querySelector('#target');
+			assert.isTrue(checks.listitem.evaluate.call(checkContext, target));
 		}
 	);
 
@@ -68,11 +74,12 @@ describe('listitem', function() {
 		'should return false in a shadow DOM fail',
 		function() {
 			var node = document.createElement('div');
-			node.innerHTML = '<li>My list item </li>';
+			node.innerHTML = '<li id="target">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));
+			fixture.appendChild(node);
+			var target = node.querySelector('#target');
+			assert.isFalse(checks.listitem.evaluate.call(checkContext, target));
 		}
 	);
 });