Skip to content

Commit a76a454

Browse files
WilcoFiersWilcoFiers
authored
fix(core/utils/querySelectorAll): Ensure that elements do not get tested twice (#666)
* fix(core/utils/querySelectorAll): Ensure that results do not contain duplicate nodes * chore(qsa): Improve code readability
1 parent c665d0b commit a76a454

File tree

6 files changed

+91
-8
lines changed

6 files changed

+91
-8
lines changed

‎lib/core/utils/qsa.js

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,10 @@ function matchSelector (targets, exp, recurse) {
4444
targets = Array.isArray(targets) ? targets : [targets];
4545
targets.forEach((target) => {
4646
if (matchesTag(target.actualNode, exp) &&
47-
matchesClasses(target.actualNode, exp) &&
48-
matchesAttributes(target.actualNode, exp) &&
49-
matchesId(target.actualNode, exp) &&
50-
matchesPseudos(target, exp)) {
47+
matchesClasses(target.actualNode, exp) &&
48+
matchesAttributes(target.actualNode, exp) &&
49+
matchesId(target.actualNode, exp) &&
50+
matchesPseudos(target, exp)) {
5151
result.push(target);
5252
}
5353
if (recurse) {
@@ -101,7 +101,7 @@ function convertAttributes (atts) {
101101
break;
102102
case '*=' :
103103
test = function(value){
104-
return value && value.indexOf(attributeValue) > -1;
104+
return value && value.includes(attributeValue);
105105
};
106106
break;
107107
case '!=' :
@@ -199,14 +199,21 @@ matchExpressions = function (domTree, expressions, recurse) {
199199
var candidates = domTree;
200200
exprArr.forEach((exp, index) => {
201201
recurse = exp.combinator === '>' ? false : recurse;
202-
if ([' ', '>'].indexOf(exp.combinator) === -1) {
202+
if ([' ', '>'].includes(exp.combinator) === false) {
203203
throw new Error('axe.utils.querySelectorAll does not support the combinator: ' + exp.combinator);
204204
}
205205
candidates = candidates.reduce((result, node) => {
206206
return result.concat(matchSelector(index ? node.children : node, exp, recurse));
207207
}, []);
208208
});
209-
return collected.concat(candidates);
209+
210+
// Ensure elements aren't added multiple times
211+
return candidates.reduce((collected, candidate) => {
212+
if (collected.includes(candidate) === false) {
213+
collected.push(candidate);
214+
}
215+
return collected;
216+
}, collected);
210217
}, []);
211218
};
212219

‎test/core/utils/qsa.js

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,4 +174,12 @@ describe('axe.utils.querySelectorAll', function () {
174174
'.first[data-a11yhero="faulkner"] > ul li.breaking');
175175
assert.equal(result.length, 2);
176176
});
177+
it('should find an element only once', function () {
178+
var divs = axe.utils.querySelectorAll(dom, 'div');
179+
var ones = axe.utils.querySelectorAll(dom, '#one');
180+
var divOnes = axe.utils.querySelectorAll(dom, 'div, #one');
181+
182+
assert.isBelow(divOnes.length, divs.length + ones.length,
183+
'Elements matching both parts of a selector should not be included twice');
184+
});
177185
});

‎test/integration/full/landmark-one-main/landmark-one-main-pass.html renamed to ‎test/integration/full/landmark-one-main/landmark-one-main-pass1.html

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
<p>No main content</p>
1919
<iframe id="frame1" src="frames/level1.html"></iframe>
2020
<div id="mocha"></div>
21-
<script src="landmark-one-main-pass.js"></script>
21+
<script src="landmark-one-main-pass1.js"></script>
2222
<script src="/test/integration/adapter.js"></script>
2323
</body>
2424
</html>

‎test/integration/full/landmark-one-main/landmark-one-main-pass.js renamed to ‎test/integration/full/landmark-one-main/landmark-one-main-pass1.js

File renamed without changes.

‎test/integration/full/landmark-one-main/landmark-one-main-pass2.html

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
<!doctype html>
2+
<html lang="en" id="pass1">
3+
<head>
4+
<meta charset="utf8">
5+
<link rel="stylesheet" type="text/css" href="/node_modules/mocha/mocha.css" />
6+
<script src="/node_modules/mocha/mocha.js"></script>
7+
<script src="/node_modules/chai/chai.js"></script>
8+
<script src="/axe.js"></script>
9+
<script>
10+
mocha.setup({
11+
timeout: 10000,
12+
ui: 'bdd'
13+
});
14+
var assert = chai.assert;
15+
</script>
16+
</head>
17+
<body>
18+
<main role="main">
19+
<p>Main content</p>
20+
</main>
21+
<div id="mocha"></div>
22+
<script src="landmark-one-main-pass2.js"></script>
23+
<script src="/test/integration/adapter.js"></script>
24+
</body>
25+
</html>

‎test/integration/full/landmark-one-main/landmark-one-main-pass2.js

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
describe('landmark-one-main test pass', function () {
2+
'use strict';
3+
var results;
4+
before(function (done) {
5+
function start() {
6+
axe.run({ runOnly: { type: 'rule', values: ['landmark-one-main'] } }, function (err, r) {
7+
assert.isNull(err);
8+
results = r;
9+
done();
10+
});
11+
}
12+
if (document.readyState !== 'complete') {
13+
window.addEventListener('load', start);
14+
} else {
15+
start();
16+
}
17+
});
18+
19+
describe('violations', function () {
20+
it('should find 0', function () {
21+
assert.lengthOf(results.violations, 0);
22+
});
23+
});
24+
25+
describe('passes', function () {
26+
it('should find 1', function () {
27+
assert.lengthOf(results.passes[0].nodes, 1);
28+
});
29+
30+
it('should find #pass1', function () {
31+
assert.deepEqual(results.passes[0].nodes[0].target, ['#pass1']);
32+
});
33+
});
34+
35+
it('should find 0 inapplicable', function () {
36+
assert.lengthOf(results.inapplicable, 0);
37+
});
38+
39+
it('should find 0 incomplete', function () {
40+
assert.lengthOf(results.incomplete, 0);
41+
});
42+
43+
});

0 commit comments

Comments
 (0)