Skip to content

Commit

Permalink
fix(region): return outermost regionless node instead of html (#1980)
Browse files Browse the repository at this point in the history 
* fix(region): return outermost regionless node instead of html

* fix

* typo

* fix tests

* fix test again

* fixes

* revert pass

* Update test/checks/navigation/region.js

Co-Authored-By: Wilco Fiers <WilcoFiers@users.noreply.github.com>

Co-authored-by: Wilco Fiers <WilcoFiers@users.noreply.github.com>
  • Loading branch information
@straker @WilcoFiers
straker and WilcoFiers committed Jan 29, 2020
1 parent 2600a06 commit 8d77be2
Show file tree
Hide file tree
Showing 7 changed files with 119 additions and 95 deletions.
1 change: 0 additions & 1 deletion lib/checks/navigation/region-after.js
View file

This file was deleted.

37 changes: 33 additions & 4 deletions lib/checks/navigation/region.js
View file
Expand Up @@ -2,6 +2,11 @@ const { dom, aria } = axe.commons;
const landmarkRoles = aria.getRolesByType('landmark');
const implicitAriaLiveRoles = ['alert', 'log', 'status'];

let regionlessNodes = axe._cache.get('regionlessNodes');
if (regionlessNodes) {
return !regionlessNodes.includes(virtualNode);
}

// Create a list of nodeNames that have a landmark as an implicit role
const implicitLandmarks = landmarkRoles
.reduce((arr, role) => arr.concat(aria.implicitNodes(role)), [])
Expand Down Expand Up @@ -52,11 +57,18 @@ function findRegionlessElms(virtualNode) {
dom.getElementByReference(virtualNode.actualNode, 'href')) ||
!dom.isVisible(node, true)
) {
// Mark each parent node as having region descendant
let vNode = virtualNode;
while (vNode) {
vNode._hasRegionDescendant = true;
vNode = vNode.parent;
}

return [];

// Return the node is a content element
} else if (dom.hasContent(node, /* noRecursion: */ true)) {
return [node];
return [virtualNode];

// Recursively look at all child elements
} else {
Expand All @@ -67,7 +79,24 @@ function findRegionlessElms(virtualNode) {
}
}

var regionlessNodes = findRegionlessElms(virtualNode);
this.relatedNodes(regionlessNodes);
regionlessNodes = findRegionlessElms(axe._tree[0])
// Find first parent marked as having region descendant (or body) and
// return the node right before it as the "outer" element
.map(vNode => {
while (
vNode.parent &&
!vNode.parent._hasRegionDescendant &&
vNode.parent.actualNode !== document.body
) {
vNode = vNode.parent;
}

return vNode;
})
// Remove duplicate containers
.filter((vNode, index, array) => {
return array.indexOf(vNode) === index;
});
axe._cache.set('regionlessNodes', regionlessNodes);

return regionlessNodes.length === 0;
return !regionlessNodes.includes(virtualNode);
1 change: 0 additions & 1 deletion lib/checks/navigation/region.json
View file
@@ -1,7 +1,6 @@
{
"id": "region",
"evaluate": "region.js",
"after": "region-after.js",
"metadata": {
"impact": "moderate",
"messages": {
Expand Down
3 changes: 1 addition & 2 deletions lib/rules/region.json
View file
@@ -1,7 +1,6 @@
{
"id": "region",
"selector": "html",
"pageLevel": true,
"selector": "body *",
"tags": ["cat.keyboard", "best-practice"],
"metadata": {
"description": "Ensures all page content is contained by landmarks",
Expand Down
144 changes: 79 additions & 65 deletions test/checks/navigation/region.js
View file
Expand Up @@ -4,6 +4,7 @@ describe('region', function() {
var fixture = document.getElementById('fixture');
var shadowSupport = axe.testUtils.shadowSupport;
var checkSetup = axe.testUtils.checkSetup;
var fixtureSetup = axe.testUtils.fixtureSetup;

var checkContext = new axe.testUtils.MockCheckContext();

Expand All @@ -12,152 +13,148 @@ describe('region', function() {
checkContext.reset();
});

it('should return true when all content is inside the region', function() {
it('should return true when content is inside the region', function() {
var checkArgs = checkSetup(
'<div id="target"><div role="main"><a href="a.html#mainheader">Click Here</a><div><h1 id="mainheader" tabindex="0">Introduction</h1></div></div></div>'
'<div role="main"><a id="target" href="a.html#mainheader">Click Here</a><div><h1 id="mainheader" tabindex="0">Introduction</h1></div></div>'
);

assert.isTrue(checks.region.evaluate.apply(checkContext, checkArgs));
assert.equal(checkContext._relatedNodes.length, 0);
});

it('should return false when img content is outside the region', function() {
var checkArgs = checkSetup(
'<div id="target"><img src="data:image/gif;base64,R0lGODlhEAAQAMQAAORHHOVSKudfOulrSOp3WOyDZu6QdvCchPGolfO0o/XBs/fNwfjZ0frl3/zy7////wAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAACH5BAkAABAALAAAAAAQABAAAAVVICSOZGlCQAosJ6mu7fiyZeKqNKToQGDsM8hBADgUXoGAiqhSvp5QAnQKGIgUhwFUYLCVDFCrKUE1lBavAViFIDlTImbKC5Gm2hB0SlBCBMQiB0UjIQA7"><div role="main"><h1 id="mainheader" tabindex="0">Introduction</h1></div></div>'
'<img id="target" src="data:image/gif;base64,R0lGODlhEAAQAMQAAORHHOVSKudfOulrSOp3WOyDZu6QdvCchPGolfO0o/XBs/fNwfjZ0frl3/zy7////wAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAACH5BAkAABAALAAAAAAQABAAAAVVICSOZGlCQAosJ6mu7fiyZeKqNKToQGDsM8hBADgUXoGAiqhSvp5QAnQKGIgUhwFUYLCVDFCrKUE1lBavAViFIDlTImbKC5Gm2hB0SlBCBMQiB0UjIQA7"><div role="main"><h1 id="mainheader" tabindex="0">Introduction</h1></div>'
);

assert.isFalse(checks.region.evaluate.apply(checkContext, checkArgs));
assert.equal(checkContext._relatedNodes.length, 1);
});

it('should return true when textless text content is outside the region', function() {
var checkArgs = checkSetup(
'<div id="target"><p></p><div role="main"><h1 id="mainheader" tabindex="0">Introduction</h1></div></div>'
'<p id="target"></p><div role="main"><h1 id="mainheader" tabindex="0">Introduction</h1></div>'
);

assert.isTrue(checks.region.evaluate.apply(checkContext, checkArgs));
assert.equal(checkContext._relatedNodes.length, 0);
});

it('should return true when wrapper content is outside the region', function() {
var checkArgs = checkSetup(
'<div id="target"><div><div role="main"><h1 id="mainheader" tabindex="0">Introduction</h1></div></div></div>'
'<div id="target"><div role="main"><h1 id="mainheader" tabindex="0">Introduction</h1></div></div>'
);

assert.isTrue(checks.region.evaluate.apply(checkContext, checkArgs));
assert.equal(checkContext._relatedNodes.length, 0);
});

it('should return true when invisible content is outside the region', function() {
var checkArgs = checkSetup(
'<div id="target"><p style="display: none">Click Here</p><div role="main"><h1 id="mainheader" tabindex="0">Introduction</h1></div></div>'
'<p id="target" style="display: none">Click Here</p><div role="main"><h1 id="mainheader" tabindex="0">Introduction</h1></div>'
);

assert.isTrue(checks.region.evaluate.apply(checkContext, checkArgs));
assert.equal(checkContext._relatedNodes.length, 0);
});

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

assert.isTrue(checks.region.evaluate.apply(checkContext, checkArgs));
assert.equal(checkContext._relatedNodes.length, 0);
});

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

assert.isTrue(checks.region.evaluate.apply(checkContext, checkArgs));
assert.equal(checkContext._relatedNodes.length, 0);
});

it('should return false when there is a non-region element', function() {
var checkArgs = checkSetup(
'<div id="target"><div>This is random content.</div><div role="main"><h1 id="mainheader">Introduction</h1></div></div>'
'<div id="target">This is random content.</div><div role="main"><h1 id="mainheader">Introduction</h1></div>'
);

assert.isFalse(checks.region.evaluate.apply(checkContext, checkArgs));
assert.equal(checkContext._relatedNodes.length, 1);
});

it('should return the first item when after is called', function() {
assert.equal(checks.region.after([2, 3, 1]), 2);
});

it('should return false when there is a non-skiplink', function() {
var checkArgs = checkSetup(
'<div id="target"><a href="something.html#mainheader">Click Here</a><div role="main"><h1 id="mainheader">Introduction</h1></div></div>'
'<a id="target" href="something.html#mainheader">Click Here</a><div role="main"><h1 id="mainheader">Introduction</h1></div>'
);

assert.isFalse(checks.region.evaluate.apply(checkContext, checkArgs));
assert.equal(checkContext._relatedNodes.length, 1);
});

it('should return true if the non-region element is a script', function() {
var checkArgs = checkSetup(
'<div id="target"><script>axe.run()</script><div role="main">Content</div></div>'
'<script id="target">axe.run()</script><div role="main">Content</div>'
);

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

it('should considered aria labelled elements as content', function() {
var checkArgs = checkSetup(
'<div id="target"><div aria-label="axe-core logo" role="img"></div><div role="main">Content</div></div>'
'<div id="target" aria-label="axe-core logo" role="img"></div><div role="main">Content</div>'
);

assert.isFalse(checks.region.evaluate.apply(checkContext, checkArgs));
assert.deepEqual(checkContext._relatedNodes, [
fixture.querySelector('div[aria-label]')
]);
});

it('should allow native landmark elements', function() {
it('should allow native header elements', function() {
var checkArgs = checkSetup(
'<header id="target">branding</header><main>Content </main><aside>stuff</aside><footer>copyright</footer>'
);

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

it('should allow native main elements', function() {
var checkArgs = checkSetup(
'<header>branding</header><main id="target">Content </main><aside>stuff</aside><footer>copyright</footer>'
);

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

it('should allow native aside elements', function() {
var checkArgs = checkSetup(
'<header>branding</header><main>Content </main><aside id="target">stuff</aside><footer>copyright</footer>'
);

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

it('should allow native footer elements', function() {
var checkArgs = checkSetup(
'<div id="target"><header>branding</header><main>Content </main><aside>stuff</aside><footer>copyright</footer></div>'
'<header>branding</header><main>Content </main><aside>stuff</aside><footer id="target">copyright</footer>'
);

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

it('ignores native landmark elements with an overwriting role', function() {
var checkArgs = checkSetup(
'<div id="target"><header role="heading" level="1"></header><main role="none">Content</main></div>'
'<main id="target" role="none">Content</main><div role="main">Content</div>'
);

assert.isFalse(checks.region.evaluate.apply(checkContext, checkArgs));
assert.lengthOf(checkContext._relatedNodes, 1);
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>'
'<p id="target">Text</p><form aria-label="form"></form>'
);

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>'
'<form id="target"><fieldset>foo</fieldset></form><div role="main">Content</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() {
Expand All @@ -176,22 +173,18 @@ describe('region', function() {

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

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>'
'<form id="target" aria-label=" "><p>This is random content.</p></form><div role="main">Content</div>'
);

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

it('treats forms with non empty titles as landmarks', function() {
Expand All @@ -204,12 +197,10 @@ describe('region', function() {

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

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

it('treats ARIA forms with no label or title as landmarks', function() {
Expand All @@ -236,7 +227,7 @@ describe('region', function() {

it('does not allow content in aria-live=off', function() {
var checkArgs = checkSetup(
'<div aria-live="off" id="target"><p>This is random content.</p></div>'
'<div aria-live="off" id="target"><p>This is random content.</p></div><div role="main">Content</div>'
);
assert.isFalse(checks.region.evaluate.apply(checkContext, checkArgs));
});
Expand Down Expand Up @@ -284,14 +275,30 @@ describe('region', function() {
assert.isTrue(checks.region.evaluate.apply(checkContext, checkArgs));
});

it('returns the outermost element as the error', function() {
var checkArgs = checkSetup(
'<div id="target"><p>This is random content.</p></div><div role="main"><h1 id="mainheader" tabindex="0">Introduction</h1></div>'
);

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

(shadowSupport.v1 ? it : xit)('should test Shadow tree content', function() {
var div = document.createElement('div');
var shadow = div.attachShadow({ mode: 'open' });
shadow.innerHTML = 'Some text';
var checkArgs = checkSetup(div);

assert.isFalse(checks.region.evaluate.apply(checkContext, checkArgs));
assert.deepEqual(checkContext._relatedNodes, [div]);
fixtureSetup(div);
var virutalNode = axe._tree[0];

// fixture is the outermost element
assert.isFalse(
checks.region.evaluate.call(
checkContext,
virutalNode.actualNode,
null,
virutalNode
)
);
});

(shadowSupport.v1 ? it : xit)('should test slotted content', function() {
Expand All @@ -302,21 +309,28 @@ describe('region', function() {
var checkArgs = checkSetup(div);

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

(shadowSupport.v1 ? it : xit)(
'should ignore skiplink targets inside shadow trees',
function() {
var div = document.createElement('div');
div.innerHTML = '<a href="#foo">skiplink</a><div>Content</div>';
div.innerHTML =
'<a id="target" href="#foo">skiplink</a><div>Content</div>';

var shadow = div.querySelector('div').attachShadow({ mode: 'open' });
shadow.innerHTML = '<div role="main" id=#foo"><slot></slot></div>';
var checkArgs = checkSetup(div);

assert.isFalse(checks.region.evaluate.apply(checkContext, checkArgs));
assert.deepEqual(checkContext._relatedNodes, [div.querySelector('a')]);
fixtureSetup(div);
var virutalNode = axe.utils.getNodeFromTree(div.querySelector('#target'));

assert.isFalse(
checks.region.evaluate.call(
checkContext,
virutalNode.actualNode,
null,
virutalNode
)
);
}
);

Expand Down

0 comments on commit 8d77be2

Please sign in to comment.