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

straker · web-flow · commit ab5a011e72f6 · 2020-03-06T09:56:17.000-07:00
* fix(skip-link): identify as skip-link only if the link is offscreen

* fix region test

* revert changes, only check offscreen in skip-link
diff --git a/lib/rules/skip-link-matches.js b/lib/rules/skip-link-matches.js
@@ -1 +1 @@
-return axe.commons.dom.isSkipLink(node);
+return axe.commons.dom.isSkipLink(node) && axe.commons.dom.isOffscreen(node);
diff --git a/test/integration/full/skip-link/skip-link-fail.html b/test/integration/full/skip-link/skip-link-fail.html
@@ -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>
diff --git a/test/integration/full/skip-link/skip-link-pass.html b/test/integration/full/skip-link/skip-link-pass.html
@@ -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,
@@ -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>
diff --git a/test/rule-matches/skip-link-matches.js b/test/rule-matches/skip-link-matches.js
@@ -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);
 	});
 
@@ -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));

Original file line number	Diff line number	Diff line change
`@@ -1 +1 @@`
`1`		`-return axe.commons.dom.isSkipLink(node);`
	`1`	`+return axe.commons.dom.isSkipLink(node) && axe.commons.dom.isOffscreen(node);`