feat: Break up duplicate-id rule for ARIA+labels and active elements

doc/rule-descriptions.md

@@ -23,7 +23,9 @@
 | definition-list | Ensures <dl> elements are structured correctly | Serious | cat.structure, wcag2a, wcag131 | true |
 | dlitem | Ensures <dt> and <dd> elements are contained by a <dl> | Serious | cat.structure, wcag2a, wcag131 | true |
 | document-title | Ensures each HTML document contains a non-empty <title> element | Serious | cat.text-alternatives, wcag2a, wcag242 | true |
-| duplicate-id | Ensures every id attribute value is unique | Minor, Serious | cat.parsing, wcag2a, wcag411 | true |
+| duplicate-id-active | Ensures every id attribute value is unique | Serious | cat.parsing, wcag2a, wcag411 | true |
+| duplicate-id-aria | Ensures every id attribute value used in ARIA and in labels is unique | Critical | cat.parsing, wcag2a, wcag411 | true |
+| duplicate-id | Ensures every id attribute value is unique | Minor | cat.parsing, wcag2a, wcag411 | true |
 | empty-heading | Ensures headings have discernible text | Minor | cat.name-role-value, best-practice | true |
 | focus-order-semantics | Ensures elements in the focus order have an appropriate role | Minor | cat.keyboard, best-practice, experimental | true |
 | frame-tested | Ensures <iframe> and <frame> elements contain the axe-core script | Critical | cat.structure, review-item | true |

lib/checks/shared/duplicate-id-accessible.json → lib/checks/parsing/duplicate-id-active.json

@@ -1,10 +1,7 @@
 {
-  "id": "duplicate-id-accessible",
+  "id": "duplicate-id-active",
   "evaluate": "duplicate-id.js",
   "after": "duplicate-id-after.js",
-  "options": {
-    "accReferred": true
-  },
   "metadata": {
     "impact": "serious",
     "messages": {

lib/checks/parsing/duplicate-id-aria.json

@@ -0,0 +1,12 @@
+{
+  "id": "duplicate-id-aria",
+  "evaluate": "duplicate-id.js",
+  "after": "duplicate-id-after.js",
+  "metadata": {
+    "impact": "critical",
+    "messages": {
+      "pass": "Document has no elements that share the same id attribute",
+      "fail": "Document has multiple elements with the same id attribute: {{=it.data}}"
+    }
+  }
+}

lib/checks/shared/duplicate-id.js → lib/checks/parsing/duplicate-id.js

@@ -1,18 +1,13 @@
-const { aria, dom, utils } = axe.commons;
+const { dom, utils } = axe.commons;
 const id = node.getAttribute('id').trim();
-const { accReferred } = options || {};
+const root = dom.getRootNode(node);
 
 // Since empty ID's are not meaningful and are ignored by Edge, we'll
 // let those pass.
-if (
-	!id ||
-	(typeof accReferred === 'boolean' &&
-		accReferred === !aria.isAccessibleRef(node))
-) {
+if (!id) {
 	return true;
 }
 
-const root = dom.getRootNode(node);
 const matchingNodes = Array.from(
 	root.querySelectorAll(`[id="${utils.escapeSelector(id)}"]`)
 ).filter(foundNode => foundNode !== node);

lib/checks/shared/duplicate-id.json → lib/checks/parsing/duplicate-id.json

@@ -2,9 +2,6 @@
   "id": "duplicate-id",
   "evaluate": "duplicate-id.js",
   "after": "duplicate-id-after.js",
-  "options": {
-    "accReferred": false
-  },
   "metadata": {
     "impact": "minor",
     "messages": {

lib/commons/aria/is-accessible-ref.js

@@ -3,8 +3,8 @@ function findDomNode(node, functor) {
 	if (functor(node)) {
 		return node;
 	}
-	for (const child of node.children) {
-		const out = findDomNode(child, functor);
+	for (let i = 0; i < node.children.length; i++) {
+		const out = findDomNode(node.children[i], functor);
 		if (out) {
 			return out;
 		}

lib/rules/duplicate-id-active-matches.js

@@ -0,0 +1,8 @@
+const { dom, aria } = axe.commons;
+const id = node.getAttribute('id').trim();
+const idSelector = `*[id="${axe.utils.escapeSelector(id)}"]`;
+const idMatchingElms = Array.from(
+	dom.getRootNode(node).querySelectorAll(idSelector)
+);
+
+return idMatchingElms.some(dom.isFocusable) && !aria.isAccessibleRef(node);

lib/rules/duplicate-id-active.json

@@ -0,0 +1,20 @@
+{
+  "id": "duplicate-id-active",
+  "selector": "[id]",
+  "matches": "duplicate-id-active-matches.js",
+  "excludeHidden": false,
+  "tags": [
+    "cat.parsing",
+    "wcag2a",
+    "wcag411"
+  ],
+  "metadata": {
+    "description": "Ensures every id attribute value is unique",
+    "help": "IDs of active elements must be unique"
+  },
+  "all": [
+    "duplicate-id-active"
+  ],
+  "any": [],
+  "none": []
+}

lib/rules/duplicate-id-aria-matches.js

@@ -0,0 +1 @@
+return axe.commons.aria.isAccessibleRef(node);

lib/rules/duplicate-id-aria.json

@@ -0,0 +1,20 @@
+{
+  "id": "duplicate-id-aria",
+  "selector": "[id]",
+  "matches": "duplicate-id-aria-matches.js",
+  "excludeHidden": false,
+  "tags": [
+    "cat.parsing",
+    "wcag2a",
+    "wcag411"
+  ],
+  "metadata": {
+    "description": "Ensures every id attribute value used in ARIA and in labels is unique",
+    "help": "IDs used in ARIA and labels must be unique"
+  },
+  "all": [
+    "duplicate-id-aria"
+  ],
+  "any": [],
+  "none": []
+}

lib/rules/duplicate-id-misc-matches.js

@@ -0,0 +1,11 @@
+const { dom, aria } = axe.commons;
+const id = node.getAttribute('id').trim();
+const idSelector = `*[id="${axe.utils.escapeSelector(id)}"]`;
+const idMatchingElms = Array.from(
+	dom.getRootNode(node).querySelectorAll(idSelector)
+);
+
+return (
+	idMatchingElms.every(elm => !dom.isFocusable(elm)) &&
+	!aria.isAccessibleRef(node)
+);

lib/rules/duplicate-id.json

@@ -1,6 +1,7 @@
 {
   "id": "duplicate-id",
   "selector": "[id]",
+  "matches": "duplicate-id-misc-matches.js",
   "excludeHidden": false,
   "tags": [
     "cat.parsing",
@@ -12,8 +13,7 @@
     "help": "id attribute value must be unique"
   },
   "all": [
-    "duplicate-id",
-    "duplicate-id-accessible"
+    "duplicate-id"
   ],
   "any": [],
   "none": []

test/checks/shared/duplicate-id.js → test/checks/parser/duplicate-id.js

@@ -116,50 +116,4 @@ describe('duplicate-id', function() {
 			assert.deepEqual(checkContext._relatedNodes, [node.querySelector('p')]);
 		}
 	);
-
-	describe('options.accReferred', function() {
-		it('ignores unreffed elements with accReferred: true', function() {
-			fixture.innerHTML = '<div id="foo"></div> <span id="foo"></span>';
-			var node = fixture.querySelector('div[id="foo"]');
-			assert.isTrue(
-				checks['duplicate-id'].evaluate.call(checkContext, node, {
-					accReferred: true
-				})
-			);
-		});
-
-		it('tests reffed elements with accReferred: true', function() {
-			fixture.innerHTML =
-				'<div id="foo"></div> <span id="foo"></span>' +
-				'<div aria-labelledby="foo"></div>';
-			var node = fixture.querySelector('div[id="foo"]');
-			assert.isFalse(
-				checks['duplicate-id'].evaluate.call(checkContext, node, {
-					accReferred: true
-				})
-			);
-		});
-
-		it('ignores reffed elements with accReferred: false', function() {
-			fixture.innerHTML =
-				'<div id="foo"></div> <span id="foo"></span>' +
-				'<div aria-labelledby="foo"></div>';
-			var node = fixture.querySelector('div[id="foo"]');
-			assert.isTrue(
-				checks['duplicate-id'].evaluate.call(checkContext, node, {
-					accReferred: false
-				})
-			);
-		});
-
-		it('tests unreffed elements with accReferred: false', function() {
-			fixture.innerHTML = '<div id="foo"></div> <span id="foo"></span>';
-			var node = fixture.querySelector('div[id="foo"]');
-			assert.isFalse(
-				checks['duplicate-id'].evaluate.call(checkContext, node, {
-					accReferred: false
-				})
-			);
-		});
-	});
 });

test/commons/aria/is-accessible-ref.js

@@ -4,7 +4,7 @@ describe('aria.isAccessibleRef', function() {
 	var __atrs;
 	var fixture = document.getElementById('fixture');
 	var isAccessibleRef = axe.commons.aria.isAccessibleRef;
-	var shadowSupport = axe.testUtils.shadowSupport;
+	var shadowSupport = axe.testUtils.shadowSupport.v1;
 
 	function setLookup(attrs) {
 		axe.commons.aria.lookupTable.attributes = attrs;

test/integration/rules/duplicate-id-active/duplicate-id-active.html

@@ -0,0 +1,19 @@
+<p id="ignore1">This is my first paragraph with this ID.</p>
+<div style="display:none">
+  <p id="ignore1">This is my second paragraph with this ID.</p>
+</div>
+
+<button id="fail1" class="fail1"></button>
+<button id="fail1"></button>
+
+<input id="pass1" />
+<textarea id="pass2"></textarea>
+<select id="pass3"></select>
+<div tabindex="0" id="pass4"></div>
+
+<span id="ignored1"></span>
+<button id="ignored2"></button>
+<div aria-labelledby="ignored1 ignored2"></div>
+<input id="ignore3" type="hidden" />
+<button id="ignore4" disabled></button>
+<button id="ignore5" style="display:none"></button>

test/integration/rules/duplicate-id-active/duplicate-id-active.json

@@ -0,0 +1,6 @@
+{
+	"description": "duplicate-id-active test",
+	"rule": "duplicate-id-active",
+	"violations": [[".fail1"]],
+	"passes": [["#pass1"], ["#pass2"], ["#pass3"], ["#pass4"]]
+}

test/integration/rules/duplicate-id-aria/duplicate-id-aria.html

@@ -0,0 +1,19 @@
+<p id="ignore1">This is my first paragraph with this ID.</p>
+<div style="display:none">
+  <p id="ignore1">This is my second paragraph with this ID.</p>
+</div>
+
+<input id="ignore2" />
+<textarea id="ignore3"></textarea>
+<select id="ignore4"></select>
+<div tabindex="0" id="ignore5"></div>
+
+<span id="fail1" class="fail1"></span>
+<button id="fail1"></button>
+<span id="pass1"></span>
+<button id="pass2"></button>
+<div aria-labelledby="fail1 pass1 pass2"></div>
+
+<input id="ignore6" type="hidden" />
+<button id="ignore7" disabled></button>
+<button id="ignore8" style="display:none"></button>

test/integration/rules/duplicate-id-aria/duplicate-id-aria.json

@@ -0,0 +1,6 @@
+{
+	"description": "duplicate-id-aria test",
+	"rule": "duplicate-id-aria",
+	"violations": [[".fail1"]],
+	"passes": [["#pass1"], ["#pass2"]]
+}

test/integration/rules/duplicate-id/duplicate-id.html

@@ -1,5 +1,17 @@
-<p id="dupe">This is my first paragraph with this ID.</p>
+<p id="fail1" class="fail1">This is my first paragraph with this ID.</p>
 <div style="display:none">
-  <p id="dupe">This is my second paragraph with this ID.</p>
+  <p id="fail1">This is my second paragraph with this ID.</p>
 </div>
-<p id="single">This is my only paragraph with this ID.</p>
+
+<input id="ignored1" />
+<textarea id="ignored2"></textarea>
+<select id="ignored3"></select>
+<div tabindex="0" id="ignored4"></div>
+
+<span id="ignored5"></span>
+<button id="ignored6"></button>
+<div aria-labelledby="ignored5 ignored6"></div>
+
+<input id="pass1" type="hidden" />
+<button id="pass2" disabled></button>
+<button id="pass3" style="display:none"></button>

test/integration/rules/duplicate-id/duplicate-id.json

@@ -1,6 +1,6 @@
 {
 	"description": "duplicate-id test",
 	"rule": "duplicate-id",
-	"violations": [["#fixture > p:nth-child(1)"]],
-	"passes": [["#fixture"], ["#single"]]
+	"violations": [[".fail1"]],
+	"passes": [["#fixture"], ["#pass1"], ["#pass2"], ["#pass3"]]
 }

test/rule-matches/duplicate-id-active-matches.js

@@ -0,0 +1,79 @@
+describe('duplicate-id-mismatches', function() {
+	'use strict';
+
+	var fixture = document.getElementById('fixture');
+	var fixtureSetup = axe.testUtils.fixtureSetup;
+	var rule;
+
+	beforeEach(function() {
+		rule = axe._audit.rules.find(function(rule) {
+			return rule.id === 'duplicate-id-active';
+		});
+	});
+
+	afterEach(function() {
+		fixture.innerHTML = '';
+	});
+
+	it('is a function', function() {
+		assert.isFunction(rule.matches);
+	});
+
+	it('returns false if the ID is of an inactive non-referenced element', function() {
+		fixtureSetup('<div id="foo"></div>');
+		var vNode = axe.utils.querySelectorAll(axe._tree[0], 'div[id=foo]')[0];
+		assert.isFalse(rule.matches(vNode.actualNode, vNode));
+	});
+
+	it('returns false if the ID is of an inactive non-referenced element with a duplicate', function() {
+		fixtureSetup('<div id="foo"></div><span id="foo"></span>');
+		var vNode = axe.utils.querySelectorAll(axe._tree[0], 'span[id=foo]')[0];
+		assert.isFalse(rule.matches(vNode.actualNode, vNode));
+	});
+
+	it('returns true if the ID is of an active non-referenced element', function() {
+		fixtureSetup('<button id="foo"></button>');
+		var vNode = axe.utils.querySelectorAll(axe._tree[0], 'button[id=foo]')[0];
+		assert.isTrue(rule.matches(vNode.actualNode, vNode));
+	});
+
+	it('returns true if the ID is a duplicate of an active non-referenced element', function() {
+		fixtureSetup('<div id="foo"></div>' + '<button id="foo"></button>');
+		var vNode = axe.utils.querySelectorAll(axe._tree[0], 'div[id=foo]')[0];
+		assert.isTrue(rule.matches(vNode.actualNode, vNode));
+	});
+
+	it('returns false if the ID is of an inactive ARIA referenced element', function() {
+		fixtureSetup('<div id="foo"></div>' + '<div aria-labelledby="foo"></div>');
+		var vNode = axe.utils.querySelectorAll(axe._tree[0], 'div[id=foo]')[0];
+		assert.isFalse(rule.matches(vNode.actualNode, vNode));
+	});
+
+	it('returns false if the ID is a duplicate of an inactive ARIA referenced element', function() {
+		fixtureSetup(
+			'<div id="foo"></div>' +
+				'<div aria-labelledby="foo"></div>' +
+				'<span id="foo"></span>'
+		);
+		var vNode = axe.utils.querySelectorAll(axe._tree[0], 'span[id=foo]')[0];
+		assert.isFalse(rule.matches(vNode.actualNode, vNode));
+	});
+
+	it('returns false if the ID is of an active ARIA referenced element', function() {
+		fixtureSetup(
+			'<button id="foo"></button>' + '<div aria-labelledby="foo"></div>'
+		);
+		var vNode = axe.utils.querySelectorAll(axe._tree[0], 'button[id=foo]')[0];
+		assert.isFalse(rule.matches(vNode.actualNode, vNode));
+	});
+
+	it('returns false if the ID is a duplicate of of an active ARIA referenced element', function() {
+		fixtureSetup(
+			'<button id="foo"></button>' +
+				'<div aria-labelledby="foo"></div>' +
+				'<span id="foo"></span>'
+		);
+		var vNode = axe.utils.querySelectorAll(axe._tree[0], 'span[id=foo]')[0];
+		assert.isFalse(rule.matches(vNode.actualNode, vNode));
+	});
+});