Skip to content

Commit

Permalink
fix(region): contents in iframes should pass the region rule if the i…
Browse files Browse the repository at this point in the history
…frame itself is in a region (#2949)

* only run region rule on initiatior; iframe results will be on their own reports

* add test

* content in iframes that are in regions should be counted as within the region

* add another nested iframe test

* move match ancestry function to shared file

* attempt to move things forward

* working now

* remove test that won't work because that logic is in the after method now

* add unit tests for after method

* test only result

* match ancestry tests

* passes

* add comment

* refactor

* more refactor

* add additional test
  • Loading branch information
clottman committed Jun 23, 2021
1 parent 4e978d4 commit 43145d6
Show file tree
Hide file tree
Showing 21 changed files with 518 additions and 36 deletions.
21 changes: 2 additions & 19 deletions lib/checks/navigation/heading-order-after.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { matchAncestry } from '../../core/utils';

export default function headingOrderAfter(results) {
// Construct a map of all headings on the page
const headingOrder = getHeadingOrder(results);
Expand Down Expand Up @@ -105,25 +107,6 @@ function addFrameToHeadingAncestry(heading, frameAncestry) {
return { ...heading, ancestry };
}

/**
* Check if two ancestries are identical
*/
function matchAncestry(ancestryA, ancestryB) {
if (ancestryA.length !== ancestryB.length) {
return false;
}
return ancestryA.every((selectorA, index) => {
const selectorB = ancestryB[index];
if (!Array.isArray(selectorA)) {
return selectorA === selectorB;
}
if (selectorA.length !== selectorB.length) {
return false;
}
return selectorA.every((str, index) => selectorB[index] === str);
});
}

/**
* Shorten an array by some number of items
*/
Expand Down
31 changes: 31 additions & 0 deletions lib/checks/navigation/region-after.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import { matchAncestry } from '../../core/utils';

function regionAfter(results) {
const iframeResults = results.filter(r => r.data.isIframe);

results.forEach(r => {
// continue if the element passed the check or is not in a frame
if (r.result || r.node.ancestry.length === 1) {
return;
}

const frameAncestry = r.node.ancestry.slice(0, -1);
for (const iframeResult of iframeResults) {
// if the container frame passed the check, this element should also pass
if (matchAncestry(frameAncestry, iframeResult.node.ancestry)) {
r.result = iframeResult.result;
break;
}
}
});

// iframe elements should always pass
iframeResults.forEach(r => {
if (!r.result) {
r.result = true;
}
});
return results;
}

export default regionAfter;
13 changes: 12 additions & 1 deletion lib/checks/navigation/region-evaluate.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ function findRegionlessElms(virtualNode, options) {
// End recursion if the element is a landmark, skiplink, or hidden content
if (
isRegion(virtualNode, options) ||
['iframe', 'frame'].includes(virtualNode.props.nodeName) ||
(dom.isSkipLink(virtualNode.actualNode) &&
dom.getElementByReference(virtualNode.actualNode, 'href')) ||
!dom.isVisible(node, true)
Expand All @@ -52,7 +53,12 @@ function findRegionlessElms(virtualNode, options) {
vNode._hasRegionDescendant = true;
vNode = vNode.parent;
}

// iframes are counted as regionless here, but parent tree should treat them as having content
// after method makes iframes count as regions
// this means siblings to iframes will fail, not the parent that contains an iframe and other children
if (['iframe', 'frame'].includes(virtualNode.props.nodeName)) {
return [virtualNode];
}
return [];

// Return the node is a content element. Ignore any direct text children
Expand All @@ -75,6 +81,11 @@ function findRegionlessElms(virtualNode, options) {

function regionEvaluate(node, options, virtualNode) {
let regionlessNodes = cache.get('regionlessNodes');

this.data({
isIframe: ['iframe', 'frame'].includes(virtualNode.props.nodeName)
});

if (regionlessNodes) {
return !regionlessNodes.includes(virtualNode);
}
Expand Down
3 changes: 2 additions & 1 deletion lib/checks/navigation/region.json
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
{
"id": "region",
"evaluate": "region-evaluate",
"after": "region-after",
"options": {
"regionMatcher": "dialog, [role=dialog], [role=alertdialog], svg, iframe"
"regionMatcher": "dialog, [role=dialog], [role=alertdialog], svg"
},
"metadata": {
"impact": "moderate",
Expand Down
2 changes: 2 additions & 0 deletions lib/core/base/metadata-function-map.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ import internalLinkPresentEvaluate from '../../checks/navigation/internal-link-p
import metaRefreshEvaluate from '../../checks/navigation/meta-refresh-evaluate';
import pAsHeadingEvaluate from '../../checks/navigation/p-as-heading-evaluate';
import regionEvaluate from '../../checks/navigation/region-evaluate';
import regionAfter from '../../checks/navigation/region-after';
import skipLinkEvaluate from '../../checks/navigation/skip-link-evaluate';
import uniqueFrameTitleAfter from '../../checks/navigation/unique-frame-title-after';
import uniqueFrameTitleEvaluate from '../../checks/navigation/unique-frame-title-evaluate';
Expand Down Expand Up @@ -235,6 +236,7 @@ const metadataFunctionMap = {
'meta-refresh-evaluate': metaRefreshEvaluate,
'p-as-heading-evaluate': pAsHeadingEvaluate,
'region-evaluate': regionEvaluate,
'region-after': regionAfter,
'skip-link-evaluate': skipLinkEvaluate,
'unique-frame-title-after': uniqueFrameTitleAfter,
'unique-frame-title-evaluate': uniqueFrameTitleEvaluate,
Expand Down
1 change: 1 addition & 0 deletions lib/core/utils/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ export {
matchesExpression,
convertSelector
} from './matches';
export { default as matchAncestry } from './match-ancestry';
export { default as memoize } from './memoize';
export { default as mergeResults } from './merge-results';
export { default as nodeSorter } from './node-sorter';
Expand Down
20 changes: 20 additions & 0 deletions lib/core/utils/match-ancestry.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/**
* Check if two ancestries are identical
*/
function matchAncestry(ancestryA, ancestryB) {
if (ancestryA.length !== ancestryB.length) {
return false;
}
return ancestryA.every((selectorA, index) => {
const selectorB = ancestryB[index];
if (!Array.isArray(selectorA)) {
return selectorA === selectorB;
}
if (selectorA.length !== selectorB.length) {
return false;
}
return selectorA.every((str, index) => selectorB[index] === str);
});
}

export default matchAncestry;
153 changes: 153 additions & 0 deletions test/checks/navigation/region-after.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,153 @@
describe('region-after', function() {
'use strict';

var checkContext = axe.testUtils.MockCheckContext();

afterEach(function() {
checkContext.reset();
});

it('should always pass iframes', function() {
var results = checks.region.after([
{
data: { isIframe: true },
node: {
ancestry: ['html > body > iframe']
},
result: false
},
{
data: { isIframe: false },
node: {
ancestry: ['html > body > iframe', 'html > body > p']
},
result: false
}
]);
assert.equal(results[0].result, true);
assert.equal(results[1].result, false);
});

it('should pass children of iframes if the iframe contained in it is in a region', function() {
var results = checks.region.after([
{
data: { isIframe: true },
node: {
ancestry: ['html > body > iframe']
},
result: true
},
{
data: { isIframe: false },
node: {
ancestry: ['html > body > iframe', 'html > body > p']
},
result: false
}
]);

assert.equal(results[0].result, true);
assert.equal(results[1].result, true);
});

it('should pass nested iframes', function() {
var results = checks.region.after([
{
data: { isIframe: true },
node: {
ancestry: ['html > body > iframe']
},
result: false
},
{
data: { isIframe: true },
node: {
ancestry: ['html > body > iframe', 'html > body > iframe']
},
result: false
},
{
data: { isIframe: false },
node: {
ancestry: [
'html > body > iframe',
'html > body > iframe',
'html > body > p'
]
},
result: false
}
]);

assert.equal(results[0].result, true);
assert.equal(results[1].result, true);
assert.equal(results[2].result, false);
});

it('should pass children of nested iframes if the nested iframe is in a region', function() {
var results = checks.region.after([
{
data: { isIframe: true },
node: {
ancestry: ['html > body > iframe']
},
result: false
},
{
data: { isIframe: true },
node: {
ancestry: ['html > body > iframe', 'html > body > iframe']
},
result: true
},
{
data: { isIframe: false },
node: {
ancestry: [
'html > body > iframe',
'html > body > iframe',
'html > body > p'
]
},
result: false
}
]);

assert.equal(results[0].result, true);
assert.equal(results[1].result, true);
assert.equal(results[2].result, true);
});

it('should pass content if a grandparent frame passes', function() {
var results = checks.region.after([
{
data: { isIframe: true },
node: {
ancestry: ['html > body > iframe']
},
result: true
},
{
data: { isIframe: true },
node: {
ancestry: ['html > body > iframe', 'html > body > iframe']
},
result: false
},
{
data: { isIframe: false },
node: {
ancestry: [
'html > body > iframe',
'html > body > iframe',
'html > body > p'
]
},
result: false
}
]);
assert.equal(results[0].result, true);
assert.equal(results[1].result, true);
assert.equal(results[2].result, true);
});
});
8 changes: 0 additions & 8 deletions test/checks/navigation/region.js
Original file line number Diff line number Diff line change
Expand Up @@ -290,14 +290,6 @@ describe('region', function() {
assert.isTrue(checkEvaluate.apply(checkContext, checkArgs));
});

it('treats iframe elements as regions', function() {
var checkArgs = checkSetup(
'<iframe id="target"></iframe><div role="main">Content</div>'
);

assert.isTrue(checkEvaluate.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>'
Expand Down
60 changes: 60 additions & 0 deletions test/core/utils/matchAncestry.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
describe('axe.utils.matchAncestry', function() {
'use strict';
var fixture = document.getElementById('fixture');

afterEach(function() {
fixture.innerHTML = '';
});

it('should be a function', function() {
assert.isFunction(axe.utils.matchAncestry);
});

it('should match when ancestry is the same and one level', function() {
var result = axe.utils.matchAncestry(
['html > body > div:nth-child(1)'],
['html > body > div:nth-child(1)']
);
assert.isTrue(result);
});

it('should not match when ancestry is different and one level', function() {
var result = axe.utils.matchAncestry(
['html > body > div:nth-child(3)'],
['html > body > div:nth-child(1)']
);
assert.isFalse(result);
});

it('should not match when ancestries have different numbers of elements', function() {
var result = axe.utils.matchAncestry(
['iframe', 'html > body > div:nth-child(1)'],
['html > body > div:nth-child(1)']
);
assert.isFalse(result);
});

it('should not match when first level is different and second level is the same', function() {
var result = axe.utils.matchAncestry(
['iframe', 'html > body > div:nth-child(1)'],
['otherIframe', 'html > body > div:nth-child(1)']
);
assert.isFalse(result);
});

it('should not match when second level is different and first level is the same', function() {
var result = axe.utils.matchAncestry(
['iframe', 'html > body > div:nth-child(1)'],
['iframe', 'html > body > div:nth-child(2)']
);
assert.isFalse(result);
});

it('should match when all levels are the same', function() {
var result = axe.utils.matchAncestry(
['iframe', 'iframe2', 'html > body > div:nth-child(1)'],
['iframe', 'iframe2', 'html > body > div:nth-child(1)']
);
assert.isTrue(result);
});
});
10 changes: 10 additions & 0 deletions test/integration/full/region/frames/region-fail.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="utf8" />
<script src="/axe.js"></script>
</head>
<body>
<p id="wrapper">Region content no region</p>
</body>
</html>
Loading

0 comments on commit 43145d6

Please sign in to comment.