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.

Sign up for GitHub

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): identify as skip-link only if the link is offscreen #2079

Merged
merged 3 commits into from
Mar 6, 2020
Merged

fix(skip-link): identify as skip-link only if the link is offscreen #2079

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
134dadd
fix(skip-link): identify as skip-link only if the link is offscreen
straker Mar 2, 2020
7008647
fix region test
straker Mar 2, 2020
ee9101b
revert changes, only check offscreen in skip-link
straker Mar 5, 2020
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
12 changes: 6 additions & 6 deletions lib/commons/dom/is-skip-link.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@ dom.isSkipLink = function(element) {
if (typeof axe._cache.get('firstPageLink') !== 'undefined') {
firstPageLink = axe._cache.get('firstPageLink');
} else {
// 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)
// define a skip link as any offscreen 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)
firstPageLink = axe.utils.querySelectorAll(
axe._tree,
'a:not([href^="#"]):not([href^="/#"]):not([href^="javascript"])'
Expand All @@ -32,14 +32,14 @@ dom.isSkipLink = function(element) {
axe._cache.set('firstPageLink', firstPageLink || null);
}

// if there are no page links then all all links will need to be
// if there are no page links then all offscreen links will need to be
// considered as skip links
if (!firstPageLink) {
return true;
return dom.isOffscreen(element);
straker marked this conversation as resolved.
Show resolved Hide resolved
}

return (
element.compareDocumentPosition(firstPageLink.actualNode) ===
element.DOCUMENT_POSITION_FOLLOWING
element.DOCUMENT_POSITION_FOLLOWING && dom.isOffscreen(element)
);
};
4 changes: 2 additions & 2 deletions test/checks/navigation/region.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,15 +55,15 @@ describe('region', function() {

it('should return true when there is a skiplink', function() {
var checkArgs = checkSetup(
'<a id="target" href="#mainheader">Click Here</a><div role="main"><h1 id="mainheader" tabindex="0">Introduction</h1></div>'
'<a id="target" href="#mainheader" style="position: absolute; top: -1000px">Click Here</a><div role="main"><h1 id="mainheader" tabindex="0">Introduction</h1></div>'
);

assert.isTrue(checks.region.evaluate.apply(checkContext, checkArgs));
});

it('should return true when there is an Angular skiplink', function() {
var checkArgs = checkSetup(
'<a id="target" href="/#mainheader">Click Here</a><div role="main"><h1 id="mainheader" tabindex="0">Introduction</h1></div>'
'<a id="target" href="/#mainheader" style="position: absolute; top: -1000px">Click Here</a><div role="main"><h1 id="mainheader" tabindex="0">Introduction</h1></div>'
);

assert.isTrue(checks.region.evaluate.apply(checkContext, checkArgs));
Expand Down
27 changes: 19 additions & 8 deletions test/commons/dom/is-skip-link.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,37 +7,48 @@ describe('dom.isSkipLink', function() {
fixture.innerHTML = '';
});

it('should return true if the href points to an ID', function() {
it('should return false if link is not offscreen', function() {
fixture.innerHTML = '<a href="#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 href points to an ID', function() {
fixture.innerHTML =
'<a href="#target" style="position: absolute; top: -10000px">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>';
fixture.innerHTML =
'<a href="something.html#target" style="position: absolute; top: -10000px">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>';
fixture.innerHTML =
'<a href="#%3Ctarget%3E" style="position: absolute; top: -10000px">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>';
fixture.innerHTML =
'<a href="/#target" style="position: absolute; top: -10000px">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>';
'<a id="skip-link1" href="#target1" style="position: absolute; top: -10000px">Click Here></a><a id="skip-link2" href="/#target2" style="position: absolute; top: -10000px">Click Here></a><a id="skip-link3" href="#target3" style="position: absolute; top: -10000px">Click Here></a>';
axe._tree = axe.utils.getFlattenedTree(fixture);
var nodes = fixture.querySelectorAll('a');
for (var i = 0; i < nodes.length; i++) {
Expand All @@ -47,23 +58,23 @@ describe('dom.isSkipLink', function() {

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>';
'<a id="skip-link" href="#target" style="position: absolute; top: -10000px">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>';
'<a href="/page">New Page</a><a id="skip-link" href="#target" style="position: absolute; top: -10000px">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>';
'<a href="javascript:void" style="position: absolute; top: -10000px">New Page</a><a id="skip-link" href="#target" style="position: absolute; top: -10000px">Click Here></a>';
axe._tree = axe.utils.getFlattenedTree(fixture);
var node = fixture.querySelector('#skip-link');
assert.isTrue(axe.commons.dom.isSkipLink(node));
Expand Down
3 changes: 2 additions & 1 deletion test/integration/full/region/region-fail.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,10 @@
</head>
<body>
<div id="wrapper">
<a href="stuff.html#mainheader" id="notskiplink"
<a href="stuff.html#mainheader" id="notskiplink1"
>This is not a skip link.</a
>
<a href="#mainheader" id="notskiplink2">This is not a skip link.</a>
<form>
<div>
<h1 id="mainheader" tabindex="0">This is a header.</h1>
Expand Down
7 changes: 6 additions & 1 deletion test/integration/full/region/region-pass.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,12 @@
</script>
</head>
<body>
<a href="#mainheader" id="skiplink">This is a skip link.</a>
<a
href="#mainheader"
id="skiplink"
style="position: absolute; left: -10000px"
>This is a skip link.</a
>
<div id="wrapper">
<div role="main">
<h1 id="mainheader" tabindex="0">This is a header.</h1>
Expand Down
4 changes: 3 additions & 1 deletion test/integration/full/skip-link/skip-link-fail.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@
</script>
</head>
<body>
<a href="#fail1-tgt" id="fail1">bad link 1</a>
<a href="#fail1-tgt" style="position: absolute; margin: -10000px" id="fail1"
>bad link 1</a
>
<div id="mocha"></div>
<script src="/test/testutils.js"></script>
<script src="skip-link-fail.js"></script>
Expand Down
38 changes: 31 additions & 7 deletions test/integration/full/skip-link/skip-link-pass.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,21 @@
<script src="/node_modules/mocha/mocha.js"></script>
<script src="/node_modules/chai/chai.js"></script>
<script src="/axe.js"></script>
<style type="text/css">
.sr-only {
border: 0;
clip: rect(0 0 0 0);
clip-path: polygon(0px 0px, 0px 0px, 0px 0px);
-webkit-clip-path: polygon(0px 0px, 0px 0px, 0px 0px);
height: 1px;
margin: -1px;
overflow: hidden;
padding: 0;
position: absolute;
width: 1px;
white-space: nowrap;
}
</style>
<script>
mocha.setup({
timeout: 10000,
Expand All @@ -21,22 +36,31 @@
</head>
<body>
<div id="pass1-tgt"></div>
<a href="#pass1-tgt" id="pass1">Link</a>
<a href="#pass1-tgt" class="sr-only" id="pass1">Link</a>

<a href="#pass2-tgt" id="pass2">Link</a>
<a href="#pass2-tgt" style="position: absolute; left: -10000px" id="pass2"
>Link</a
>

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

<div id="canttell1-tgt" style="display:none"></div>
<a href="#canttell1-tgt" id="canttell1">Link</a>
<a href="#canttell1-tgt" class="sr-only" 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>
<header>
<ul>
<li><a id="ignore1" href="/#about">About</a></li>
<li><a id="ignore2" href="/#contact">Contact</a></li>
</ul>
</header>

<a href="foo#bar" id="ignore1">link</a>
<a href="#" id="ignore2">link</a>
<h2 name="pass2-tgt">Heading</h2>

<a href="foo#bar" id="ignore3">link</a>
<a href="#" id="ignore4">link</a>
<div id="mocha"></div>
<script src="/test/testutils.js"></script>
<script src="skip-link-pass.js"></script>
Expand Down
5 changes: 4 additions & 1 deletion test/playground.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@
<html lang="en">
<title>O hai</title>

<div class="foo" id="foo">foo</div>
<button aria-description="Undefined global">myVar</button>

<div id="aria-describedby">Undefined global</div>
<button aria-describedby="aria-describedby">myVar</button>
straker marked this conversation as resolved.
Show resolved Hide resolved

<script src="/axe.js"></script>
<script>
Expand Down
11 changes: 9 additions & 2 deletions test/rule-matches/skip-link-matches.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@ describe('skip-link-matches', function() {
rule = axe._audit.rules.find(function(rule) {
return rule.id === 'skip-link';
});
link = document.createElement('a');
fixture.innerHTML = '<div id="main"></div>';
fixture.innerHTML =
'<a href="" id="target" style="position: absolute; left: -10000px;">Click me</a><div id="main"></div>';
link = fixture.querySelector('#target');
axe._tree = axe.utils.getFlattenedTree(fixture);
});

Expand All @@ -22,6 +23,12 @@ describe('skip-link-matches', function() {
assert.isFunction(rule.matches);
});

it('returns false if the links is onscreen', function() {
link.removeAttribute('style');
link.href = '#main';
assert.isFalse(rule.matches(link));
});

it('returns false if the href attribute does not start with #', function() {
link.href = 'foo#bar';
assert.isFalse(rule.matches(link));
Expand Down