From ace25cd51fab327a9a5239608bdfba3bdad2f0ee Mon Sep 17 00:00:00 2001 From: Ava Gaiety W Date: Tue, 16 Apr 2024 13:06:29 -0500 Subject: [PATCH 1/9] fix(region): img alt='' ignored by region rule added isPresentationGraphic check which currently only handles alt='' and ignores them for the region rule svg I believe is handled differently already role='presentation' test added as well, use-case already handled fix: #4145 --- lib/checks/navigation/region-evaluate.js | 13 ++++++++++-- test/checks/navigation/region.js | 25 +++++++++++++++++++++--- 2 files changed, 33 insertions(+), 5 deletions(-) diff --git a/lib/checks/navigation/region-evaluate.js b/lib/checks/navigation/region-evaluate.js index 0e14b8a168..c29e29b8a0 100644 --- a/lib/checks/navigation/region-evaluate.js +++ b/lib/checks/navigation/region-evaluate.js @@ -45,14 +45,19 @@ 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 + // - a presentation graphic if ( getRole(virtualNode) === 'button' || isRegion(virtualNode, options) || ['iframe', 'frame'].includes(virtualNode.props.nodeName) || (dom.isSkipLink(virtualNode.actualNode) && dom.getElementByReference(virtualNode.actualNode, 'href')) || - !dom.isVisibleToScreenReaders(node) + !dom.isVisibleToScreenReaders(node) || + isPresentationGraphic(virtualNode) ) { // Mark each parent node as having region descendant let vNode = virtualNode; @@ -86,6 +91,10 @@ function findRegionlessElms(virtualNode, options) { } } +function isPresentationGraphic(virtualNode) { + return virtualNode.props.nodeName === 'img' && virtualNode.attr('alt') === ''; +} + // Check if the current element is a landmark function isRegion(virtualNode, options) { const node = virtualNode.actualNode; diff --git a/test/checks/navigation/region.js b/test/checks/navigation/region.js index fb4d13b3e9..ca19f35dac 100644 --- a/test/checks/navigation/region.js +++ b/test/checks/navigation/region.js @@ -47,13 +47,32 @@ describe('region', function () { }); it('should return false when img content is outside the region', function () { - var checkArgs = checkSetup( - '

Introduction

' - ); + var checkArgs = checkSetup(` + +
Content
+ `); 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 () { + var checkArgs = checkSetup(` + +
Content
+ `); + + assert.isTrue(checkEvaluate.apply(checkContext, checkArgs)); + }); + + it('should return true when img content outside of the region is decorative, via a presentation role', function () { + var checkArgs = checkSetup(` + Content + `); + + assert.isTrue(checkEvaluate.apply(checkContext, checkArgs)); + }); + it('should return true when textless text content is outside the region', function () { var checkArgs = checkSetup( '

Introduction

' From 7c810e2462f41a7181218c108a5d5f8c95e122ba Mon Sep 17 00:00:00 2001 From: Ava Gaiety W Date: Tue, 16 Apr 2024 13:15:47 -0500 Subject: [PATCH 2/9] fix(region): replaces tabs for spaces in prior commit my IDE was adding tabs ew, replaced with spaces Refs: #4145 --- test/checks/navigation/region.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/checks/navigation/region.js b/test/checks/navigation/region.js index ca19f35dac..9e6f1acd59 100644 --- a/test/checks/navigation/region.js +++ b/test/checks/navigation/region.js @@ -49,7 +49,7 @@ describe('region', function () { it('should return false when img content is outside the region', function () { var checkArgs = checkSetup(` -
Content
+
Content
`); assert.isFalse(checkEvaluate.apply(checkContext, checkArgs)); @@ -58,7 +58,7 @@ describe('region', function () { it('should return true when img content outside of the region is decorative, via an empty alt attr', function () { var checkArgs = checkSetup(` -
Content
+
Content
`); assert.isTrue(checkEvaluate.apply(checkContext, checkArgs)); @@ -67,7 +67,7 @@ describe('region', function () { it('should return true when img content outside of the region is decorative, via a presentation role', function () { var checkArgs = checkSetup(` Content +
Content
`); assert.isTrue(checkEvaluate.apply(checkContext, checkArgs)); From f914665083b658ccf7e4e3e3d7ad83e86f2979b5 Mon Sep 17 00:00:00 2001 From: Ava Gaiety W Date: Tue, 30 Apr 2024 13:31:39 -0500 Subject: [PATCH 3/9] add suggested isShallowlyHidden instead of presentational --- lib/checks/navigation/region-evaluate.js | 27 ++++++++++++++++---- test/checks/navigation/region.js | 32 ++++++++++++++++++------ 2 files changed, 47 insertions(+), 12 deletions(-) diff --git a/lib/checks/navigation/region-evaluate.js b/lib/checks/navigation/region-evaluate.js index c29e29b8a0..f42593a6b5 100644 --- a/lib/checks/navigation/region-evaluate.js +++ b/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'; @@ -56,8 +57,7 @@ function findRegionlessElms(virtualNode, options) { ['iframe', 'frame'].includes(virtualNode.props.nodeName) || (dom.isSkipLink(virtualNode.actualNode) && dom.getElementByReference(virtualNode.actualNode, 'href')) || - !dom.isVisibleToScreenReaders(node) || - isPresentationGraphic(virtualNode) + !dom.isVisibleToScreenReaders(node) ) { // Mark each parent node as having region descendant let vNode = virtualNode; @@ -78,7 +78,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) && + !isPresentational(node, virtualNode) ) { return [virtualNode]; @@ -91,8 +92,24 @@ function findRegionlessElms(virtualNode, options) { } } -function isPresentationGraphic(virtualNode) { - return virtualNode.props.nodeName === 'img' && virtualNode.attr('alt') === ''; +function isPresentational(node, virtualNode) { + return ( + isShallowlyHidden(virtualNode) && + !hasGlobalAriaAttr(virtualNode) && + !dom.isFocusable(node) + ); +} + +function hasGlobalAriaAttr(virtualNode) { + return standards.getGlobalAriaAttrs().some(attr => virtualNode.hasAttr(attr)); +} + +function isShallowlyHidden(virtualNode) { + // The element itself is not visible to screen readers, but its descendants might be + return ( + getRole(virtualNode, { noPresentational: true }) === null && + !hasChildTextNodes(virtualNode) + ); } // Check if the current element is a landmark diff --git a/test/checks/navigation/region.js b/test/checks/navigation/region.js index 9e6f1acd59..4a1df6db96 100644 --- a/test/checks/navigation/region.js +++ b/test/checks/navigation/region.js @@ -46,8 +46,8 @@ describe('region', function () { assert.isTrue(checkEvaluate.apply(checkContext, checkArgs)); }); - it('should return false when img content is outside the region', function () { - var checkArgs = checkSetup(` + it('should return false when img content is outside the region, no alt attribute at all', function () { + const checkArgs = checkSetup(`
Content
`); @@ -56,23 +56,41 @@ describe('region', function () { }); it('should return true when img content outside of the region is decorative, via an empty alt attr', function () { - var checkArgs = checkSetup(` - + const checkArgs = checkSetup(` +
Content
`); assert.isTrue(checkEvaluate.apply(checkContext, checkArgs)); }); - it('should return true when img content outside of the region is decorative, via a presentation role', function () { - var checkArgs = checkSetup(` -
Content
`); 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(` + +
Content
+ `); + + 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(` + +
Content
+ `); + + assert.isFalse(checkEvaluate.apply(checkContext, checkArgs)); + }); + it('should return true when textless text content is outside the region', function () { var checkArgs = checkSetup( '

Introduction

' From be633afc26fbf797add34f3ddbab7375a222b6a8 Mon Sep 17 00:00:00 2001 From: Ava Gaiety W Date: Tue, 30 Apr 2024 16:06:31 -0500 Subject: [PATCH 4/9] rely on getRole instead of adding hasGlobalAttr and focusable unecessarily --- lib/checks/navigation/region-evaluate.js | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/lib/checks/navigation/region-evaluate.js b/lib/checks/navigation/region-evaluate.js index f42593a6b5..3c0729840f 100644 --- a/lib/checks/navigation/region-evaluate.js +++ b/lib/checks/navigation/region-evaluate.js @@ -50,7 +50,6 @@ function findRegionlessElms(virtualNode, options) { // - a landmark // - a skiplink // - hidden content - // - a presentation graphic if ( getRole(virtualNode) === 'button' || isRegion(virtualNode, options) || @@ -79,7 +78,7 @@ function findRegionlessElms(virtualNode, options) { } else if ( node !== document.body && dom.hasContent(node, /* noRecursion: */ true) && - !isPresentational(node, virtualNode) + !isShallowlyHidden(virtualNode) ) { return [virtualNode]; @@ -92,18 +91,6 @@ function findRegionlessElms(virtualNode, options) { } } -function isPresentational(node, virtualNode) { - return ( - isShallowlyHidden(virtualNode) && - !hasGlobalAriaAttr(virtualNode) && - !dom.isFocusable(node) - ); -} - -function hasGlobalAriaAttr(virtualNode) { - return standards.getGlobalAriaAttrs().some(attr => virtualNode.hasAttr(attr)); -} - function isShallowlyHidden(virtualNode) { // The element itself is not visible to screen readers, but its descendants might be return ( From d51532937dd5bb08b65bf4eddc5b021446a653da Mon Sep 17 00:00:00 2001 From: Ava Gaiety W Date: Tue, 30 Apr 2024 16:16:01 -0500 Subject: [PATCH 5/9] additional test for overwritten role with nested child content --- test/checks/navigation/region.js | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/test/checks/navigation/region.js b/test/checks/navigation/region.js index 4a1df6db96..a1794d3b33 100644 --- a/test/checks/navigation/region.js +++ b/test/checks/navigation/region.js @@ -203,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(` +

Content

+
Content
+ `); + + assert.isFalse(checkEvaluate.apply(checkContext, checkArgs)); + }); + it('returns false for content outside of form tags with accessible names', function () { var checkArgs = checkSetup( '

Text

' From b29a90d6b4e5c58cfd35ed03777dccc67520d8c5 Mon Sep 17 00:00:00 2001 From: "Ava (Gaiety)" <165677673+gaiety-deque@users.noreply.github.com> Date: Thu, 2 May 2024 08:17:42 -0500 Subject: [PATCH 6/9] Update lib/checks/navigation/region-evaluate.js Co-authored-by: Wilco Fiers --- lib/checks/navigation/region-evaluate.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/checks/navigation/region-evaluate.js b/lib/checks/navigation/region-evaluate.js index 3c0729840f..0e20b472cb 100644 --- a/lib/checks/navigation/region-evaluate.js +++ b/lib/checks/navigation/region-evaluate.js @@ -94,7 +94,7 @@ 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 && + ['none', 'presentation'].includes(getRole(virtualNode)) && !hasChildTextNodes(virtualNode) ); } From 53782f2e765f072da36ede25d8843ed5f6a329db Mon Sep 17 00:00:00 2001 From: Ava Gaiety W Date: Thu, 2 May 2024 13:36:47 -0500 Subject: [PATCH 7/9] additional test coverage --- test/checks/navigation/region.js | 36 +++++++++++++++++++ test/integration/full/region/region-fail.html | 5 +++ test/integration/full/region/region-pass.html | 10 ++++++ 3 files changed, 51 insertions(+) diff --git a/test/checks/navigation/region.js b/test/checks/navigation/region.js index a1794d3b33..2620661d9d 100644 --- a/test/checks/navigation/region.js +++ b/test/checks/navigation/region.js @@ -91,6 +91,42 @@ describe('region', function () { assert.isFalse(checkEvaluate.apply(checkContext, checkArgs)); }); + it('should return true when canvas role=none', function () { + const checkArgs = checkSetup(` + +
Content
+ `); + + assert.isTrue(checkEvaluate.apply(checkContext, checkArgs)); + }); + + it('should return false when object has an aria-label', function () { + const checkArgs = checkSetup(` + +
Content
+ `); + + 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(` +
apples
+
Content
+ `); + + 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(` +
+
Content
+ `); + + assert.isTrue(checkEvaluate.apply(checkContext, checkArgs)); + }); + it('should return true when textless text content is outside the region', function () { var checkArgs = checkSetup( '

Introduction

' diff --git a/test/integration/full/region/region-fail.html b/test/integration/full/region/region-fail.html index 79fc58c0fb..10971298d7 100644 --- a/test/integration/full/region/region-fail.html +++ b/test/integration/full/region/region-fail.html @@ -23,6 +23,11 @@ This is not a skip link. + + + + +
apples

This is a header.

diff --git a/test/integration/full/region/region-pass.html b/test/integration/full/region/region-pass.html index 3c1a881117..b10ad2d69b 100644 --- a/test/integration/full/region/region-pass.html +++ b/test/integration/full/region/region-pass.html @@ -20,6 +20,16 @@ + + + + + + + +
+ +

This is a header.

From 8938496442ddc756666df5331cc5593c9dec67e4 Mon Sep 17 00:00:00 2001 From: Ava Gaiety W Date: Thu, 2 May 2024 14:47:54 -0500 Subject: [PATCH 8/9] targeted test failure cases --- test/integration/full/region/region-fail.html | 21 +++++++++--- test/integration/full/region/region-fail.js | 32 +++++++++++++++++-- 2 files changed, 46 insertions(+), 7 deletions(-) diff --git a/test/integration/full/region/region-fail.html b/test/integration/full/region/region-fail.html index 10971298d7..6f44a2eada 100644 --- a/test/integration/full/region/region-fail.html +++ b/test/integration/full/region/region-fail.html @@ -23,11 +23,6 @@ This is not a skip link. - - - - -
apples

This is a header.

@@ -45,6 +40,22 @@

This is a header.

+
+ +
+
+ +
+
+ +
+
+ +
+
+
apples
+
+ This should be ignored
diff --git a/test/integration/full/region/region-fail.js b/test/integration/full/region/region-fail.js index a1fd2952a6..09e7ae9906 100644 --- a/test/integration/full/region/region-fail.js +++ b/test/integration/full/region/region-fail.js @@ -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' + ]); + }); }); }); From 738e9e2b41e73e5be5d96c76d9a65666fd88ae73 Mon Sep 17 00:00:00 2001 From: "Ava (Gaiety)" <165677673+gaiety-deque@users.noreply.github.com> Date: Fri, 3 May 2024 08:04:14 -0500 Subject: [PATCH 9/9] Update test/integration/full/region/region-pass.html Co-authored-by: Wilco Fiers --- test/integration/full/region/region-pass.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/integration/full/region/region-pass.html b/test/integration/full/region/region-pass.html index b10ad2d69b..1e739bb390 100644 --- a/test/integration/full/region/region-pass.html +++ b/test/integration/full/region/region-pass.html @@ -21,9 +21,9 @@ - + - +