Skip to content

Commit

Permalink
Merge pull request #1097 from dequelabs/duplicate-id-breakup
Browse files Browse the repository at this point in the history
feat: Break up duplicate-id rule for ARIA+labels and for active elements
  • Loading branch information
dylanb authored Aug 24, 2018
2 parents b6ac084 + 5a855df commit 24dd39c
Show file tree
Hide file tree
Showing 25 changed files with 561 additions and 18 deletions.
4 changes: 3 additions & 1 deletion doc/rule-descriptions.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 | Moderate | cat.parsing, wcag2a, wcag411 | true |
| duplicate-id-active | Ensures every id attribute value of active elements 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 |
Expand Down
12 changes: 12 additions & 0 deletions lib/checks/parsing/duplicate-id-active.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"id": "duplicate-id-active",
"evaluate": "duplicate-id.js",
"after": "duplicate-id-after.js",
"metadata": {
"impact": "serious",
"messages": {
"pass": "Document has no active elements that share the same id attribute",
"fail": "Document has active elements with the same id attribute: {{=it.data}}"
}
}
}
File renamed without changes.
12 changes: 12 additions & 0 deletions lib/checks/parsing/duplicate-id-aria.json
Original file line number Diff line number Diff line change
@@ -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 referenced with ARIA or labels that share the same id attribute",
"fail": "Document has multiple elements referenced with ARIA with the same id attribute: {{=it.data}}"
}
}
}
File renamed without changes.
12 changes: 12 additions & 0 deletions lib/checks/parsing/duplicate-id.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"id": "duplicate-id",
"evaluate": "duplicate-id.js",
"after": "duplicate-id-after.js",
"metadata": {
"impact": "minor",
"messages": {
"pass": "Document has no static elements that share the same id attribute",
"fail": "Document has multiple static elements with the same id attribute"
}
}
}
12 changes: 0 additions & 12 deletions lib/checks/shared/duplicate-id.json

This file was deleted.

53 changes: 53 additions & 0 deletions lib/commons/aria/is-accessible-ref.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
/* global aria, axe, dom */
function findDomNode(node, functor) {
if (functor(node)) {
return node;
}
for (let i = 0; i < node.children.length; i++) {
const out = findDomNode(node.children[i], functor);
if (out) {
return out;
}
}
}

/**
* Check that a DOM node is a reference in the accessibility tree
* @param {Element} node
* @returns {Boolean}
*/
aria.isAccessibleRef = function isAccessibleRef(node) {
node = node.actualNode || node;
let root = dom.getRootNode(node);
root = root.documentElement || root; // account for shadow roots
const id = node.id;

// Get all idref(s) attributes on the lookup table
const refAttrs = Object.keys(aria.lookupTable.attributes).filter(attr => {
const { type } = aria.lookupTable.attributes[attr];
return /^idrefs?$/.test(type);
});

// Find the first element that IDREF(S) the node
let refElm = findDomNode(root, elm => {
if (elm.nodeType !== 1) {
// Elements only
return;
}
if (
elm.nodeName.toUpperCase() === 'LABEL' &&
elm.getAttribute('for') === id
) {
return true;
}
// See if there are any aria attributes that reference the node
return refAttrs.filter(attr => elm.hasAttribute(attr)).some(attr => {
const attrValue = elm.getAttribute(attr);
if (aria.lookupTable.attributes[attr].type === 'idref') {
return attrValue === id;
}
return axe.utils.tokenList(attrValue).includes(id);
});
});
return typeof refElm !== 'undefined';
};
8 changes: 8 additions & 0 deletions lib/rules/duplicate-id-active-matches.js
Original file line number Diff line number Diff line change
@@ -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);
20 changes: 20 additions & 0 deletions lib/rules/duplicate-id-active.json
Original file line number Diff line number Diff line change
@@ -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 of active elements is unique",
"help": "IDs of active elements must be unique"
},
"all": [],
"any": [
"duplicate-id-active"
],
"none": []
}
1 change: 1 addition & 0 deletions lib/rules/duplicate-id-aria-matches.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
return axe.commons.aria.isAccessibleRef(node);
20 changes: 20 additions & 0 deletions lib/rules/duplicate-id-aria.json
Original file line number Diff line number Diff line change
@@ -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": [],
"any": [
"duplicate-id-aria"
],
"none": []
}
11 changes: 11 additions & 0 deletions lib/rules/duplicate-id-misc-matches.js
Original file line number Diff line number Diff line change
@@ -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)
);
1 change: 1 addition & 0 deletions lib/rules/duplicate-id.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
{
"id": "duplicate-id",
"selector": "[id]",
"matches": "duplicate-id-misc-matches.js",
"excludeHidden": false,
"tags": [
"cat.parsing",
Expand Down
File renamed without changes.
96 changes: 96 additions & 0 deletions test/commons/aria/is-accessible-ref.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
describe('aria.isAccessibleRef', function() {
'use strict';

var __atrs;
var fixture = document.getElementById('fixture');
var isAccessibleRef = axe.commons.aria.isAccessibleRef;
var shadowSupport = axe.testUtils.shadowSupport.v1;

function setLookup(attrs) {
axe.commons.aria.lookupTable.attributes = attrs;
}

afterEach(function() {
fixture.innerHTML = '';
__atrs = axe.commons.aria.lookupTable.attributes;
});

afterEach(function() {
axe.commons.aria.lookupTable.attributes = __atrs;
});

it('returns false by default', function() {
fixture.innerHTML = '<div id="foo"><div>';
var node = document.getElementById('foo');
assert.isFalse(isAccessibleRef(node));
});

it('returns true for IDs used in aria IDREF attributes', function() {
setLookup({ 'aria-foo': { type: 'idref' } });
fixture.innerHTML = '<div aria-foo="foo"></div><i id="foo"></i>';
var node = document.getElementById('foo');
assert.isTrue(isAccessibleRef(node));
});

it('returns true for IDs used in aria IDREFS attributes', function() {
setLookup({ 'aria-bar': { type: 'idrefs' } });
fixture.innerHTML =
'<div aria-bar="foo bar"></div><i id="foo"></i><b id="bar"></b>';

var node1 = document.getElementById('foo');
var node2 = document.getElementById('bar');
assert.isTrue(isAccessibleRef(node1));
assert.isTrue(isAccessibleRef(node2));
});

it('returns true for IDs used in label[for] attributes', function() {
setLookup({ 'aria-foo': { type: 'idref' } });
fixture.innerHTML = '<label for="baz">baz</label><input id="baz">';
var node = document.getElementById('baz');
assert.isTrue(isAccessibleRef(node));
});

(shadowSupport ? it : xit)('works inside shadow DOM', function() {
setLookup({ 'aria-bar': { type: 'idref' } });
fixture.innerHTML = '<div id="foo"></div>';

var shadow = document.getElementById('foo').attachShadow({ mode: 'open' });
shadow.innerHTML = '<div aria-bar="bar"></div><b id="bar"></b>';

var node = shadow.getElementById('bar');
assert.isTrue(isAccessibleRef(node));
});

(shadowSupport ? it : xit)(
'returns false for IDREFs inside shadow DOM',
function() {
setLookup({ 'aria-foo': { type: 'idrefs' } });
fixture.innerHTML = '<div id="foo"><div id="bar"></div></div>';
var node1 = document.getElementById('foo');
var node2 = document.getElementById('bar');

var shadow = node1.attachShadow({ mode: 'open' });
shadow.innerHTML = '<div aria-foo="foo bar"><slot></slot></div>';

assert.isFalse(isAccessibleRef(node1));
assert.isFalse(isAccessibleRef(node2));
}
);

(shadowSupport ? it : xit)(
'returns false for IDREFs outside shadow DOM',
function() {
setLookup({ 'aria-bar': { type: 'idref' } });
fixture.innerHTML =
'<div id="foo" aria-bar="bar"><div aria-bar="bar"></div></div>';

var shadow = document
.getElementById('foo')
.attachShadow({ mode: 'open' });
shadow.innerHTML = '<div id="bar"><slot></slot></div>';

var node = shadow.getElementById('bar');
assert.isFalse(isAccessibleRef(node));
}
);
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<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="fail2" class="fail2" />
<input id="fail2" />
<textarea id="fail3" class="fail3"></textarea>
<textarea id="fail3"></textarea>
<select id="fail4" class="fail4"></select>
<select id="fail4"></select>
<div tabindex="0" id="fail5" class="fail5"></div>
<div tabindex="0" id="fail5"></div>

<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>
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"description": "duplicate-id-active test",
"rule": "duplicate-id-active",
"violations": [[".fail1"], [".fail2"], [".fail3"], [".fail4"], [".fail5"]],
"passes": [["#pass1"], ["#pass2"], ["#pass3"], ["#pass4"]]
}
19 changes: 19 additions & 0 deletions test/integration/rules/duplicate-id-aria/duplicate-id-aria.html
Original file line number Diff line number Diff line change
@@ -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>
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"description": "duplicate-id-aria test",
"rule": "duplicate-id-aria",
"violations": [[".fail1"]],
"passes": [["#pass1"], ["#pass2"]]
}
18 changes: 15 additions & 3 deletions test/integration/rules/duplicate-id/duplicate-id.html
Original file line number Diff line number Diff line change
@@ -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>
4 changes: 2 additions & 2 deletions test/integration/rules/duplicate-id/duplicate-id.json
Original file line number Diff line number Diff line change
@@ -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"]]
}
Loading

0 comments on commit 24dd39c

Please sign in to comment.