Skip to content

Commit

Permalink
fix(region): Ignore forms without accessible name as landmarks
Browse files Browse the repository at this point in the history
  • Loading branch information
WilcoFiers committed Mar 3, 2018
1 parent aa074fe commit 8ad2718
Show file tree
Hide file tree
Showing 5 changed files with 92 additions and 19 deletions.
24 changes: 17 additions & 7 deletions lib/checks/navigation/region.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,28 @@ const implicitLandmarks = landmarkRoles
.filter(r => r !== null);

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

// Check if the current element is a landmark
function isLandmark (node) {
if (node.hasAttribute('role')) {
return landmarkRoles.includes(node.getAttribute('role').toLowerCase());
function isLandmark (virtualNode) {
var node = virtualNode.actualNode;
var explictRole = (node.getAttribute('role') || '').trim().toLowerCase();

if (explictRole) {
if (explictRole === 'form') {
return !!aria.labelVirtual(virtualNode);
}
return landmarkRoles.includes(explictRole);
} else {
// Check if the node matches any of the CSS selectors of implicit landmarks
return implicitLandmarks.some((implicitSelector) => {
return axe.utils.matchesSelector(node, implicitSelector);
let matches = axe.utils.matchesSelector(node, implicitSelector);
if (node.tagName.toLowerCase() === 'form') {
return matches && !!aria.labelVirtual(virtualNode);
}
return matches;
});
}
}
Expand All @@ -39,7 +49,7 @@ function isLandmark (node) {
function findRegionlessElms (virtualNode) {
const node = virtualNode.actualNode;
// End recursion if the element is a landmark, skiplink, or hidden content
if (isLandmark(node) || isSkipLink(node) || !dom.isVisible(node, true)) {
if (isLandmark(virtualNode) || isSkipLink(virtualNode) || !dom.isVisible(node, true)) {
return [];

// Return the node is a content element
Expand Down
51 changes: 51 additions & 0 deletions test/checks/navigation/region.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,57 @@ describe('region', function () {
assert.deepEqual(checkContext._relatedNodes, [fixture.querySelector('main')]);
});

it('returns false for content outside of form tags with accessible names', function () {
var checkArgs = checkSetup('<div id="target"><p>Text</p><form aria-label="form"></form></div>');

assert.isFalse(checks.region.evaluate.apply(checkContext, checkArgs));
assert.lengthOf(checkContext._relatedNodes, 1);
assert.deepEqual(checkContext._relatedNodes, [fixture.querySelector('p')]);
});

it('ignores unlabeled forms as they are not landmarks', function () {
var checkArgs = checkSetup('<div id="target"><p>Text</p><form><fieldset>foo</fieldset></form></div>');

assert.isFalse(checks.region.evaluate.apply(checkContext, checkArgs));
assert.lengthOf(checkContext._relatedNodes, 2);
assert.deepEqual(checkContext._relatedNodes, [fixture.querySelector('p'), fixture.querySelector('fieldset')]);
});

it('treats <forms> with aria label as landmarks', function () {
var checkArgs = checkSetup('<form id="target" aria-label="foo"><p>This is random content.</p></form>');
assert.isTrue(checks.region.evaluate.apply(checkContext, checkArgs));
});

it('treats role=forms with aria label as landmarks', function () {
var checkArgs = checkSetup('<div role="form" id="target" aria-label="foo"><p>This is random content.</p></form>');
assert.isTrue(checks.region.evaluate.apply(checkContext, checkArgs));
});

it('treats forms without aria label as not a landmarks', function () {
var checkArgs = checkSetup('<form id="target"><p>This is random content.</p></form>');

assert.isFalse(checks.region.evaluate.apply(checkContext, checkArgs));
assert.lengthOf(checkContext._relatedNodes, 1);
assert.deepEqual(checkContext._relatedNodes, [fixture.querySelector('p')]);
});

it('treats forms with an empty aria label as not a landmarks', function () {
var checkArgs = checkSetup('<form id="target" aria-label=" "><p>This is random content.</p></form>');

assert.isFalse(checks.region.evaluate.apply(checkContext, checkArgs));
assert.lengthOf(checkContext._relatedNodes, 1);
assert.deepEqual(checkContext._relatedNodes, [fixture.querySelector('p')]);
});

it('treats forms with an empty aria label as not a landmarks', function () {
var checkArgs = checkSetup('<div role="form" id="target"><p>This is random content.</p></form>');

assert.isFalse(checks.region.evaluate.apply(checkContext, checkArgs));
assert.lengthOf(checkContext._relatedNodes, 1);
assert.deepEqual(checkContext._relatedNodes, [fixture.querySelector('p')]);
});


(shadowSupport.v1 ? it : xit)('should test Shadow tree content', function () {
var div = document.createElement('div');
var shadow = div.attachShadow({ mode: 'open' });
Expand Down
27 changes: 15 additions & 12 deletions test/integration/full/region/region-fail.html
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,23 @@
</script>
</head>
<body>
<a href="stuff.html#mainheader" id="notskiplink">This is not a skip link.</a>
<div id="wrapper">
<div>
<h1 id="mainheader" tabindex="0">This is a header.</h1>
</div>
<a href="stuff.html#mainheader" id="notskiplink">This is not a skip link.</a>
<form>
<div>
<h1 id="mainheader" tabindex="0">This is a header.</h1>
</div>
</form>
<ul>
<li>Home</li>
<li>Other</li>
</ul>
<section>
<p>Content</p>
</section>
</div>
<ul>
<li>Home</li>
<li>Other</li>
</ul>
<section>
<p>Content</p>
</section>
<div id="mocha"></div>

<main id="mocha"></main>
<script src="region-fail.js"></script>
<script src="/test/integration/adapter.js"></script>
</body>
Expand Down
6 changes: 6 additions & 0 deletions test/integration/full/region/region-fail.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@ describe('region fail test', function () {
it('should find html', function () {
assert.deepEqual(results.violations[0].nodes[0].target, ['html']);
});

it('should have all text content as related nodes', function () {
var wrapper = document.querySelector('#wrapper');
assert.equal(results.violations[0].nodes[0].any[0].relatedNodes.length,
wrapper.querySelectorAll('h1, li, p, a').length);
});
});

describe('passes', function () {
Expand Down
3 changes: 3 additions & 0 deletions test/integration/full/region/region-pass.html
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@
<h1 id="mainheader" tabindex="0">This is a header.</h1>
</div>
</div>
<form aria-label="My wonderful form">
<p>My form!</p>
</form>
<ul role="navigation">
<li>Home</li>
<li>Other</li>
Expand Down

0 comments on commit 8ad2718

Please sign in to comment.