Skip to content

Commit d183a34

Browse files
rajathmr2000claude
andcommitted
AXE-3625: Reuse shared duplicate-id check for the aria variant (DRY)
Collapse the dedicated duplicate-id-aria evaluate/after back into the shared duplicate-id-evaluate/duplicate-id-after, gated by a `reviewPayload` check option. The aria check enables the bulk-review payload via that option; the static/active checks keep their original string `data` and are behaviourally unchanged. Adds tests for both data shapes and verdict parity. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 9db3fa6 commit d183a34

7 files changed

Lines changed: 69 additions & 80 deletions

File tree

lib/checks/parsing/duplicate-id-after.js

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
function duplicateIdAfter(results) {
22
const uniqueIds = [];
33
return results.filter(r => {
4-
if (uniqueIds.indexOf(r.data) === -1) {
5-
uniqueIds.push(r.data);
4+
const id = r.data && typeof r.data === 'object' ? r.data.id : r.data;
5+
if (uniqueIds.indexOf(id) === -1) {
6+
uniqueIds.push(id);
67
return true;
78
}
89
return false;

lib/checks/parsing/duplicate-id-aria-after.js

Lines changed: 0 additions & 18 deletions
This file was deleted.

lib/checks/parsing/duplicate-id-aria-evaluate.js

Lines changed: 0 additions & 55 deletions
This file was deleted.

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
{
22
"id": "duplicate-id-aria",
3-
"evaluate": "duplicate-id-aria-evaluate",
4-
"after": "duplicate-id-aria-after",
3+
"evaluate": "duplicate-id-evaluate",
4+
"after": "duplicate-id-after",
5+
"options": { "reviewPayload": true },
56
"metadata": {
67
"impact": "critical",
78
"messages": {

lib/checks/parsing/duplicate-id-evaluate.js

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { getRootNode } from '../../commons/dom';
22
import { escapeSelector } from '../../core/utils';
33

4-
function duplicateIdEvaluate(node) {
4+
function duplicateIdEvaluate(node, options = {}) {
55
const id = node.getAttribute('id').trim();
66
// Since empty ID's are not meaningful and are ignored by Edge, we'll
77
// let those pass.
@@ -16,6 +16,28 @@ function duplicateIdEvaluate(node) {
1616
if (matchingNodes.length) {
1717
this.relatedNodes(matchingNodes);
1818
}
19+
20+
if (options.reviewPayload) {
21+
const verdict = matchingNodes.length === 0;
22+
const data = { id };
23+
try {
24+
if (matchingNodes.length) {
25+
data.reviewPayload = {
26+
visualHelperData: {
27+
duplicateId: id,
28+
elements: matchingNodes
29+
.filter(Boolean)
30+
.map(foundNode => new axe.utils.DqElement(foundNode).selector)
31+
}
32+
};
33+
}
34+
} catch {
35+
// best-effort emit — preserve the scan result
36+
}
37+
this.data(data);
38+
return verdict;
39+
}
40+
1941
this.data(id);
2042

2143
return matchingNodes.length === 0;

test/checks/parser/duplicate-id-aria.js

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ describe('duplicate-id-aria', function () {
55
var fixtureSetup = axe.testUtils.fixtureSetup;
66
var checkContext = axe.testUtils.MockCheckContext();
77
var checkEvaluate = axe.testUtils.getCheckEvaluate('duplicate-id-aria');
8+
// duplicate-id-aria enables the bulk-review payload via this check option
9+
var options = { reviewPayload: true };
810

911
afterEach(function () {
1012
fixture.innerHTML = '';
@@ -15,7 +17,7 @@ describe('duplicate-id-aria', function () {
1517
it('returns true and emits no reviewPayload when the id is unique', function () {
1618
fixtureSetup('<div id="solo"></div>');
1719
var node = fixture.querySelector('#solo');
18-
assert.isTrue(checkEvaluate.call(checkContext, node));
20+
assert.isTrue(checkEvaluate.call(checkContext, node, options));
1921
assert.equal(checkContext._data.id, 'solo');
2022
assert.notProperty(checkContext._data, 'reviewPayload');
2123
assert.deepEqual(checkContext._relatedNodes, []);
@@ -26,7 +28,7 @@ describe('duplicate-id-aria', function () {
2628
'<div id="dup"></div><div id="dup"></div><div id="dup"></div>'
2729
);
2830
var node = fixture.querySelector('div');
29-
assert.isFalse(checkEvaluate.call(checkContext, node));
31+
assert.isFalse(checkEvaluate.call(checkContext, node, options));
3032

3133
assert.equal(checkContext._data.id, 'dup');
3234
var visualHelperData = checkContext._data.reviewPayload.visualHelperData;
@@ -38,6 +40,16 @@ describe('duplicate-id-aria', function () {
3840
assert.isString(visualHelperData.elements[0][0]);
3941
});
4042

43+
it('emits additively — the reviewPayload option does not change the verdict', function () {
44+
fixtureSetup('<div id="dup"></div><div id="dup"></div>');
45+
var node = fixture.querySelector('div');
46+
var withOption = checkEvaluate.call(checkContext, node, options);
47+
checkContext.reset();
48+
var withoutOption = checkEvaluate.call(checkContext, node, {});
49+
assert.equal(withOption, withoutOption);
50+
assert.isFalse(withOption);
51+
});
52+
4153
it('after dedupes results by data.id', function () {
4254
var results = checks['duplicate-id-aria'].after([
4355
{ data: { id: 'a' } },

test/checks/parser/duplicate-id.js

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,19 @@ describe('duplicate-id', function () {
3131
assert.deepEqual(checkContext._relatedNodes, [node.nextSibling]);
3232
});
3333

34+
it('keeps data a plain id string and emits no reviewPayload when the reviewPayload option is off', function () {
35+
// Guards the static/active duplicate-id checks: they share this evaluate
36+
// but never pass the reviewPayload option, so they must keep string data.
37+
fixture.innerHTML = '<div id="target"></div><div id="target"></div>';
38+
var node = fixture.querySelector('#target');
39+
assert.isFalse(
40+
axe.testUtils.getCheckEvaluate('duplicate-id').call(checkContext, node)
41+
);
42+
assert.isString(checkContext._data);
43+
assert.equal(checkContext._data, 'target');
44+
assert.isUndefined(checkContext._data.reviewPayload);
45+
});
46+
3447
it('should return remove duplicates', function () {
3548
assert.deepEqual(
3649
checks['duplicate-id'].after([
@@ -42,6 +55,19 @@ describe('duplicate-id', function () {
4255
);
4356
});
4457

58+
it('removes duplicates for object-shaped data (shared after with the aria variant)', function () {
59+
// The shared after dedupes by data.id when data is an object
60+
// ({ id, reviewPayload }), as emitted by the aria (bulk-review) variant.
61+
assert.deepEqual(
62+
checks['duplicate-id'].after([
63+
{ data: { id: 'a' } },
64+
{ data: { id: 'b' } },
65+
{ data: { id: 'b' } }
66+
]),
67+
[{ data: { id: 'a' } }, { data: { id: 'b' } }]
68+
);
69+
});
70+
4571
it('should ignore empty ids', function () {
4672
fixture.innerHTML =
4773
'<div data-testelm="1" id=""></div><div data-testelm="2" id=""></div>';

0 commit comments

Comments
 (0)