From c9b310d1f55effc71fc03e2ce41fa6d66fa66dd6 Mon Sep 17 00:00:00 2001
From: Wilco Fiers <WilcoFiers@users.noreply.github.com>
Date: Wed, 23 Jan 2019 15:06:27 +0100
Subject: [PATCH] feat(rule): Require unique aria labels in checkboxgroup &
 radiogroup (#1316)

---
 lib/checks/forms/group-labelledby-after.js    |   3 +
 lib/checks/forms/group-labelledby.js          |  81 +++++---
 lib/checks/forms/group-labelledby.json        |   4 +-
 test/checks/forms/group-labelledby.js         | 191 ++++++++++++++----
 .../rules/checkboxgroup/checkboxgroup.html    |  19 +-
 .../rules/checkboxgroup/checkboxgroup.json    |   2 +-
 .../rules/radiogroup/radiogroup.html          |  19 +-
 .../rules/radiogroup/radiogroup.json          |   2 +-
 8 files changed, 245 insertions(+), 76 deletions(-)

diff --git a/lib/checks/forms/group-labelledby-after.js b/lib/checks/forms/group-labelledby-after.js
index 00edf8f5af..27b5f77d6c 100644
--- a/lib/checks/forms/group-labelledby-after.js
+++ b/lib/checks/forms/group-labelledby-after.js
@@ -1,5 +1,8 @@
 var seen = {};
 
+// Filter out nodes that have the same type and name.
+// If you have two `<input type="radio" name="foo" />` elements
+// only the first one can pass / fail the rule.
 return results.filter(function(result) {
 	var data = result.data;
 	if (data) {
diff --git a/lib/checks/forms/group-labelledby.js b/lib/checks/forms/group-labelledby.js
index 2612cb9455..e4186f2eb7 100644
--- a/lib/checks/forms/group-labelledby.js
+++ b/lib/checks/forms/group-labelledby.js
@@ -1,34 +1,61 @@
-this.data({
-	name: node.getAttribute('name'),
-	type: node.getAttribute('type')
-});
+const { dom, text, utils } = axe.commons;
+
+const type = utils.escapeSelector(node.type);
+const name = utils.escapeSelector(node.name);
+const doc = dom.getRootNode(node);
+const data = {
+	name: node.name,
+	type: node.type
+};
 
-var doc = axe.commons.dom.getRootNode(node);
-var matchingNodes = doc.querySelectorAll(
-	'input[type="' +
-		axe.commons.utils.escapeSelector(node.type) +
-		'"][name="' +
-		axe.commons.utils.escapeSelector(node.name) +
-		'"]'
+const matchingNodes = Array.from(
+	doc.querySelectorAll(`input[type="${type}"][name="${name}"]`)
 );
+// There is only one node with this name & type, so there's no need for a group
 if (matchingNodes.length <= 1) {
+	this.data(data);
 	return true;
 }
 
-// Check to see if there's an aria-labelledby value that all nodes have in common
-return (
-	[].map
-		.call(matchingNodes, function(m) {
-			var l = m.getAttribute('aria-labelledby');
-			return l ? l.split(/\s+/) : [];
-		})
-		.reduce(function(prev, curr) {
-			return prev.filter(function(n) {
-				return curr.includes(n);
-			});
-		})
-		.filter(function(n) {
-			var labelNode = doc.getElementById(n);
-			return labelNode && axe.commons.text.accessibleText(labelNode, true);
-		}).length !== 0
+let sharedLabels = dom.idrefs(node, 'aria-labelledby').filter(label => !!label); // Filter for "null" labels
+let uniqueLabels = sharedLabels.slice();
+
+// Figure out which labels are unique, which are shared by all items, or neither
+matchingNodes.forEach(groupItem => {
+	if (groupItem === node) {
+		return;
+	}
+	// Find new labels associated with current groupItem
+	const labels = dom
+		.idrefs(groupItem, 'aria-labelledby')
+		.filter(newLabel => newLabel);
+
+	sharedLabels = sharedLabels.filter(sharedLabel =>
+		labels.includes(sharedLabel)
+	);
+	uniqueLabels = uniqueLabels.filter(
+		uniqueLabel => !labels.includes(uniqueLabel)
+	);
+});
+
+// filter items with no accessible name, do this last for performance reasons
+uniqueLabels = uniqueLabels.filter(labelNode =>
+	text.accessibleText(labelNode, true)
+);
+sharedLabels = sharedLabels.filter(labelNode =>
+	text.accessibleText(labelNode, true)
 );
+
+if (uniqueLabels.length > 0 && sharedLabels.length > 0) {
+	this.data(data);
+	return true;
+}
+
+if (uniqueLabels.length > 0 && sharedLabels.length === 0) {
+	data.failureCode = 'no-shared-label';
+} else if (uniqueLabels.length === 0 && sharedLabels.length > 0) {
+	data.failureCode = 'no-unique-label';
+}
+
+this.data(data);
+return false;
diff --git a/lib/checks/forms/group-labelledby.json b/lib/checks/forms/group-labelledby.json
index d5fb036e7a..34b4f1e18f 100644
--- a/lib/checks/forms/group-labelledby.json
+++ b/lib/checks/forms/group-labelledby.json
@@ -5,8 +5,8 @@
 	"metadata": {
 		"impact": "critical",
 		"messages": {
-			"pass": "All elements with the name \"{{=it.data.name}}\" reference the same element with aria-labelledby",
-			"fail": "All elements with the name \"{{=it.data.name}}\" do not reference the same element with aria-labelledby"
+			"pass": "Elements with the name \"{{=it.data.name}}\" have both a shared label, and a unique label, referenced through aria-labelledby",
+			"fail": "{{var code = it.data && it.data.failureCode;}}Elements with the name \"{{=it.data.name}}\" do not all have {{? code === 'no-shared-label' }}a shared label{{?? code === 'no-unique-label' }}a unique label{{??}}both a shared label, and a unique label{{?}}, referenced through aria-labelledby"
 		}
 	}
 }
diff --git a/test/checks/forms/group-labelledby.js b/test/checks/forms/group-labelledby.js
index 258635a08b..daca2f19f6 100644
--- a/test/checks/forms/group-labelledby.js
+++ b/test/checks/forms/group-labelledby.js
@@ -28,7 +28,7 @@ describe('group-labelledby', function() {
 					fixtureSetup(
 						'<input type="' +
 							type +
-							'" id="target" name="uniqueyname">' +
+							'" id="target" name="groupname">' +
 							'<input type="' +
 							type +
 							'" name="differentname">'
@@ -47,16 +47,16 @@ describe('group-labelledby', function() {
 					fixtureSetup(
 						'<input type="' +
 							type +
-							'" id="target" name="uniqueyname">' +
+							'" id="target" name="groupname">' +
 							'<input type="' +
 							type +
-							'" name="uniqueyname">'
+							'" name="groupname">'
 					);
 
 					var node = fixture.querySelector('#target');
 					assert.isFalse(check.evaluate.call(checkContext, node));
 					assert.deepEqual(checkContext._data, {
-						name: 'uniqueyname',
+						name: 'groupname',
 						type: type
 					});
 				}
@@ -70,23 +70,79 @@ describe('group-labelledby', function() {
 					fixtureSetup(
 						'<input type="' +
 							type +
-							'" id="target" aria-labelledby="unique one" name="uniqueyname">' +
+							'" id="target" aria-labelledby="unique one" name="groupname">' +
 							'<input type="' +
 							type +
-							'" aria-labelledby="notshared two" name="uniqueyname">' +
+							'" aria-labelledby="notshared two" name="groupname">' +
 							'<input type="' +
 							type +
-							'" aria-labelledby="different three" name="uniqueyname">'
+							'" aria-labelledby="different three" name="groupname">'
 					);
 					var node = fixture.querySelector('#target');
 					assert.isFalse(check.evaluate.call(checkContext, node));
 					assert.deepEqual(checkContext._data, {
-						name: 'uniqueyname',
+						name: 'groupname',
 						type: type
 					});
 				}
 			);
 
+			it(
+				'should return false if there are ungrouped ' +
+					type +
+					' elements with the same name and with shared labelledby but no unique label',
+				function() {
+					fixtureSetup(
+						'<p id="shared">Shared label</p>' +
+							'<input type="' +
+							type +
+							'" id="target" aria-labelledby="shared" name="groupname">' +
+							'<input type="' +
+							type +
+							'" aria-labelledby="shared" name="groupname">' +
+							'<input type="' +
+							type +
+							'" aria-labelledby="shared" name="groupname">'
+					);
+					var node = fixture.querySelector('#target');
+					assert.isFalse(check.evaluate.call(checkContext, node));
+					assert.deepEqual(checkContext._data, {
+						name: 'groupname',
+						type: type,
+						failureCode: 'no-unique-label'
+					});
+				}
+			);
+
+			it(
+				'should return false if there are ungrouped ' +
+					type +
+					' elements with the same name and with unique labelledby but no shared label',
+				function() {
+					fixtureSetup(
+						'<p id="one">Label 1</p>' +
+							'<p id="two">Label 2</p>' +
+							'<p id="three">Label 3</p>' +
+							'<input type="' +
+							type +
+							'" id="target" aria-labelledby="one" name="groupname">' +
+							'<input type="' +
+							type +
+							'" aria-labelledby="two" name="groupname">' +
+							'<input type="' +
+							type +
+							'" aria-labelledby="three" name="groupname">'
+					);
+					var node = fixture.querySelector('#target');
+					assert.isFalse(check.evaluate.call(checkContext, node));
+					assert.deepEqual(checkContext._data, {
+						name: 'groupname',
+						type: type,
+						failureCode: 'no-shared-label'
+					});
+				}
+			);
+
 			it(
 				'should return false if there are ungrouped ' +
 					type +
@@ -96,19 +152,19 @@ describe('group-labelledby', function() {
 					fixtureSetup(
 						'<input type="' +
 							type +
-							'" id="target" aria-labelledby="shared one" name="uniqueyname">' +
+							'" id="target" aria-labelledby="shared one" name="groupname">' +
 							'<input type="' +
 							type +
-							'" aria-labelledby="shared two" name="uniqueyname">' +
+							'" aria-labelledby="shared two" name="groupname">' +
 							'<input type="' +
 							type +
-							'" aria-labelledby="shared three" name="uniqueyname">'
+							'" aria-labelledby="shared three" name="groupname">'
 					);
 
 					var node = fixture.querySelector('#target');
 					assert.isFalse(check.evaluate.call(checkContext, node));
 					assert.deepEqual(checkContext._data, {
-						name: 'uniqueyname',
+						name: 'groupname',
 						type: type
 					});
 				}
@@ -124,19 +180,19 @@ describe('group-labelledby', function() {
 						'<p id="shared"></p>' +
 							'<input type="' +
 							type +
-							'" id="target" aria-labelledby="shared one" name="uniqueyname">' +
+							'" id="target" aria-labelledby="shared one" name="groupname">' +
 							'<input type="' +
 							type +
-							'" aria-labelledby="shared two" name="uniqueyname">' +
+							'" aria-labelledby="shared two" name="groupname">' +
 							'<input type="' +
 							type +
-							'" aria-labelledby="shared three" name="uniqueyname">'
+							'" aria-labelledby="shared three" name="groupname">'
 					);
 
 					var node = fixture.querySelector('#target');
 					assert.isFalse(check.evaluate.call(checkContext, node));
 					assert.deepEqual(checkContext._data, {
-						name: 'uniqueyname',
+						name: 'groupname',
 						type: type
 					});
 				}
@@ -145,26 +201,29 @@ describe('group-labelledby', function() {
 			it(
 				'should return true if there are ungrouped ' +
 					type +
-					' elements with the same name and with shared labelledby' +
+					' elements with the same name and with shared and unique labelledby ' +
 					'pointing to a node with text content',
 				function() {
 					fixtureSetup(
-						'<p id="shared">Label</p>' +
+						'<p id="one">Label 1</p>' +
+							'<p id="two">Label 2</p>' +
+							'<p id="three">Label 3</p>' +
+							'<p id="shared">Shared label</p>' +
 							'<input type="' +
 							type +
-							'" id="target" aria-labelledby="shared one" name="uniqueyname">' +
+							'" id="target" aria-labelledby="shared one" name="groupname">' +
 							'<input type="' +
 							type +
-							'" aria-labelledby="shared two" name="uniqueyname">' +
+							'" aria-labelledby="shared two" name="groupname">' +
 							'<input type="' +
 							type +
-							'" aria-labelledby="shared three" name="uniqueyname">'
+							'" aria-labelledby="shared three" name="groupname">'
 					);
 
 					var node = fixture.querySelector('#target');
 					assert.isTrue(check.evaluate.call(checkContext, node));
 					assert.deepEqual(checkContext._data, {
-						name: 'uniqueyname',
+						name: 'groupname',
 						type: type
 					});
 				}
@@ -177,22 +236,27 @@ describe('group-labelledby', function() {
 					'pointing to a node with text content',
 				function() {
 					fixtureSetup(
-						'<p id="shared" style="display:none">Label</p>' +
+						'<div style="display:none">' +
+							'  <p id="one">Label 1</p>' +
+							'  <p id="two">Label 2</p>' +
+							'  <p id="three">Label 3</p>' +
+							'  <p id="shared">Shared label</p>' +
+							'</div>' +
 							'<input type="' +
 							type +
-							'" id="target" aria-labelledby="shared one" name="uniqueyname">' +
+							'" id="target" aria-labelledby="shared one" name="groupname">' +
 							'<input type="' +
 							type +
-							'" aria-labelledby="shared two" name="uniqueyname">' +
+							'" aria-labelledby="shared two" name="groupname">' +
 							'<input type="' +
 							type +
-							'" aria-labelledby="shared three" name="uniqueyname">'
+							'" aria-labelledby="shared three" name="groupname">'
 					);
 
 					var node = fixture.querySelector('#target');
 					assert.isTrue(check.evaluate.call(checkContext, node));
 					assert.deepEqual(checkContext._data, {
-						name: 'uniqueyname',
+						name: 'groupname',
 						type: type
 					});
 				}
@@ -205,7 +269,10 @@ describe('group-labelledby', function() {
 					'pointing to a node with text content - SPECIAL CHARACTERS',
 				function() {
 					fixtureSetup(
-						'<p id="shared">Label</p>' +
+						'<p id="one">Label 1</p>' +
+							'<p id="two">Label 2</p>' +
+							'<p id="three">Label 3</p>' +
+							'<p id="shared">Shared label</p>' +
 							'<input type="' +
 							type +
 							'" id="target" aria-labelledby="shared one" name="s$.#0">' +
@@ -227,23 +294,28 @@ describe('group-labelledby', function() {
 			);
 
 			(shadowSupport ? it : xit)(
-				'should return false if label is outside of shadow boundary',
+				'should return false if the shared label is outside of shadow boundary',
 				function() {
 					fixture.innerHTML =
-						'<div id="container"><h2 id="shared">Label</h2></div>';
+						'<div id="container">' +
+						'  <p id="shared">Shared label</p>' +
+						'</div>';
 					var shadowRoot = fixture
 						.querySelector('#container')
 						.attachShadow({ mode: 'open' });
 					shadowRoot.innerHTML =
+						'<p id="one">Label 1</p>' +
+						'<p id="two">Label 2</p>' +
+						'<p id="three">Label 3</p>' +
 						'<input type="' +
 						type +
-						'" id="target" aria-labelledby="shared one" name="uniqueyname">' +
+						'" id="target" aria-labelledby="shared one" name="groupname">' +
 						'<input type="' +
 						type +
-						'" aria-labelledby="shared two" name="uniqueyname">' +
+						'" aria-labelledby="shared two" name="groupname">' +
 						'<input type="' +
 						type +
-						'" aria-labelledby="shared three" name="uniqueyname">';
+						'" aria-labelledby="shared three" name="groupname">';
 
 					var tree = (axe._tree = axe.utils.getFlattenedTree(fixture));
 					var shadowContent = shadowRoot.querySelector('#target');
@@ -251,6 +323,49 @@ describe('group-labelledby', function() {
 
 					var params = [shadowContent, undefined, virtualTarget];
 					assert.isFalse(check.evaluate.apply(checkContext, params));
+					assert.deepEqual(checkContext._data, {
+						name: 'groupname',
+						type: type,
+						failureCode: 'no-shared-label'
+					});
+				}
+			);
+
+			(shadowSupport ? it : xit)(
+				'should return false if the unique label is outside of shadow boundary',
+				function() {
+					fixture.innerHTML =
+						'<div id="container">' +
+						'  <p id="one">Label 1</p>' +
+						'  <p id="two">Label 2</p>' +
+						'  <p id="three">Label 3</p>' +
+						'</div>';
+					var shadowRoot = fixture
+						.querySelector('#container')
+						.attachShadow({ mode: 'open' });
+					shadowRoot.innerHTML =
+						'<p id="shared">Shared label</p>' +
+						'<input type="' +
+						type +
+						'" id="target" aria-labelledby="shared one" name="groupname">' +
+						'<input type="' +
+						type +
+						'" aria-labelledby="shared two" name="groupname">' +
+						'<input type="' +
+						type +
+						'" aria-labelledby="shared three" name="groupname">';
+
+					var tree = (axe._tree = axe.utils.getFlattenedTree(fixture));
+					var shadowContent = shadowRoot.querySelector('#target');
+					var virtualTarget = axe.utils.getNodeFromTree(tree[0], shadowContent);
+
+					var params = [shadowContent, undefined, virtualTarget];
+					assert.isFalse(check.evaluate.apply(checkContext, params));
+					assert.deepEqual(checkContext._data, {
+						name: 'groupname',
+						type: type,
+						failureCode: 'no-unique-label'
+					});
 				}
 			);
 
@@ -265,13 +380,19 @@ describe('group-labelledby', function() {
 						.querySelector('#container')
 						.attachShadow({ mode: 'open' });
 					shadowRoot.innerHTML =
-						'<p id="shared">Label</p>' +
+						'<p id="one">Label 1</p>' +
+						'<p id="two">Label 2</p>' +
+						'<p id="three">Label 3</p>' +
+						'<p id="shared">Shared label</p>' +
 						'<input type="' +
 						type +
 						'" name="samename" aria-labelledby="shared one">' +
 						'<input type="' +
 						type +
-						'" id="target" name="samename" aria-labelledby="shared two">';
+						'" id="target" name="samename" aria-labelledby="shared two">' +
+						'<input type="' +
+						type +
+						'" id="target" name="samename" aria-labelledby="shared three">';
 
 					var tree = (axe._tree = axe.utils.getFlattenedTree(fixture));
 					var shadowContent = shadowRoot.querySelector('#target');
diff --git a/test/integration/rules/checkboxgroup/checkboxgroup.html b/test/integration/rules/checkboxgroup/checkboxgroup.html
index 089dc72adb..a6452dcaa6 100644
--- a/test/integration/rules/checkboxgroup/checkboxgroup.html
+++ b/test/integration/rules/checkboxgroup/checkboxgroup.html
@@ -1,13 +1,22 @@
 <form action="#">
 	<input type="checkbox" id="noname">
-	<input type="checkbox" name="dupe" id="fail1">
-	<input type="checkbox" name="dupe">
+	<input type="checkbox" name="fail" id="fail1">
+	<input type="checkbox" name="fail">
+
+	<input type="checkbox" name="label-fail1" id="fail2" aria-labelledby="grouplabel">
+	<input type="checkbox" name="label-fail1" aria-labelledby="grouplabel">
+
+	<input type="checkbox" name="label-fail2" id="fail3" aria-labelledby="label1">
+	<input type="checkbox" name="label-fail2" aria-labelledby="label2">
 
 	<input type="checkbox" name="single" id="pass1">
 
-	<input type="checkbox" name="label" id="pass2" aria-labelledby="grouplabel">
-	<input type="checkbox" name="label" aria-labelledby="grouplabel">
-	<div id="grouplabel">Label</div>
+	<input type="checkbox" name="label" id="pass2" aria-labelledby="grouplabel label1">
+	<input type="checkbox" name="label" aria-labelledby="grouplabel label2">
+	<div id="grouplabel">Group label</div>
+	<div id="label1">Label 1</div>
+	<div id="label2">Label 2</div>
+
 	<fieldset>
 		<legend>Name</legend>
 		<input type="checkbox" name="fieldset" id="pass3">
diff --git a/test/integration/rules/checkboxgroup/checkboxgroup.json b/test/integration/rules/checkboxgroup/checkboxgroup.json
index 191b0210aa..770b3b3fca 100644
--- a/test/integration/rules/checkboxgroup/checkboxgroup.json
+++ b/test/integration/rules/checkboxgroup/checkboxgroup.json
@@ -1,7 +1,7 @@
 {
 	"description": "checkboxgroup test",
 	"rule": "checkboxgroup",
-	"violations": [["#fail1"]],
+	"violations": [["#fail1"], ["#fail2"], ["#fail3"]],
 	"passes": [
 		["#pass1"],
 		["#pass2"],
diff --git a/test/integration/rules/radiogroup/radiogroup.html b/test/integration/rules/radiogroup/radiogroup.html
index 259fbcfae0..e502edc342 100644
--- a/test/integration/rules/radiogroup/radiogroup.html
+++ b/test/integration/rules/radiogroup/radiogroup.html
@@ -1,14 +1,23 @@
 
 <form action="#">
 	<input type="radio" id="noname">
-	<input type="radio" name="dupe" id="fail1">
-	<input type="radio" name="dupe">
+	<input type="radio" name="fail" id="fail1">
+	<input type="radio" name="fail">
+
+	<input type="radio" name="label-fail1" id="fail2" aria-labelledby="grouplabel">
+	<input type="radio" name="label-fail1" aria-labelledby="grouplabel">
+
+	<input type="radio" name="label-fail2" id="fail3" aria-labelledby="label1">
+	<input type="radio" name="label-fail2" aria-labelledby="label2">
 
 	<input type="radio" name="single" id="pass1">
 
-	<input type="radio" name="label" id="pass2" aria-labelledby="grouplabel">
-	<input type="radio" name="label" aria-labelledby="grouplabel">
-	<div id="grouplabel">Label</div>
+	<input type="radio" name="label" id="pass2" aria-labelledby="grouplabel label1">
+	<input type="radio" name="label" aria-labelledby="grouplabel label2">
+	<div id="grouplabel">Group label</div>
+	<div id="label1">Label 1</div>
+	<div id="label2">Label 2</div>
+
 	<fieldset>
 		<legend>Name</legend>
 		<input type="radio" name="fieldset" id="pass3">
diff --git a/test/integration/rules/radiogroup/radiogroup.json b/test/integration/rules/radiogroup/radiogroup.json
index deca321cfe..3ea21e5985 100644
--- a/test/integration/rules/radiogroup/radiogroup.json
+++ b/test/integration/rules/radiogroup/radiogroup.json
@@ -1,7 +1,7 @@
 {
 	"description": "radiogroup test",
 	"rule": "radiogroup",
-	"violations": [["#fail1"]],
+	"violations": [["#fail1"], ["#fail2"], ["#fail3"]],
 	"passes": [
 		["#pass1"],
 		["#pass2"],