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 (
getRole(virtualNode, { noPresentational: true }) === null &&
gaiety-deque marked this conversation as resolved.
Show resolved Hide resolved
!hasChildTextNodes(virtualNode)
);
}

// Check if the current element is a landmark
function isRegion(virtualNode, options) {
const node = virtualNode.actualNode;
Expand Down
54 changes: 50 additions & 4 deletions test/checks/navigation/region.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 a few more tests here about other elements:

  • <canvas role="none"> passes
  • <object aria-label="foo"> fails
  • <svg role="none" aria-label="foo"> fails
  • <div role="none">foo</div> fails
    • <div role="none"></div> passes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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 the other tests as both unit and integration tests

Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,47 @@ 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));
});
Expand Down Expand Up @@ -166,6 +203,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
Loading