fix(core/utils/querySelectorAll): Ensure that elements do not get tes…

…ted twice (#666) * fix(core/utils/querySelectorAll): Ensure that results do not contain duplicate nodes * chore(qsa): Improve code readability
dequelabs · Jan 6, 2018 · a76a454 · a76a454
1 parent c665d0b
commit a76a454
Show file tree

Hide file tree

Showing 6 changed files with 91 additions and 8 deletions.
diff --git a/lib/core/utils/qsa.js b/lib/core/utils/qsa.js
@@ -44,10 +44,10 @@ function matchSelector (targets, exp, recurse) {
 	targets = Array.isArray(targets) ? targets : [targets];
 	targets.forEach((target) => {
 		if (matchesTag(target.actualNode, exp) &&
-			matchesClasses(target.actualNode, exp) &&
-			matchesAttributes(target.actualNode, exp) &&
-			matchesId(target.actualNode, exp) &&
-			matchesPseudos(target, exp)) {
+				matchesClasses(target.actualNode, exp) &&
+				matchesAttributes(target.actualNode, exp) &&
+				matchesId(target.actualNode, exp) &&
+				matchesPseudos(target, exp)) {
 			result.push(target);
 		}
 		if (recurse) {
@@ -101,7 +101,7 @@ function convertAttributes (atts) {
 				break;
 			case '*=' :
 				test = function(value){
-					return value && value.indexOf(attributeValue) > -1;
+					return value && value.includes(attributeValue);
 				};
 				break;
 			case '!=' :
@@ -199,14 +199,21 @@ matchExpressions = function  (domTree, expressions, recurse) {
 		var candidates = domTree;
 		exprArr.forEach((exp, index) => {
 			recurse = exp.combinator === '>' ? false : recurse;
-			if ([' ', '>'].indexOf(exp.combinator) === -1) {
+			if ([' ', '>'].includes(exp.combinator) === false) {
 				throw new Error('axe.utils.querySelectorAll does not support the combinator: ' + exp.combinator);
 			}
 			candidates = candidates.reduce((result, node) => {
 				return result.concat(matchSelector(index ? node.children : node, exp, recurse));
 			}, []);
 		});
-		return collected.concat(candidates);
+
+		// Ensure elements aren't added multiple times
+		return candidates.reduce((collected, candidate) => {
+			if (collected.includes(candidate) === false) {
+				collected.push(candidate);
+			}
+			return collected;
+		}, collected);
 	}, []);
 };
 

diff --git a/test/core/utils/qsa.js b/test/core/utils/qsa.js
@@ -174,4 +174,12 @@ describe('axe.utils.querySelectorAll', function () {
 			'.first[data-a11yhero="faulkner"] > ul li.breaking');
 		assert.equal(result.length, 2);
 	});
+	it('should find an element only once', function () {
+		var divs = axe.utils.querySelectorAll(dom, 'div');
+		var ones = axe.utils.querySelectorAll(dom, '#one');
+		var divOnes = axe.utils.querySelectorAll(dom, 'div, #one');
+
+		assert.isBelow(divOnes.length, divs.length + ones.length,
+			'Elements matching both parts of a selector should not be included twice');
+	});
 });
diff --git a/...mark-one-main/landmark-one-main-pass.html → ...ark-one-main/landmark-one-main-pass1.html b/...mark-one-main/landmark-one-main-pass.html → ...ark-one-main/landmark-one-main-pass1.html
@@ -18,7 +18,7 @@
 	<p>No main content</p>
 	<iframe id="frame1" src="frames/level1.html"></iframe>
 	<div id="mocha"></div>
-	<script src="landmark-one-main-pass.js"></script>
+	<script src="landmark-one-main-pass1.js"></script>
 	<script src="/test/integration/adapter.js"></script>
 </body>
 </html>
diff --git a/...ndmark-one-main/landmark-one-main-pass.js → ...dmark-one-main/landmark-one-main-pass1.js b/...ndmark-one-main/landmark-one-main-pass.js → ...dmark-one-main/landmark-one-main-pass1.js
diff --git a/test/integration/full/landmark-one-main/landmark-one-main-pass2.html b/test/integration/full/landmark-one-main/landmark-one-main-pass2.html
@@ -0,0 +1,25 @@
+<!doctype html>
+<html lang="en" id="pass1">
+<head>
+	<meta charset="utf8">
+	<link rel="stylesheet" type="text/css" href="/node_modules/mocha/mocha.css" />
+	<script src="/node_modules/mocha/mocha.js"></script>
+	<script src="/node_modules/chai/chai.js"></script>
+	<script src="/axe.js"></script>
+	<script>
+		mocha.setup({
+			timeout: 10000,
+			ui: 'bdd'
+		});
+		var assert = chai.assert;
+	</script>
+</head>
+<body>
+	<main role="main">
+		<p>Main content</p>
+	</main>
+	<div id="mocha"></div>
+	<script src="landmark-one-main-pass2.js"></script>
+	<script src="/test/integration/adapter.js"></script>
+</body>
+</html>
diff --git a/test/integration/full/landmark-one-main/landmark-one-main-pass2.js b/test/integration/full/landmark-one-main/landmark-one-main-pass2.js
@@ -0,0 +1,43 @@
+describe('landmark-one-main test pass', function () {
+	'use strict';
+	var results;
+	before(function (done) {
+		function start() {
+			axe.run({ runOnly: { type: 'rule', values: ['landmark-one-main'] } }, function (err, r) {
+				assert.isNull(err);
+				results = r;
+				done();
+			});
+		}
+		if (document.readyState !== 'complete') {
+			window.addEventListener('load', start);
+		} else {
+			start();
+		}
+	});
+
+	describe('violations', function () {
+		it('should find 0', function () {
+			assert.lengthOf(results.violations, 0);
+		});
+	});
+
+	describe('passes', function () {
+		it('should find 1', function () {
+			assert.lengthOf(results.passes[0].nodes, 1);
+		});
+
+		it('should find #pass1', function () {
+			assert.deepEqual(results.passes[0].nodes[0].target, ['#pass1']);
+		});
+	});
+
+	it('should find 0 inapplicable', function () {
+		assert.lengthOf(results.inapplicable, 0);
+	});
+
+	it('should find 0 incomplete', function () {
+		assert.lengthOf(results.incomplete, 0);
+	});
+
+});