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

fix(skip-link,region): Allow multiple skiplinks at page top #1496

Merged
merged 6 commits into from
Apr 26, 2019
Merged
Show file tree
Hide file tree
Changes from all 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
21 changes: 2 additions & 19 deletions lib/checks/navigation/region.js
Original file line number Diff line number Diff line change
@@ -1,29 +1,11 @@
const { dom, aria } = axe.commons;

// Return the skplink, if any
function getSkiplink(virtualNode) {
const firstLink = axe.utils.querySelectorAll(virtualNode, 'a[href]')[0];
if (
firstLink &&
axe.commons.dom.getElementByReference(firstLink.actualNode, 'href')
) {
return firstLink.actualNode;
}
}

const skipLink = getSkiplink(virtualNode);
const landmarkRoles = aria.getRolesByType('landmark');

// Create a list of nodeNames that have a landmark as an implicit role
const implicitLandmarks = landmarkRoles
.reduce((arr, role) => arr.concat(aria.implicitNodes(role)), [])
.filter(r => r !== null);

// Check if the current element is the skiplink
function isSkipLink(vNode) {
return skipLink && skipLink === vNode.actualNode;
}

// Check if the current element is a landmark
function isRegion(virtualNode) {
const node = virtualNode.actualNode;
Expand Down Expand Up @@ -61,7 +43,8 @@ function findRegionlessElms(virtualNode) {
// End recursion if the element is a landmark, skiplink, or hidden content
if (
isRegion(virtualNode) ||
isSkipLink(virtualNode) ||
(dom.isSkipLink(virtualNode.actualNode) &&
dom.getElementByReference(virtualNode.actualNode, 'href')) ||
!dom.isVisible(node, true)
) {
return [];
Expand Down
37 changes: 37 additions & 0 deletions lib/commons/dom/is-skip-link.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/* global dom */

// test for hrefs that start with # or /# (for angular)
const isInternalLinkRegex = /^\/?#[^/!]/;

/**
* Determines if element is a skip link
* @method isSkipLink
* @memberof axe.commons.dom
* @instance
* @param {Element} element
* @return {Boolean}
*/
dom.isSkipLink = function(element) {
straker marked this conversation as resolved.
Show resolved Hide resolved
if (!isInternalLinkRegex.test(element.getAttribute('href'))) {
return false;
}

// define a skip link as any anchor element whose href starts with `#...`
// and which precedes the first anchor element whose href doesn't start
// with `#...` (that is, a link to a page)
const firstPageLink = axe.utils.querySelectorAll(
axe._tree,
'a:not([href^="#"]):not([href^="/#"]):not([href^="javascript"])'
)[0];

// if there are no page links then all all links will need to be
// considered as skip links
if (!firstPageLink) {
return true;
}

return (
element.compareDocumentPosition(firstPageLink.actualNode) ===
element.DOCUMENT_POSITION_FOLLOWING
);
};
2 changes: 1 addition & 1 deletion lib/rules/skip-link-matches.js
Original file line number Diff line number Diff line change
@@ -1 +1 @@
return /^#[^/!]/.test(node.getAttribute('href'));
return axe.commons.dom.isSkipLink(node);
2 changes: 1 addition & 1 deletion lib/rules/skip-link.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"id": "skip-link",
"selector": "a[href]",
"selector": "a[href^=\"#\"], a[href^=\"/#\"]",
"matches": "skip-link-matches.js",
"tags": ["cat.keyboard", "best-practice"],
"metadata": {
Expand Down
37 changes: 9 additions & 28 deletions test/checks/navigation/skip-link.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,33 +7,28 @@ describe('skip-link', function() {
fixture.innerHTML = '';
});

it('should return false if the href points to another document', function() {
fixture.innerHTML =
'<a href="something.html#mainheader">Click Here</a><h1 id="mainheader">Introduction</h1>';
var node = fixture.querySelector('a');
assert.isFalse(checks['skip-link'].evaluate(node));
});

it('should return false if the href points to a non-existent element', function() {
fixture.innerHTML =
'<a href="#spacecamp">Click Here</a><h1 id="mainheader">Introduction</h1>';
var node = fixture.querySelector('a');
assert.isFalse(checks['skip-link'].evaluate(node));
});

it('should return true if the href points to an element with an ID', function() {
fixture.innerHTML =
'<a href="#target">Click Here</a><h1 id="target">Introduction</h1>';
axe._tree = axe.utils.getFlattenedTree(fixture);
var node = fixture.querySelector('a');
assert.isTrue(checks['skip-link'].evaluate(node));
});

it('should return true if the href points to an element with an name', function() {
fixture.innerHTML = '<a href="#target">Click Here</a><a name="target"></a>';
axe._tree = axe.utils.getFlattenedTree(fixture);
var node = fixture.querySelector('a');
assert.isTrue(checks['skip-link'].evaluate(node));
});

it('should return false if the href points to a non-existent element', function() {
fixture.innerHTML =
'<a href="#spacecamp">Click Here</a><h1 id="mainheader">Introduction</h1>';
var node = fixture.querySelector('a');
assert.isFalse(checks['skip-link'].evaluate(node));
});

it('should return undefined if the target has display:none', function() {
fixture.innerHTML =
'<a href="#target">Click Here</a>' +
Expand All @@ -49,18 +44,4 @@ describe('skip-link', function() {
var node = fixture.querySelector('a');
assert.isUndefined(checks['skip-link'].evaluate(node));
});

it('should return true if the URI encoded href points to an element with an ID', function() {
fixture.innerHTML =
'<a href="#%3Ctarget%3E">Click Here</a><h1 id="&lt;target&gt;">Introduction</h1>';
var node = fixture.querySelector('a');
assert.isTrue(checks['skip-link'].evaluate(node));
});

it('should return true if the URI is an Angular skiplink', function() {
fixture.innerHTML =
'<a href="/#target">Click Here</a><h1 id="target">Introduction</h1>';
var node = fixture.querySelector('a');
assert.isTrue(checks['skip-link'].evaluate(node));
});
});
71 changes: 71 additions & 0 deletions test/commons/dom/is-skip-link.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
describe('dom.isSkipLink', function() {
'use strict';

var fixture = document.getElementById('fixture');

afterEach(function() {
fixture.innerHTML = '';
});

it('should return true if the href points to an ID', function() {
fixture.innerHTML = '<a href="#target">Click Here</a>';
axe._tree = axe.utils.getFlattenedTree(fixture);
var node = fixture.querySelector('a');
assert.isTrue(axe.commons.dom.isSkipLink(node));
});

it('should return false if the href points to another document', function() {
fixture.innerHTML = '<a href="something.html#target">Click Here</a>';
axe._tree = axe.utils.getFlattenedTree(fixture);
var node = fixture.querySelector('a');
assert.isFalse(axe.commons.dom.isSkipLink(node));
});

it('should return true if the URI encoded href points to an element with an ID', function() {
fixture.innerHTML = '<a href="#%3Ctarget%3E">Click Here</a>';
axe._tree = axe.utils.getFlattenedTree(fixture);
var node = fixture.querySelector('a');
assert.isTrue(axe.commons.dom.isSkipLink(node));
});

it('should return true if the URI is an Angular skiplink', function() {
fixture.innerHTML = '<a href="/#target">Click Here</a>';
axe._tree = axe.utils.getFlattenedTree(fixture);
var node = fixture.querySelector('a');
assert.isTrue(axe.commons.dom.isSkipLink(node));
});

it('should return true for multiple skip-links', function() {
fixture.innerHTML =
'<a id="skip-link1" href="#target1">Click Here></a><a id="skip-link2" href="/#target2">Click Here></a><a id="skip-link3" href="#target3">Click Here></a>';
axe._tree = axe.utils.getFlattenedTree(fixture);
var nodes = fixture.querySelectorAll('a');
for (var i = 0; i < nodes.length; i++) {
assert.isTrue(axe.commons.dom.isSkipLink(nodes[i]));
}
});

it('should return true if the element is before a page link', function() {
fixture.innerHTML =
'<a id="skip-link" href="#target">Click Here></a><a href="/page">New Page</a>';
axe._tree = axe.utils.getFlattenedTree(fixture);
var node = fixture.querySelector('#skip-link');
assert.isTrue(axe.commons.dom.isSkipLink(node));
});

it('should return false if the element is after a page link', function() {
fixture.innerHTML =
'<a href="/page">New Page</a><a id="skip-link" href="#target">Click Here></a>';
axe._tree = axe.utils.getFlattenedTree(fixture);
var node = fixture.querySelector('#skip-link');
assert.isFalse(axe.commons.dom.isSkipLink(node));
});

it('should ignore links that start with `href=javascript`', function() {
fixture.innerHTML =
'<a href="javascript:void">New Page</a><a id="skip-link" href="#target">Click Here></a>';
axe._tree = axe.utils.getFlattenedTree(fixture);
var node = fixture.querySelector('#skip-link');
assert.isTrue(axe.commons.dom.isSkipLink(node));
});
});
25 changes: 25 additions & 0 deletions test/integration/full/skip-link/skip-link-fail.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
<!DOCTYPE html>
<html lang="en" id="pass1">
<head>
<title>skip-link test</title>
<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 src="/test/testutils.js"></script>
<script>
mocha.setup({
timeout: 10000,
ui: 'bdd'
});
var assert = chai.assert;
</script>
</head>
<body>
<a href="#fail1-tgt" id="fail1">bad link 1</a>
<div id="mocha"></div>
<script src="skip-link-fail.js"></script>
<script src="/test/integration/adapter.js"></script>
</body>
</html>
41 changes: 41 additions & 0 deletions test/integration/full/skip-link/skip-link-fail.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
describe('skip-link test pass', function() {
'use strict';
var results;

before(function(done) {
axe.testUtils.awaitNestedLoad(function() {
axe.run({ runOnly: { type: 'rule', values: ['skip-link'] } }, function(
err,
r
) {
assert.isNull(err);
results = r;
done();
});
});
});

describe('violations', function() {
it('should find 1', function() {
assert.lengthOf(results.violations, 1);
});

it('should find 1 nodes', function() {
assert.lengthOf(results.violations[0].nodes, 1);
});
});

describe('passes', function() {
it('should find 0', function() {
assert.lengthOf(results.passes, 0);
});
});

it('should find 0 inapplicable', function() {
assert.lengthOf(results.inapplicable, 0);
});

it('should find 0 incomplete', function() {
assert.lengthOf(results.incomplete, 0);
});
});
42 changes: 42 additions & 0 deletions test/integration/full/skip-link/skip-link-pass.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
<!DOCTYPE html>
<html lang="en" id="pass1">
<head>
<title>skip-link test</title>
<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 src="/test/testutils.js"></script>
<script>
mocha.setup({
timeout: 10000,
ui: 'bdd'
});
var assert = chai.assert;
</script>
</head>
<body>

<div id="pass1-tgt"></div>
<a href="#pass1-tgt" id="pass1">Link</a>

<a href="#pass2-tgt" id="pass2">Link</a>

<div id="pass3-tgt"></div>
<a href="/#pass3-tgt" id="pass3">Link (angular)</a>

<div id="canttell1-tgt" style="display:none"></div>
<a href="#canttell1-tgt" id="canttell1">Link</a>

<!-- since these elements are page links, they needs to be at the bottom
of the test so all the prior links are considered skip links -->
<a name="pass2-tgt"></a>

<a href="foo#bar" id="ignore1">link</a>
<a href="#" id="ignore2">link</a>
<div id="mocha"></div>
<script src="skip-link-pass.js"></script>
<script src="/test/integration/adapter.js"></script>
</body>
</html>
37 changes: 37 additions & 0 deletions test/integration/full/skip-link/skip-link-pass.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
describe('skip-link test pass', function() {
'use strict';
var results;

before(function(done) {
axe.testUtils.awaitNestedLoad(function() {
axe.run({ runOnly: { type: 'rule', values: ['skip-link'] } }, function(
err,
r
) {
assert.isNull(err);
results = r;
done();
});
});
});

describe('violations', function() {
it('should find 0', function() {
assert.lengthOf(results.violations, 0);
});
});

describe('passes', function() {
it('should find 3', function() {
assert.lengthOf(results.passes[0].nodes, 3);
});
});

it('should find 0 inapplicable', function() {
assert.lengthOf(results.inapplicable, 0);
});

it('should find 1 incomplete', function() {
assert.lengthOf(results.incomplete, 1);
});
});
13 changes: 0 additions & 13 deletions test/integration/rules/skip-link/skip-link.html

This file was deleted.

6 changes: 0 additions & 6 deletions test/integration/rules/skip-link/skip-link.json

This file was deleted.

Loading