Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(region): Decorative images ignored by region rule #4412

Merged
merged 11 commits into from
May 3, 2024
Merged
17 changes: 15 additions & 2 deletions lib/checks/navigation/region-evaluate.js
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add an integration test for this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added :)

Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Loading