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

* fix(skip-link,region): improve defenition of skip-link

* fix selector

* add test

* use check test

* redelete unneeded file

lib/checks/navigation/region.js

@@ -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;
@@ -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 [];

lib/commons/dom/is-skip-link.js

@@ -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) {
+	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
+	);
+};

lib/rules/skip-link-matches.js

@@ -1 +1 @@
-return /^#[^/!]/.test(node.getAttribute('href'));
+return axe.commons.dom.isSkipLink(node);

lib/rules/skip-link.json

@@ -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": {

test/checks/navigation/skip-link.js

@@ -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>' +
@@ -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));
-	});
 });

test/commons/dom/is-skip-link.js

@@ -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));
+	});
+});

test/integration/full/skip-link/skip-link-fail.html

@@ -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>

test/integration/full/skip-link/skip-link-fail.js

@@ -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);
+	});
+});

test/integration/full/skip-link/skip-link-pass.html

@@ -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>

test/integration/full/skip-link/skip-link-pass.js

@@ -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);
+	});
+});