fix(multiple-label): no longer raises issue when aria-labelledby over…

…rides how AT views multiple labels
dequelabs · May 7, 2019 · 3f96aa1 · 3f96aa1
1 parent b573b1c
commit 3f96aa1
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 22 deletions.
diff --git a/lib/checks/label/multiple-label.js b/lib/checks/label/multiple-label.js
@@ -4,15 +4,7 @@ let parent = node.parentNode;
 
 if (labels.length) {
 	// filter out hidden labels because they're fine
-	// except: fail first label if hidden because of VO
-	labels = labels.filter(function(label, index) {
-		if (
-			(index === 0 && !axe.commons.dom.isVisible(label, true)) ||
-			axe.commons.dom.isVisible(label, true)
-		) {
-			return label;
-		}
-	});
+	labels = labels.filter(label => axe.commons.dom.isVisible(label, true));
 }
 
 while (parent) {
@@ -26,4 +18,14 @@ while (parent) {
 }
 
 this.relatedNodes(labels);
+
+if (labels.length > 1) {
+	const labelledby = axe.commons.dom.idrefs(node, 'aria-labelledby');
+	return !labels.every(function(label) {
+		return (
+			labelledby.includes(label) || !axe.commons.dom.isVisible(label, true)
+		);
+	});
+}
+
 return labels.length > 1;
diff --git a/test/checks/label/multiple-label.js b/test/checks/label/multiple-label.js
@@ -69,19 +69,7 @@ describe('multiple-label', function() {
 		assert.deepEqual(checkContext._relatedNodes, [l1]);
 	});
 
-	it('should return true if there are multiple explicit labels but the first one is hidden', function() {
-		fixture.innerHTML =
-			'<label style="display:none" for="test-input2" id="l1">label one</label>' +
-			'<label for="test-input2" id="l2">label two</label>' +
-			'<input id="test-input2" type="text">';
-		var target = fixture.querySelector('#test-input2');
-		var l1 = fixture.querySelector('#l1');
-		var l2 = fixture.querySelector('#l2');
-		assert.isTrue(checks['multiple-label'].evaluate.call(checkContext, target));
-		assert.deepEqual(checkContext._relatedNodes, [l1, l2]);
-	});
-
-	it('should return true if there are multiple explicit labels but the first one is hidden', function() {
+	it('should return true if there are multiple explicit labels but some are hidden', function() {
 		fixture.innerHTML =
 			'<label for="me" id="l1">visible</label>' +
 			'<label for="me" style="display:none;" id="l2">hidden</label>' +
@@ -112,4 +100,38 @@ describe('multiple-label', function() {
 			checks['multiple-label'].evaluate.call(checkContext, target)
 		);
 	});
+
+	it('should return false given multiple labels if aria-labelledby points to visible label', function() {
+		fixture.innerHTML = '<label for="target" style="display:none">Foo</label>';
+		fixture.innerHTML += '<label for="target" id="lbl2">Baz</label>';
+		fixture.innerHTML +=
+			'<input type="text" id="target" aria-labelledby="lbl2">';
+		var target = fixture.querySelector('#target');
+		assert.isFalse(
+			checks['multiple-label'].evaluate.call(checkContext, target)
+		);
+	});
+
+	it('should return false given multiple labels if aria-labelledby points non aria-hidden label', function() {
+		fixture.innerHTML = '<label for="target" aria-hidden="true">Foo</label>';
+		fixture.innerHTML += '<label for="target" id="lbl2">Baz</label>';
+		fixture.innerHTML +=
+			'<input type="text" id="target" aria-labelledby="lbl2">';
+		var target = fixture.querySelector('#target');
+		assert.isFalse(
+			checks['multiple-label'].evaluate.call(checkContext, target)
+		);
+	});
+
+	it('should return false given multiple labels if aria-labelledby points to all labels', function() {
+		fixture.innerHTML =
+			'<label for="target" id="lbl1" aria-hidden="true">Foo</label>';
+		fixture.innerHTML += '<label for="target" id="lbl2">Baz</label>';
+		fixture.innerHTML +=
+			'<input type="text" id="target" aria-labelledby="lbl1 lbl2">';
+		var target = fixture.querySelector('#target');
+		assert.isFalse(
+			checks['multiple-label'].evaluate.call(checkContext, target)
+		);
+	});
 });