From 8714d6ba6debec93d095f5f12385d92c55b0d4b3 Mon Sep 17 00:00:00 2001 From: Steven Lambert <2433219+straker@users.noreply.github.com> Date: Wed, 22 Mar 2023 14:41:07 -0600 Subject: [PATCH] fix(aria-required-children): do not fail for children with aria-hidden (#3949) * fix(aria-required-children): do not fail for children with aria-hidden * es6 --- .../aria/aria-required-children-evaluate.js | 12 +- lib/commons/dom/index.js | 2 +- ...eader.js => is-visible-to-screenreader.js} | 0 lib/commons/text/accessible-text-virtual.js | 2 +- lib/commons/text/visible-virtual.js | 2 +- test/checks/aria/required-children.js | 217 ++++++++++-------- ...eader.js => is-visible-to-screenreader.js} | 0 .../aria-required-children.html | 5 + .../aria-required-children.json | 3 +- 9 files changed, 144 insertions(+), 99 deletions(-) rename lib/commons/dom/{is-visible-for-screenreader.js => is-visible-to-screenreader.js} (100%) rename test/commons/dom/{is-visible-for-screenreader.js => is-visible-to-screenreader.js} (100%) diff --git a/lib/checks/aria/aria-required-children-evaluate.js b/lib/checks/aria/aria-required-children-evaluate.js index 3ba1c2b757..205ef1c970 100644 --- a/lib/checks/aria/aria-required-children-evaluate.js +++ b/lib/checks/aria/aria-required-children-evaluate.js @@ -5,7 +5,12 @@ import { getOwnedVirtual } from '../../commons/aria'; import { getGlobalAriaAttrs } from '../../commons/standards'; -import { hasContentVirtual, idrefs, isFocusable } from '../../commons/dom'; +import { + hasContentVirtual, + idrefs, + isFocusable, + isVisibleToScreenReaders +} from '../../commons/dom'; /** * Get all owned roles of an element @@ -15,6 +20,10 @@ function getOwnedRoles(virtualNode, required) { const ownedElements = getOwnedVirtual(virtualNode); for (let i = 0; i < ownedElements.length; i++) { const ownedElement = ownedElements[i]; + if (ownedElement.props.nodeType !== 1) { + continue; + } + const role = getRole(ownedElement, { noPresentational: true }); const hasGlobalAria = getGlobalAriaAttrs().some(attr => @@ -27,6 +36,7 @@ function getOwnedRoles(virtualNode, required) { // this means intermediate roles between a required parent and // child will fail the check if ( + !isVisibleToScreenReaders(ownedElement) || (!role && !hasGlobalAriaOrFocusable) || (['group', 'rowgroup'].includes(role) && required.some(requiredRole => requiredRole === role)) diff --git a/lib/commons/dom/index.js b/lib/commons/dom/index.js index f1e6100161..9e5f16a741 100644 --- a/lib/commons/dom/index.js +++ b/lib/commons/dom/index.js @@ -40,7 +40,7 @@ export { default as isNode } from './is-node'; export { default as isOffscreen } from './is-offscreen'; export { default as isOpaque } from './is-opaque'; export { default as isSkipLink } from './is-skip-link'; -export { default as isVisibleToScreenReaders } from './is-visible-for-screenreader'; +export { default as isVisibleToScreenReaders } from './is-visible-to-screenreader'; export { default as isVisibleOnScreen } from './is-visible-on-screen'; export { default as isVisible } from './is-visible'; export { default as isVisualContent } from './is-visual-content'; diff --git a/lib/commons/dom/is-visible-for-screenreader.js b/lib/commons/dom/is-visible-to-screenreader.js similarity index 100% rename from lib/commons/dom/is-visible-for-screenreader.js rename to lib/commons/dom/is-visible-to-screenreader.js diff --git a/lib/commons/text/accessible-text-virtual.js b/lib/commons/text/accessible-text-virtual.js index 4acecb0c25..e47c259134 100644 --- a/lib/commons/text/accessible-text-virtual.js +++ b/lib/commons/text/accessible-text-virtual.js @@ -5,7 +5,7 @@ import formControlValue from './form-control-value'; import subtreeText from './subtree-text'; import titleText from './title-text'; import sanitize from './sanitize'; -import isVisibleToScreenReaders from '../dom/is-visible-for-screenreader'; +import isVisibleToScreenReaders from '../dom/is-visible-to-screenreader'; import isIconLigature from '../text/is-icon-ligature'; /** diff --git a/lib/commons/text/visible-virtual.js b/lib/commons/text/visible-virtual.js index 02942b6775..5862a84e94 100644 --- a/lib/commons/text/visible-virtual.js +++ b/lib/commons/text/visible-virtual.js @@ -1,6 +1,6 @@ import sanitize from './sanitize'; import isVisibleOnScreen from '../dom/is-visible-on-screen'; -import isVisibleToScreenReaders from '../dom/is-visible-for-screenreader'; +import isVisibleToScreenReaders from '../dom/is-visible-to-screenreader'; import AbstractVirtualNode from '../../core/base/virtual-node/abstract-virtual-node'; import { getNodeFromTree } from '../../core/utils'; diff --git a/test/checks/aria/required-children.js b/test/checks/aria/required-children.js index ab39ead842..34c5c312c4 100644 --- a/test/checks/aria/required-children.js +++ b/test/checks/aria/required-children.js @@ -1,19 +1,17 @@ -describe('aria-required-children', function () { - 'use strict'; +describe('aria-required-children', () => { + const fixture = document.getElementById('fixture'); + const shadowSupported = axe.testUtils.shadowSupport.v1; + const checkContext = axe.testUtils.MockCheckContext(); + const checkSetup = axe.testUtils.checkSetup; - var fixture = document.getElementById('fixture'); - var shadowSupported = axe.testUtils.shadowSupport.v1; - var checkContext = axe.testUtils.MockCheckContext(); - var checkSetup = axe.testUtils.checkSetup; - - afterEach(function () { + afterEach(() => { fixture.innerHTML = ''; axe._tree = undefined; checkContext.reset(); }); - it('should detect missing sole required child', function () { - var params = checkSetup( + it('should detect missing sole required child', () => { + const params = checkSetup( '

Nothing here.

' ); @@ -27,17 +25,17 @@ describe('aria-required-children', function () { (shadowSupported ? it : xit)( 'should detect missing sole required child in shadow tree', - function () { + () => { fixture.innerHTML = '
'; - var target = document.querySelector('#target'); - var shadowRoot = target.attachShadow({ mode: 'open' }); + const target = document.querySelector('#target'); + const shadowRoot = target.attachShadow({ mode: 'open' }); shadowRoot.innerHTML = '

Nothing here.

'; axe.testUtils.flatTreeSetup(fixture); - var virtualTarget = axe.utils.getNodeFromTree(target); + const virtualTarget = axe.utils.getNodeFromTree(target); - var params = [target, undefined, virtualTarget]; + const params = [target, undefined, virtualTarget]; assert.isFalse( axe.testUtils .getCheckEvaluate('aria-required-children') @@ -47,8 +45,8 @@ describe('aria-required-children', function () { } ); - it('should detect multiple missing required children when one required', function () { - var params = checkSetup( + it('should detect multiple missing required children when one required', () => { + const params = checkSetup( '

Nothing here.

' ); @@ -62,17 +60,17 @@ describe('aria-required-children', function () { (shadowSupported ? it : xit)( 'should detect missing multiple required children in shadow tree when one required', - function () { + () => { fixture.innerHTML = '
'; - var target = document.querySelector('#target'); - var shadowRoot = target.attachShadow({ mode: 'open' }); + const target = document.querySelector('#target'); + const shadowRoot = target.attachShadow({ mode: 'open' }); shadowRoot.innerHTML = '

Nothing here.

'; axe.testUtils.flatTreeSetup(fixture); - var virtualTarget = axe.utils.getNodeFromTree(target); + const virtualTarget = axe.utils.getNodeFromTree(target); - var params = [target, undefined, virtualTarget]; + const params = [target, undefined, virtualTarget]; assert.isFalse( axe.testUtils .getCheckEvaluate('aria-required-children') @@ -82,8 +80,8 @@ describe('aria-required-children', function () { } ); - it('should pass all existing required children when all required', function () { - var params = checkSetup( + it('should pass all existing required children when all required', () => { + const params = checkSetup( '' ); assert.isTrue( @@ -93,8 +91,8 @@ describe('aria-required-children', function () { ); }); - it('should return undefined when element is empty and is in reviewEmpty options', function () { - var params = checkSetup('
', { + it('should return undefined when element is empty and is in reviewEmpty options', () => { + const params = checkSetup('
', { reviewEmpty: ['list'] }); assert.isUndefined( @@ -104,8 +102,8 @@ describe('aria-required-children', function () { ); }); - it('should return false when children do not have correct role and is in reviewEmpty options', function () { - var params = checkSetup( + it('should return false when children do not have correct role and is in reviewEmpty options', () => { + const params = checkSetup( '
', { reviewEmpty: ['list'] } ); @@ -116,8 +114,8 @@ describe('aria-required-children', function () { ); }); - it('should return false when owned children do not have correct role and is in reviewEmpty options', function () { - var params = checkSetup( + it('should return false when owned children do not have correct role and is in reviewEmpty options', () => { + const params = checkSetup( '
', { reviewEmpty: ['list'] } ); @@ -128,8 +126,8 @@ describe('aria-required-children', function () { ); }); - it('should fail when list does not have required children listitem', function () { - var params = checkSetup( + it('should fail when list does not have required children listitem', () => { + const params = checkSetup( '
Item 1
' ); assert.isFalse( @@ -141,8 +139,8 @@ describe('aria-required-children', function () { assert.deepEqual(checkContext._data, ['listitem']); }); - it('should fail when list has intermediate child with role that is not a required role', function () { - var params = checkSetup( + it('should fail when list has intermediate child with role that is not a required role', () => { + const params = checkSetup( '
List item 1
' ); assert.isFalse( @@ -151,7 +149,7 @@ describe('aria-required-children', function () { .apply(checkContext, params) ); - var unallowed = axe.utils.querySelectorAll( + const unallowed = axe.utils.querySelectorAll( axe._tree, '[role="tabpanel"]' )[0]; @@ -159,8 +157,8 @@ describe('aria-required-children', function () { assert.deepEqual(checkContext._relatedNodes, [unallowed]); }); - it('should fail when list has child with global aria attribute but no role', function () { - var params = checkSetup( + it('should fail when list has child with global aria attribute but no role', () => { + const params = checkSetup( '
List item 1
' ); assert.isFalse( @@ -169,7 +167,7 @@ describe('aria-required-children', function () { .apply(checkContext, params) ); - var unallowed = axe.utils.querySelectorAll( + const unallowed = axe.utils.querySelectorAll( axe._tree, '[aria-live="polite"]' )[0]; @@ -177,8 +175,8 @@ describe('aria-required-children', function () { assert.deepEqual(checkContext._relatedNodes, [unallowed]); }); - it('should fail when list has child with tabindex but no role', function () { - var params = checkSetup( + it('should fail when list has child with tabindex but no role', () => { + const params = checkSetup( '
List item 1
' ); assert.isFalse( @@ -187,13 +185,44 @@ describe('aria-required-children', function () { .apply(checkContext, params) ); - var unallowed = axe.utils.querySelectorAll(axe._tree, '[tabindex="0"]')[0]; + const unallowed = axe.utils.querySelectorAll( + axe._tree, + '[tabindex="0"]' + )[0]; assert.deepEqual(checkContext._data, { messageKey: 'unallowed' }); assert.deepEqual(checkContext._relatedNodes, [unallowed]); }); - it('should fail when nested child with role row does not have required child role cell', function () { - var params = checkSetup( + it('should pass when list has child with aria-hidden', () => { + const params = checkSetup( + '
' + + '' + + '
List item 1
' + + '
' + ); + assert.isTrue( + axe.testUtils + .getCheckEvaluate('aria-required-children') + .apply(checkContext, params) + ); + }); + + it('should pass when list has child with aria-hidden and is focusable', () => { + const params = checkSetup( + '
' + + '' + + '
List item 1
' + + '
' + ); + assert.isTrue( + axe.testUtils + .getCheckEvaluate('aria-required-children') + .apply(checkContext, params) + ); + }); + + it('should fail when nested child with role row does not have required child role cell', () => { + const params = checkSetup( '
Item 1
' ); assert.isFalse( @@ -205,8 +234,8 @@ describe('aria-required-children', function () { assert.includeMembers(checkContext._data, ['cell']); }); - it('should pass one indirectly aria-owned child when one required', function () { - var params = checkSetup( + it('should pass one indirectly aria-owned child when one required', () => { + const params = checkSetup( '
Nothing here.
' ); assert.isTrue( @@ -216,8 +245,8 @@ describe('aria-required-children', function () { ); }); - it('should not break if aria-owns points to non-existent node', function () { - var params = checkSetup( + it('should not break if aria-owns points to non-existent node', () => { + const params = checkSetup( '
' ); assert.isFalse( @@ -227,8 +256,8 @@ describe('aria-required-children', function () { ); }); - it('should pass one existing aria-owned child when one required', function () { - var params = checkSetup( + it('should pass one existing aria-owned child when one required', () => { + const params = checkSetup( '

Nothing here.

' ); assert.isTrue( @@ -238,8 +267,8 @@ describe('aria-required-children', function () { ); }); - it('should fail one existing aria-owned child when an intermediate child with role that is not a required role exists', function () { - var params = checkSetup( + it('should fail one existing aria-owned child when an intermediate child with role that is not a required role exists', () => { + const params = checkSetup( '
' ); assert.isFalse( @@ -249,8 +278,8 @@ describe('aria-required-children', function () { ); }); - it('should pass one existing required child when one required (has explicit role of tab)', function () { - var params = checkSetup( + it('should pass one existing required child when one required (has explicit role of tab)', () => { + const params = checkSetup( '' ); assert.isTrue( @@ -260,8 +289,8 @@ describe('aria-required-children', function () { ); }); - it('should pass required child roles (grid contains row, which contains cell)', function () { - var params = checkSetup( + it('should pass required child roles (grid contains row, which contains cell)', () => { + const params = checkSetup( '
Item 1
' ); assert.isTrue( @@ -271,8 +300,8 @@ describe('aria-required-children', function () { ); }); - it('should pass one existing required child when one required', function () { - var params = checkSetup( + it('should pass one existing required child when one required', () => { + const params = checkSetup( '

Nothing here.

' ); assert.isTrue( @@ -282,8 +311,8 @@ describe('aria-required-children', function () { ); }); - it('should pass one existing required child when one required because of implicit role', function () { - var params = checkSetup( + it('should pass one existing required child when one required because of implicit role', () => { + const params = checkSetup( '

Nothing here.

' ); assert.isTrue( @@ -293,8 +322,8 @@ describe('aria-required-children', function () { ); }); - it('should pass when a child with an implicit role is present', function () { - var params = checkSetup( + it('should pass when a child with an implicit role is present', () => { + const params = checkSetup( '
Nothing here.
' ); assert.isTrue( @@ -304,8 +333,8 @@ describe('aria-required-children', function () { ); }); - it('should pass direct existing required children', function () { - var params = checkSetup( + it('should pass direct existing required children', () => { + const params = checkSetup( '

Nothing here.

' ); assert.isTrue( @@ -315,8 +344,8 @@ describe('aria-required-children', function () { ); }); - it('should pass indirect required children', function () { - var params = checkSetup( + it('should pass indirect required children', () => { + const params = checkSetup( '

Just a regular ol p that contains a...

Nothing here.

' ); assert.isTrue( @@ -326,8 +355,8 @@ describe('aria-required-children', function () { ); }); - it('should return true when a role has no required owned', function () { - var params = checkSetup( + it('should return true when a role has no required owned', () => { + const params = checkSetup( '

Nothing here.

' ); assert.isTrue( @@ -337,8 +366,8 @@ describe('aria-required-children', function () { ); }); - it('should pass when role allows group and group has required child', function () { - var params = checkSetup( + it('should pass when role allows group and group has required child', () => { + const params = checkSetup( '' ); assert.isTrue( @@ -348,8 +377,8 @@ describe('aria-required-children', function () { ); }); - it('should fail when role allows group and group does not have required child', function () { - var params = checkSetup( + it('should fail when role allows group and group does not have required child', () => { + const params = checkSetup( '' ); assert.isFalse( @@ -359,8 +388,8 @@ describe('aria-required-children', function () { ); }); - it('should fail when role does not allow group', function () { - var params = checkSetup( + it('should fail when role does not allow group', () => { + const params = checkSetup( '
' ); assert.isFalse( @@ -370,8 +399,8 @@ describe('aria-required-children', function () { ); }); - it('should pass when role allows rowgroup and rowgroup has required child', function () { - var params = checkSetup( + it('should pass when role allows rowgroup and rowgroup has required child', () => { + const params = checkSetup( '
' ); assert.isTrue( @@ -381,8 +410,8 @@ describe('aria-required-children', function () { ); }); - it('should fail when role allows rowgroup and rowgroup does not have required child', function () { - var params = checkSetup( + it('should fail when role allows rowgroup and rowgroup does not have required child', () => { + const params = checkSetup( '
' ); assert.isFalse( @@ -392,8 +421,8 @@ describe('aria-required-children', function () { ); }); - it('should fail when role does not allow rowgroup', function () { - var params = checkSetup( + it('should fail when role does not allow rowgroup', () => { + const params = checkSetup( '
' ); assert.isFalse( @@ -403,9 +432,9 @@ describe('aria-required-children', function () { ); }); - describe('options', function () { - it('should return undefined instead of false when the role is in options.reviewEmpty', function () { - var params = checkSetup('
', { + describe('options', () => { + it('should return undefined instead of false when the role is in options.reviewEmpty', () => { + const params = checkSetup('
', { reviewEmpty: [] }); assert.isFalse( @@ -425,8 +454,8 @@ describe('aria-required-children', function () { ); }); - it('should not throw when options is incorrect', function () { - var params = checkSetup('
'); + it('should not throw when options is incorrect', () => { + const params = checkSetup('
'); // Options: (incorrect) params[1] = ['menu']; @@ -453,8 +482,8 @@ describe('aria-required-children', function () { ); }); - it('should return undefined when the element has empty children', function () { - var params = checkSetup( + it('should return undefined when the element has empty children', () => { + const params = checkSetup( '
' ); params[1] = { @@ -467,8 +496,8 @@ describe('aria-required-children', function () { ); }); - it('should return false when the element has empty child with role', function () { - var params = checkSetup( + it('should return false when the element has empty child with role', () => { + const params = checkSetup( '
' ); params[1] = { @@ -481,8 +510,8 @@ describe('aria-required-children', function () { ); }); - it('should return undefined when the element has empty child with role=presentation', function () { - var params = checkSetup( + it('should return undefined when the element has empty child with role=presentation', () => { + const params = checkSetup( '
' ); params[1] = { @@ -495,8 +524,8 @@ describe('aria-required-children', function () { ); }); - it('should return undefined when the element has empty child with role=none', function () { - var params = checkSetup( + it('should return undefined when the element has empty child with role=none', () => { + const params = checkSetup( '
' ); params[1] = { @@ -509,8 +538,8 @@ describe('aria-required-children', function () { ); }); - it('should return undefined when the element has empty child and aria-label', function () { - var params = checkSetup( + it('should return undefined when the element has empty child and aria-label', () => { + const params = checkSetup( '
' ); params[1] = { diff --git a/test/commons/dom/is-visible-for-screenreader.js b/test/commons/dom/is-visible-to-screenreader.js similarity index 100% rename from test/commons/dom/is-visible-for-screenreader.js rename to test/commons/dom/is-visible-to-screenreader.js diff --git a/test/integration/rules/aria-required-children/aria-required-children.html b/test/integration/rules/aria-required-children/aria-required-children.html index 274527e339..14a52d2d10 100644 --- a/test/integration/rules/aria-required-children/aria-required-children.html +++ b/test/integration/rules/aria-required-children/aria-required-children.html @@ -135,3 +135,8 @@ Item 3 + + diff --git a/test/integration/rules/aria-required-children/aria-required-children.json b/test/integration/rules/aria-required-children/aria-required-children.json index c1d7f46d4b..23f2822f0a 100644 --- a/test/integration/rules/aria-required-children/aria-required-children.json +++ b/test/integration/rules/aria-required-children/aria-required-children.json @@ -32,7 +32,8 @@ ["#pass14"], ["#pass15"], ["#pass16"], - ["#pass17"] + ["#pass17"], + ["#pass18"] ], "incomplete": [ ["#incomplete1"],