Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 5 commits into from
Aug 24, 2018
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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}}"
}
}
}
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 or with the same id attribute: {{=it.data}}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change to Document has multiple elements referenced with ARIA with the same id attribute: {{=it.data}}

}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"evaluate": "duplicate-id.js",
"after": "duplicate-id-after.js",
"metadata": {
"impact": "moderate",
"impact": "minor",
"messages": {
"pass": "Document has no elements that share the same id attribute",
"fail": "Document has multiple elements with the same id attribute: {{=it.data}}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change this to Document has multiple static elements with the same id attribute?

Expand Down
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
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,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>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we have more examples of failures? For example a <div tabindex="-1" id="fail2">blah</div><input id="fail2" /> etc.?


<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"]],
"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