From ae07b8e8144d342163510fdb19e028926b5a74f6 Mon Sep 17 00:00:00 2001
From: Marcy Sutton <marcy.sutton@deque.com>
Date: Tue, 20 Mar 2018 14:42:17 -0700
Subject: [PATCH] fix(rule): improve messaging for hidden labels

Closes https://github.com/dequelabs/axe-core/issues/242
---
 lib/checks/label/explicit.js                |  7 ++-
 lib/checks/label/hidden-explicit-label.js   | 10 +++++
 lib/checks/label/hidden-explicit-label.json | 11 +++++
 lib/rules/label.json                        |  3 +-
 test/checks/label/explicit.js               | 10 ++---
 test/checks/label/hidden-explicit-label.js  | 49 +++++++++++++++++++++
 test/integration/rules/label/label.html     |  1 +
 test/integration/rules/label/label.json     |  1 +
 8 files changed, 85 insertions(+), 7 deletions(-)
 create mode 100644 lib/checks/label/hidden-explicit-label.js
 create mode 100644 lib/checks/label/hidden-explicit-label.json
 create mode 100644 test/checks/label/hidden-explicit-label.js

diff --git a/lib/checks/label/explicit.js b/lib/checks/label/explicit.js
index eee064f700..dc314576ea 100644
--- a/lib/checks/label/explicit.js
+++ b/lib/checks/label/explicit.js
@@ -4,7 +4,12 @@ if (node.getAttribute('id')) {
 	const label = root.querySelector(`label[for="${id}"]`);
 
 	if (label) {
-		return !!axe.commons.text.accessibleText(label);
+		// defer to hidden-explicit-label check for better messaging
+		if (!axe.commons.dom.isVisible(label)) {
+			return true;
+		} else {
+			return !!axe.commons.text.accessibleText(label);
+		}
 	}
 }
 return false;
diff --git a/lib/checks/label/hidden-explicit-label.js b/lib/checks/label/hidden-explicit-label.js
new file mode 100644
index 0000000000..0fcdd0dd24
--- /dev/null
+++ b/lib/checks/label/hidden-explicit-label.js
@@ -0,0 +1,10 @@
+if (node.getAttribute('id')) {
+	const root = axe.commons.dom.getRootNode(node);
+	const id = axe.commons.utils.escapeSelector(node.getAttribute('id'));
+	const label = root.querySelector(`label[for="${id}"]`);
+
+	if (label && !axe.commons.dom.isVisible(label)) {
+		return true;
+	}
+}
+return false;
diff --git a/lib/checks/label/hidden-explicit-label.json b/lib/checks/label/hidden-explicit-label.json
new file mode 100644
index 0000000000..70d342c103
--- /dev/null
+++ b/lib/checks/label/hidden-explicit-label.json
@@ -0,0 +1,11 @@
+{
+  "id": "hidden-explicit-label",
+  "evaluate": "hidden-explicit-label.js",
+  "metadata": {
+    "impact": "critical",
+    "messages": {
+      "pass": "Form element has a visible explicit <label>",
+      "fail": "Form element has explicit <label> that is hidden"
+    }
+  }
+}
diff --git a/lib/rules/label.json b/lib/rules/label.json
index df36f551a2..af1747f790 100644
--- a/lib/rules/label.json
+++ b/lib/rules/label.json
@@ -24,6 +24,7 @@
   ],
   "none": [
     "help-same-as-label",
-    "multiple-label"
+    "multiple-label",
+    "hidden-explicit-label"
   ]
 }
diff --git a/test/checks/label/explicit.js b/test/checks/label/explicit.js
index 805f261062..72c4c1a3f4 100644
--- a/test/checks/label/explicit.js
+++ b/test/checks/label/explicit.js
@@ -15,14 +15,14 @@ describe('explicit-label', function () {
 		assert.isFalse(checks['explicit-label'].evaluate(node));
 	});
 
-	it('should return false if an invisible non-empty label is present', function () {
-		fixtureSetup('<label for="target" style="display: none">Label</label><input type="text" id="target">');
+	it('should return true if a non-empty label is present', function () {
+		fixtureSetup('<label for="target">Text</label><input type="text" id="target">');
 		var node = fixture.querySelector('#target');
-		assert.isFalse(checks['explicit-label'].evaluate(node));
+		assert.isTrue(checks['explicit-label'].evaluate(node));
 	});
 
-	it('should return true if a non-empty label is present', function () {
-		fixtureSetup('<label for="target">Text</label><input type="text" id="target">');
+	it('should return true if an invisible non-empty label is present, to defer to hidden-explicit-label', function () {
+		fixtureSetup('<label for="target" style="display: none;">Text</label><input type="text" id="target">');
 		var node = fixture.querySelector('#target');
 		assert.isTrue(checks['explicit-label'].evaluate(node));
 	});
diff --git a/test/checks/label/hidden-explicit-label.js b/test/checks/label/hidden-explicit-label.js
new file mode 100644
index 0000000000..c3ed1a1469
--- /dev/null
+++ b/test/checks/label/hidden-explicit-label.js
@@ -0,0 +1,49 @@
+describe('hidden-explicit-label', function () {
+	'use strict';
+
+	var fixture = document.getElementById('fixture');
+	var fixtureSetup = axe.testUtils.fixtureSetup;
+	var shadowSupport = axe.testUtils.shadowSupport;
+	var shadowCheckSetup = axe.testUtils.shadowCheckSetup;
+	var checkContext = axe.testUtils.MockCheckContext();
+
+	afterEach(function () {
+		checkContext.reset();
+	});
+
+	it('should return true if a hidden non-empty label is present', function () {
+		fixtureSetup('<label for="target" style="display:none">Text</label><input type="text" id="target">');
+		var node = fixture.querySelector('#target');
+		assert.isTrue(checks['hidden-explicit-label'].evaluate(node));
+	});
+
+	it('should return false if a visible non-empty label is present', function () {
+		fixtureSetup('<label for="target">Label</label><input type="text" id="target">');
+		var node = fixture.querySelector('#target');
+		assert.isFalse(checks['hidden-explicit-label'].evaluate(node));
+	});
+
+	it('should return true if an invisible empty label is present', function () {
+		fixtureSetup('<label for="target" style="display: none;"></label><input type="text" id="target">');
+		var node = fixture.querySelector('#target');
+		assert.isTrue(checks['explicit-label'].evaluate(node));
+	});
+
+	(shadowSupport.v1 ? it : xit)('should return true if content is inside of shadow DOM', function () {
+		var params = shadowCheckSetup(
+			'<div></div>',
+			'<label for="target" style="display:none">Text</label><input type="text" id="target">'
+		);
+
+		assert.isTrue(checks['hidden-explicit-label'].evaluate.apply(shadowCheckSetup, params));
+	});
+
+	(shadowSupport.v1 ? it : xit)('should return false if part of the pairing is inside of shadow DOM', function () {
+		var params = shadowCheckSetup(
+			'<div><label for="target" style="display:none">Text</label></div>',
+			'<input type="text" id="target">'
+		);
+
+		assert.isFalse(checks['hidden-explicit-label'].evaluate.apply(shadowCheckSetup, params));
+	});
+});
diff --git a/test/integration/rules/label/label.html b/test/integration/rules/label/label.html
index 3ef8971cc9..32a090ec66 100644
--- a/test/integration/rules/label/label.html
+++ b/test/integration/rules/label/label.html
@@ -25,6 +25,7 @@
 	<label for="fail8"></label><select id="fail8"></select>
 	<label for="fail9"></label><textarea id="fail9"></textarea>
 	<label for="fail10"><select id="fail10"><option>Thing</option></select></label>
+	<label for="fail11" style="display: none;">Text</label><input type="text" id="fail11">
 	<label for="pass10">Label</label><input type="text" id="pass10">
 	<label for="pass11">Label</label><select id="pass11"></select>
 	<label for="pass12">Label</label><textarea id="pass12"></textarea>
diff --git a/test/integration/rules/label/label.json b/test/integration/rules/label/label.json
index c2c550bfce..6ecf46b157 100644
--- a/test/integration/rules/label/label.json
+++ b/test/integration/rules/label/label.json
@@ -12,6 +12,7 @@
 		["#fail8"],
 		["#fail9"],
 		["#fail10"],
+		["#fail11"],
 		["#fail22"],
 		["#fail23"],
 		["#fail24"],