Skip to content

Commit

Permalink
fix(region): Decorative images ignored by region rule (#4412)
Browse files Browse the repository at this point in the history
Issue was around [decorative
images](https://www.w3.org/WAI/tutorials/images/decorative/),
specifically 1 pixel wide/tall marketing tracking images, that get added
outside of regions failing the region rule. An easy and fairly robust
solution [the issue opener agreed
with](#4145 (comment))
was ignoring images with `alt=''` and so that's what this PR implements.

Dev notes:

- Added `isPresentationGraphic/1` check which currently only handles
`alt=''` and ignores them for the region rule
- role='presentation' test added as well, but this already worked
previously prior to this code change
- svg I believe is handled differently already, there's a test called
`treats svg elements as regions` so the new function doesn't check for
svg's

fix: #4145
  • Loading branch information
gaiety-deque committed May 3, 2024
2 parents 19bde94 + 738e9e2 commit f8c918d
Show file tree
Hide file tree
Showing 5 changed files with 157 additions and 8 deletions.
17 changes: 15 additions & 2 deletions lib/checks/navigation/region-evaluate.js
@@ -1,4 +1,5 @@
import * as dom from '../../commons/dom';
import { hasChildTextNodes } from '../../commons/dom/has-content-virtual';
import { getRole } from '../../commons/aria';
import * as standards from '../../commons/standards';
import matches from '../../commons/matches';
Expand Down Expand Up @@ -45,7 +46,10 @@ function getRegionlessNodes(options) {
*/
function findRegionlessElms(virtualNode, options) {
const node = virtualNode.actualNode;
// End recursion if the element is a landmark, skiplink, or hidden content
// End recursion if the element is...
// - a landmark
// - a skiplink
// - hidden content
if (
getRole(virtualNode) === 'button' ||
isRegion(virtualNode, options) ||
Expand Down Expand Up @@ -73,7 +77,8 @@ function findRegionlessElms(virtualNode, options) {
// @see https://github.com/dequelabs/axe-core/issues/2049
} else if (
node !== document.body &&
dom.hasContent(node, /* noRecursion: */ true)
dom.hasContent(node, /* noRecursion: */ true) &&
!isShallowlyHidden(virtualNode)
) {
return [virtualNode];

Expand All @@ -86,6 +91,14 @@ function findRegionlessElms(virtualNode, options) {
}
}

function isShallowlyHidden(virtualNode) {
// The element itself is not visible to screen readers, but its descendants might be
return (
['none', 'presentation'].includes(getRole(virtualNode)) &&
!hasChildTextNodes(virtualNode)
);
}

// Check if the current element is a landmark
function isRegion(virtualNode, options) {
const node = virtualNode.actualNode;
Expand Down
90 changes: 86 additions & 4 deletions test/checks/navigation/region.js
Expand Up @@ -46,14 +46,87 @@ describe('region', function () {
assert.isTrue(checkEvaluate.apply(checkContext, checkArgs));
});

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

assert.isFalse(checkEvaluate.apply(checkContext, checkArgs));
});

it('should return true when img content outside of the region is decorative, via an empty alt attr', function () {
const checkArgs = checkSetup(`
<img id="target" src="#" alt="" />
<div role="main">Content</div>
`);

assert.isTrue(checkEvaluate.apply(checkContext, checkArgs));
});

it('should return true when img content outside of the region is explicitly decorative, via a presentation role', function () {
const checkArgs = checkSetup(`
<img id="target" src="#" role="presentation" />
<div role="main">Content</div>
`);

assert.isTrue(checkEvaluate.apply(checkContext, checkArgs));
});

it('should return false when img content outside of the region is focusable (implicit role=img)', function () {
const checkArgs = checkSetup(`
<img id="target" src="#" tabindex="0" alt="" />
<div role="main">Content</div>
`);

assert.isFalse(checkEvaluate.apply(checkContext, checkArgs));
});

it('should return false when img content outside of the region has a global aria attribute (implicit role=img)', function () {
const checkArgs = checkSetup(`
<img id="target" src="#" aria-atomic="true" alt="" />
<div role="main">Content</div>
`);

assert.isFalse(checkEvaluate.apply(checkContext, checkArgs));
});

it('should return true when canvas role=none', function () {
const checkArgs = checkSetup(`
<canvas id="target" role="none" />
<div role="main">Content</div>
`);

assert.isTrue(checkEvaluate.apply(checkContext, checkArgs));
});

it('should return false when object has an aria-label', function () {
const checkArgs = checkSetup(`
<object id="target" aria-label="bar"></object>
<div role="main">Content</div>
`);

assert.isFalse(checkEvaluate.apply(checkContext, checkArgs));
});

it('should return false when a non-landmark has text content but a role=none', function () {
const checkArgs = checkSetup(`
<div id="target" role="none">apples</div>
<div role="main">Content</div>
`);

assert.isFalse(checkEvaluate.apply(checkContext, checkArgs));
});

it('should return true when a non-landmark does NOT have text content and a role=none', function () {
const checkArgs = checkSetup(`
<div id="target" role="none"></div>
<div role="main">Content</div>
`);

assert.isTrue(checkEvaluate.apply(checkContext, checkArgs));
});

it('should return true when textless text content is outside the region', function () {
var checkArgs = checkSetup(
'<p id="target"></p><div role="main"><h1 id="mainheader" tabindex="0">Introduction</h1></div>'
Expand Down Expand Up @@ -166,6 +239,15 @@ describe('region', function () {
assert.isFalse(checkEvaluate.apply(checkContext, checkArgs));
});

it('ignores native landmark elements with an overwriting role with a nested child', function () {
var checkArgs = checkSetup(`
<main id="target" role="none"><p>Content</p></main>
<div role="main">Content</div>
`);

assert.isFalse(checkEvaluate.apply(checkContext, checkArgs));
});

it('returns false for content outside of form tags with accessible names', function () {
var checkArgs = checkSetup(
'<p id="target">Text</p><form aria-label="form"></form>'
Expand Down
16 changes: 16 additions & 0 deletions test/integration/full/region/region-fail.html
Expand Up @@ -40,6 +40,22 @@ <h1 id="mainheader" tabindex="0">This is a header.</h1>
</section>
</div>

<div id="img-no-alt">
<img src="#" />
</div>
<div id="img-focusable">
<img src="#" tabindex="0" alt="" />
</div>
<div id="img-aria-global">
<img src="#" aria-atomic="true" alt="" />
</div>
<div id="labeled-object">
<object aria-label="bar"></object>
</div>
<div id="none-role-div">
<div id="target" role="none">apples</div>
</div>

This should be ignored

<main id="mocha"></main>
Expand Down
32 changes: 30 additions & 2 deletions test/integration/full/region/region-fail.js
Expand Up @@ -15,12 +15,40 @@ describe('region fail test', function () {
});

describe('violations', function () {
it('should find one', function () {
assert.lengthOf(results.violations[0].nodes, 1);
it('should find all violations', function () {
assert.lengthOf(results.violations[0].nodes, 6);
});

it('should find wrapper', function () {
assert.deepEqual(results.violations[0].nodes[0].target, ['#wrapper']);
});

it('should find image without an alt tag', function () {
assert.deepEqual(results.violations[0].nodes[1].target, ['#img-no-alt']);
});

it('should find focusable image', function () {
assert.deepEqual(results.violations[0].nodes[2].target, [
'#img-focusable'
]);
});

it('should find image with global aria attr', function () {
assert.deepEqual(results.violations[0].nodes[3].target, [
'#img-aria-global'
]);
});

it('should find object with a label', function () {
assert.deepEqual(results.violations[0].nodes[4].target, [
'#labeled-object'
]);
});

it('should find div with an role of none', function () {
assert.deepEqual(results.violations[0].nodes[5].target, [
'#none-role-div'
]);
});
});
});
10 changes: 10 additions & 0 deletions test/integration/full/region/region-pass.html
Expand Up @@ -20,6 +20,16 @@
</head>
<body>
<a href="#mainheader" id="skiplink">This is a skip link.</a>
<!-- Decorative image -->
<img src="#" alt="" />
<!-- Decorative image -->
<img src="#" role="presentation" />
<!-- SVG's may be outside of a landmark -->
<svg role="none" aria-label="foo"></svg>
<!-- Div with a role of none may be outside a landmark -->
<div role="none"></div>
<!-- Canvas with a role of none may be outside a landmark -->
<canvas role="none" />
<div id="wrapper">
<div role="main">
<h1 id="mainheader" tabindex="0">This is a header.</h1>
Expand Down

0 comments on commit f8c918d

Please sign in to comment.