From a19b6cb5252deb062f6170ab035d804742e7c1df Mon Sep 17 00:00:00 2001 From: Wilco Fiers Date: Fri, 12 May 2023 12:40:55 +0200 Subject: [PATCH] fix(aria-required-children): trigger reviewEmpty with hidden children (#4012) * fix(aria-required-children): trigger reviewEmpty with hidden children * :robot: Automated formatting fixes * Fix code block --- .../aria/aria-required-children-evaluate.js | 30 +- lib/standards/aria-roles.js | 14 +- test/checks/aria/required-children.js | 299 +++++------------- .../aria-required-children.html | 5 +- 4 files changed, 115 insertions(+), 233 deletions(-) diff --git a/lib/checks/aria/aria-required-children-evaluate.js b/lib/checks/aria/aria-required-children-evaluate.js index 940c824b62..beef5289af 100644 --- a/lib/checks/aria/aria-required-children-evaluate.js +++ b/lib/checks/aria/aria-required-children-evaluate.js @@ -7,7 +7,6 @@ import { import { getGlobalAriaAttrs } from '../../commons/standards'; import { hasContentVirtual, - idrefs, isFocusable, isVisibleToScreenReaders } from '../../commons/dom'; @@ -35,7 +34,7 @@ export default function ariaRequiredChildrenEvaluate( return true; } - const ownedRoles = getOwnedRoles(virtualNode, required); + const { ownedRoles, ownedElements } = getOwnedRoles(virtualNode, required); const unallowed = ownedRoles.filter(({ role }) => !required.includes(role)); if (unallowed.length) { @@ -65,12 +64,7 @@ export default function ariaRequiredChildrenEvaluate( this.data(missing); // Only review empty nodes when a node is both empty and does not have an aria-owns relationship - if ( - reviewEmpty.includes(role) && - !hasContentVirtual(virtualNode, false, true) && - !ownedRoles.length && - (!virtualNode.hasAttr('aria-owns') || !idrefs(node, 'aria-owns').length) - ) { + if (reviewEmpty.includes(role) && !ownedElements.some(isContent)) { return undefined; } @@ -82,7 +76,10 @@ export default function ariaRequiredChildrenEvaluate( */ function getOwnedRoles(virtualNode, required) { const ownedRoles = []; - const ownedElements = getOwnedVirtual(virtualNode); + const ownedElements = getOwnedVirtual(virtualNode).filter(vNode => { + return vNode.props.nodeType !== 1 || isVisibleToScreenReaders(vNode); + }); + for (let i = 0; i < ownedElements.length; i++) { const ownedElement = ownedElements[i]; if (ownedElement.props.nodeType !== 1) { @@ -100,7 +97,6 @@ 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)) @@ -115,7 +111,7 @@ function getOwnedRoles(virtualNode, required) { } } - return ownedRoles; + return { ownedRoles, ownedElements }; } /** @@ -171,3 +167,15 @@ function getUnallowedSelector(vNode, attr) { return nodeName; } + +/** + * Check if the node has content, or is itself content + * @param {VirtualNode} vNode + * @returns {Boolean} + */ +function isContent(vNode) { + if (vNode.props.nodeType === 3) { + return vNode.props.nodeValue.trim().length > 0; + } + return hasContentVirtual(vNode, false, true); +} diff --git a/lib/standards/aria-roles.js b/lib/standards/aria-roles.js index f26c9428d4..09d1b012b4 100644 --- a/lib/standards/aria-roles.js +++ b/lib/standards/aria-roles.js @@ -378,7 +378,12 @@ const ariaRoles = { type: 'widget', requiredContext: ['menu', 'menubar', 'group'], requiredAttrs: ['aria-checked'], - allowedAttrs: ['aria-expanded', 'aria-posinset', 'aria-readonly', 'aria-setsize'], + allowedAttrs: [ + 'aria-expanded', + 'aria-posinset', + 'aria-readonly', + 'aria-setsize' + ], superclassRole: ['checkbox', 'menuitem'], accessibleNameRequired: true, nameFromContent: true, @@ -388,7 +393,12 @@ const ariaRoles = { type: 'widget', requiredContext: ['menu', 'menubar', 'group'], requiredAttrs: ['aria-checked'], - allowedAttrs: ['aria-expanded', 'aria-posinset', 'aria-readonly', 'aria-setsize'], + allowedAttrs: [ + 'aria-expanded', + 'aria-posinset', + 'aria-readonly', + 'aria-setsize' + ], superclassRole: ['menuitemcheckbox', 'radio'], accessibleNameRequired: true, nameFromContent: true, diff --git a/test/checks/aria/required-children.js b/test/checks/aria/required-children.js index 47191efb09..1b7165b127 100644 --- a/test/checks/aria/required-children.js +++ b/test/checks/aria/required-children.js @@ -3,6 +3,9 @@ describe('aria-required-children', () => { const shadowSupported = axe.testUtils.shadowSupport.v1; const checkContext = axe.testUtils.MockCheckContext(); const checkSetup = axe.testUtils.checkSetup; + const requiredChildrenCheck = axe.testUtils.getCheckEvaluate( + 'aria-required-children' + ); afterEach(() => { fixture.innerHTML = ''; @@ -15,11 +18,7 @@ describe('aria-required-children', () => { '

Nothing here.

' ); - assert.isFalse( - axe.testUtils - .getCheckEvaluate('aria-required-children') - .apply(checkContext, params) - ); + assert.isFalse(requiredChildrenCheck.apply(checkContext, params)); assert.deepEqual(checkContext._data, ['listitem']); }); @@ -36,11 +35,7 @@ describe('aria-required-children', () => { const virtualTarget = axe.utils.getNodeFromTree(target); const params = [target, undefined, virtualTarget]; - assert.isFalse( - axe.testUtils - .getCheckEvaluate('aria-required-children') - .apply(checkContext, params) - ); + assert.isFalse(requiredChildrenCheck.apply(checkContext, params)); assert.deepEqual(checkContext._data, ['listitem']); } ); @@ -50,11 +45,7 @@ describe('aria-required-children', () => { '

Nothing here.

' ); - assert.isFalse( - axe.testUtils - .getCheckEvaluate('aria-required-children') - .apply(checkContext, params) - ); + assert.isFalse(requiredChildrenCheck.apply(checkContext, params)); assert.deepEqual(checkContext._data, ['rowgroup', 'row']); }); @@ -71,11 +62,7 @@ describe('aria-required-children', () => { const virtualTarget = axe.utils.getNodeFromTree(target); const params = [target, undefined, virtualTarget]; - assert.isFalse( - axe.testUtils - .getCheckEvaluate('aria-required-children') - .apply(checkContext, params) - ); + assert.isFalse(requiredChildrenCheck.apply(checkContext, params)); assert.deepEqual(checkContext._data, ['rowgroup', 'row']); } ); @@ -84,22 +71,14 @@ describe('aria-required-children', () => { const params = checkSetup( '' ); - assert.isTrue( - axe.testUtils - .getCheckEvaluate('aria-required-children') - .apply(checkContext, params) - ); + assert.isTrue(requiredChildrenCheck.apply(checkContext, params)); }); it('should return undefined when element is empty and is in reviewEmpty options', () => { const params = checkSetup('
', { reviewEmpty: ['list'] }); - assert.isUndefined( - axe.testUtils - .getCheckEvaluate('aria-required-children') - .apply(checkContext, params) - ); + assert.isUndefined(requiredChildrenCheck.apply(checkContext, params)); }); it('should return false when children do not have correct role and is in reviewEmpty options', () => { @@ -107,11 +86,7 @@ describe('aria-required-children', () => { '
', { reviewEmpty: ['list'] } ); - assert.isFalse( - axe.testUtils - .getCheckEvaluate('aria-required-children') - .apply(checkContext, params) - ); + assert.isFalse(requiredChildrenCheck.apply(checkContext, params)); }); it('should return false when owned children do not have correct role and is in reviewEmpty options', () => { @@ -119,22 +94,14 @@ describe('aria-required-children', () => { '
', { reviewEmpty: ['list'] } ); - assert.isFalse( - axe.testUtils - .getCheckEvaluate('aria-required-children') - .apply(checkContext, params) - ); + assert.isFalse(requiredChildrenCheck.apply(checkContext, params)); }); it('should fail when list does not have required children listitem', () => { const params = checkSetup( '
Item 1
' ); - assert.isFalse( - axe.testUtils - .getCheckEvaluate('aria-required-children') - .apply(checkContext, params) - ); + assert.isFalse(requiredChildrenCheck.apply(checkContext, params)); assert.deepEqual(checkContext._data, ['listitem']); }); @@ -143,11 +110,7 @@ describe('aria-required-children', () => { const params = checkSetup( '
List item 1
' ); - assert.isFalse( - axe.testUtils - .getCheckEvaluate('aria-required-children') - .apply(checkContext, params) - ); + assert.isFalse(requiredChildrenCheck.apply(checkContext, params)); const unallowed = axe.utils.querySelectorAll( axe._tree, @@ -164,11 +127,7 @@ describe('aria-required-children', () => { const params = checkSetup( '
List item 1
' ); - assert.isFalse( - axe.testUtils - .getCheckEvaluate('aria-required-children') - .apply(checkContext, params) - ); + assert.isFalse(requiredChildrenCheck.apply(checkContext, params)); const unallowed = axe.utils.querySelectorAll( axe._tree, @@ -185,11 +144,7 @@ describe('aria-required-children', () => { const params = checkSetup( '
List item 1
' ); - assert.isFalse( - axe.testUtils - .getCheckEvaluate('aria-required-children') - .apply(checkContext, params) - ); + assert.isFalse(requiredChildrenCheck.apply(checkContext, params)); const unallowed = axe.utils.querySelectorAll( axe._tree, @@ -210,11 +165,7 @@ describe('aria-required-children', () => {
`); - assert.isFalse( - axe.testUtils - .getCheckEvaluate('aria-required-children') - .apply(checkContext, params) - ); + assert.isFalse(requiredChildrenCheck.apply(checkContext, params)); assert.deepEqual(checkContext._data, { messageKey: 'unallowed', @@ -229,11 +180,7 @@ describe('aria-required-children', () => { '
List item 1
' + '' ); - assert.isTrue( - axe.testUtils - .getCheckEvaluate('aria-required-children') - .apply(checkContext, params) - ); + assert.isTrue(requiredChildrenCheck.apply(checkContext, params)); }); it('should pass when list has child with aria-hidden and is focusable', () => { @@ -243,22 +190,14 @@ describe('aria-required-children', () => { '
List item 1
' + '' ); - assert.isTrue( - axe.testUtils - .getCheckEvaluate('aria-required-children') - .apply(checkContext, params) - ); + assert.isTrue(requiredChildrenCheck.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( - axe.testUtils - .getCheckEvaluate('aria-required-children') - .apply(checkContext, params) - ); + assert.isFalse(requiredChildrenCheck.apply(checkContext, params)); assert.includeMembers(checkContext._data, ['cell']); }); @@ -267,198 +206,126 @@ describe('aria-required-children', () => { const params = checkSetup( '
Nothing here.
' ); - assert.isTrue( - axe.testUtils - .getCheckEvaluate('aria-required-children') - .apply(checkContext, params) - ); + assert.isTrue(requiredChildrenCheck.apply(checkContext, params)); }); it('should not break if aria-owns points to non-existent node', () => { const params = checkSetup( - '
' - ); - assert.isFalse( - axe.testUtils - .getCheckEvaluate('aria-required-children') - .apply(checkContext, params) + '
' ); + assert.isFalse(requiredChildrenCheck.apply(checkContext, params)); }); it('should pass one existing aria-owned child when one required', () => { const params = checkSetup( '

Nothing here.

' ); - assert.isTrue( - axe.testUtils - .getCheckEvaluate('aria-required-children') - .apply(checkContext, params) - ); + assert.isTrue(requiredChildrenCheck.apply(checkContext, params)); }); 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( - axe.testUtils - .getCheckEvaluate('aria-required-children') - .apply(checkContext, params) - ); + assert.isFalse(requiredChildrenCheck.apply(checkContext, params)); }); it('should pass one existing required child when one required (has explicit role of tab)', () => { const params = checkSetup( '' ); - assert.isTrue( - axe.testUtils - .getCheckEvaluate('aria-required-children') - .apply(checkContext, params) - ); + assert.isTrue(requiredChildrenCheck.apply(checkContext, params)); }); it('should pass required child roles (grid contains row, which contains cell)', () => { const params = checkSetup( '
Item 1
' ); - assert.isTrue( - axe.testUtils - .getCheckEvaluate('aria-required-children') - .apply(checkContext, params) - ); + assert.isTrue(requiredChildrenCheck.apply(checkContext, params)); }); it('should pass one existing required child when one required', () => { const params = checkSetup( '

Nothing here.

' ); - assert.isTrue( - axe.testUtils - .getCheckEvaluate('aria-required-children') - .apply(checkContext, params) - ); + assert.isTrue(requiredChildrenCheck.apply(checkContext, params)); }); it('should pass one existing required child when one required because of implicit role', () => { const params = checkSetup( '

Nothing here.

' ); - assert.isTrue( - axe.testUtils - .getCheckEvaluate('aria-required-children') - .apply(checkContext, params) - ); + assert.isTrue(requiredChildrenCheck.apply(checkContext, params)); }); it('should pass when a child with an implicit role is present', () => { const params = checkSetup( '
Nothing here.
' ); - assert.isTrue( - axe.testUtils - .getCheckEvaluate('aria-required-children') - .apply(checkContext, params) - ); + assert.isTrue(requiredChildrenCheck.apply(checkContext, params)); }); it('should pass direct existing required children', () => { const params = checkSetup( '

Nothing here.

' ); - assert.isTrue( - axe.testUtils - .getCheckEvaluate('aria-required-children') - .apply(checkContext, params) - ); + assert.isTrue(requiredChildrenCheck.apply(checkContext, params)); }); it('should pass indirect required children', () => { const params = checkSetup( '

Just a regular ol p that contains a...

Nothing here.

' ); - assert.isTrue( - axe.testUtils - .getCheckEvaluate('aria-required-children') - .apply(checkContext, params) - ); + assert.isTrue(requiredChildrenCheck.apply(checkContext, params)); }); it('should return true when a role has no required owned', () => { const params = checkSetup( '

Nothing here.

' ); - assert.isTrue( - axe.testUtils - .getCheckEvaluate('aria-required-children') - .apply(checkContext, params) - ); + assert.isTrue(requiredChildrenCheck.apply(checkContext, params)); }); it('should pass when role allows group and group has required child', () => { const params = checkSetup( '' ); - assert.isTrue( - axe.testUtils - .getCheckEvaluate('aria-required-children') - .apply(checkContext, params) - ); + assert.isTrue(requiredChildrenCheck.apply(checkContext, params)); }); it('should fail when role allows group and group does not have required child', () => { const params = checkSetup( '' ); - assert.isFalse( - axe.testUtils - .getCheckEvaluate('aria-required-children') - .apply(checkContext, params) - ); + assert.isFalse(requiredChildrenCheck.apply(checkContext, params)); }); it('should fail when role does not allow group', () => { const params = checkSetup( '
' ); - assert.isFalse( - axe.testUtils - .getCheckEvaluate('aria-required-children') - .apply(checkContext, params) - ); + assert.isFalse(requiredChildrenCheck.apply(checkContext, params)); }); it('should pass when role allows rowgroup and rowgroup has required child', () => { const params = checkSetup( '
' ); - assert.isTrue( - axe.testUtils - .getCheckEvaluate('aria-required-children') - .apply(checkContext, params) - ); + assert.isTrue(requiredChildrenCheck.apply(checkContext, params)); }); it('should fail when role allows rowgroup and rowgroup does not have required child', () => { const params = checkSetup( '
' ); - assert.isFalse( - axe.testUtils - .getCheckEvaluate('aria-required-children') - .apply(checkContext, params) - ); + assert.isFalse(requiredChildrenCheck.apply(checkContext, params)); }); it('should fail when role does not allow rowgroup', () => { const params = checkSetup( '
' ); - assert.isFalse( - axe.testUtils - .getCheckEvaluate('aria-required-children') - .apply(checkContext, params) - ); + assert.isFalse(requiredChildrenCheck.apply(checkContext, params)); }); describe('options', () => { @@ -466,21 +333,13 @@ describe('aria-required-children', () => { const params = checkSetup('
', { reviewEmpty: [] }); - assert.isFalse( - axe.testUtils - .getCheckEvaluate('aria-required-children') - .apply(checkContext, params) - ); + assert.isFalse(requiredChildrenCheck.apply(checkContext, params)); // Options: params[1] = { reviewEmpty: ['grid'] }; - assert.isUndefined( - axe.testUtils - .getCheckEvaluate('aria-required-children') - .apply(checkContext, params) - ); + assert.isUndefined(requiredChildrenCheck.apply(checkContext, params)); }); it('should not throw when options is incorrect', () => { @@ -488,27 +347,15 @@ describe('aria-required-children', () => { // Options: (incorrect) params[1] = ['menu']; - assert.isFalse( - axe.testUtils - .getCheckEvaluate('aria-required-children') - .apply(checkContext, params) - ); + assert.isFalse(requiredChildrenCheck.apply(checkContext, params)); // Options: (incorrect) params[1] = null; - assert.isFalse( - axe.testUtils - .getCheckEvaluate('aria-required-children') - .apply(checkContext, params) - ); + assert.isFalse(requiredChildrenCheck.apply(checkContext, params)); // Options: (incorrect) params[1] = 'menu'; - assert.isFalse( - axe.testUtils - .getCheckEvaluate('aria-required-children') - .apply(checkContext, params) - ); + assert.isFalse(requiredChildrenCheck.apply(checkContext, params)); }); it('should return undefined when the element has empty children', () => { @@ -518,11 +365,7 @@ describe('aria-required-children', () => { params[1] = { reviewEmpty: ['listbox'] }; - assert.isUndefined( - axe.testUtils - .getCheckEvaluate('aria-required-children') - .apply(checkContext, params) - ); + assert.isUndefined(requiredChildrenCheck.apply(checkContext, params)); }); it('should return false when the element has empty child with role', () => { @@ -532,11 +375,27 @@ describe('aria-required-children', () => { params[1] = { reviewEmpty: ['listbox'] }; - assert.isFalse( - axe.testUtils - .getCheckEvaluate('aria-required-children') - .apply(checkContext, params) + assert.isFalse(requiredChildrenCheck.apply(checkContext, params)); + }); + + it('should return undefined when there is a empty text node', () => { + const params = checkSetup( + '
  \n\t
' + ); + params[1] = { + reviewEmpty: ['listbox'] + }; + assert.isUndefined(requiredChildrenCheck.apply(checkContext, params)); + }); + + it('should return false when there is a non-empty text node', () => { + const params = checkSetup( + '
hello
' ); + params[1] = { + reviewEmpty: ['listbox'] + }; + assert.isFalse(requiredChildrenCheck.apply(checkContext, params)); }); it('should return undefined when the element has empty child with role=presentation', () => { @@ -546,11 +405,7 @@ describe('aria-required-children', () => { params[1] = { reviewEmpty: ['listbox'] }; - assert.isUndefined( - axe.testUtils - .getCheckEvaluate('aria-required-children') - .apply(checkContext, params) - ); + assert.isUndefined(requiredChildrenCheck.apply(checkContext, params)); }); it('should return undefined when the element has empty child with role=none', () => { @@ -560,11 +415,21 @@ describe('aria-required-children', () => { params[1] = { reviewEmpty: ['listbox'] }; - assert.isUndefined( - axe.testUtils - .getCheckEvaluate('aria-required-children') - .apply(checkContext, params) + assert.isUndefined(requiredChildrenCheck.apply(checkContext, params)); + }); + + it('should return undefined when the element has hidden children', () => { + const params = checkSetup( + `` ); + params[1] = { + reviewEmpty: ['menu'] + }; + assert.isUndefined(requiredChildrenCheck.apply(checkContext, params)); }); it('should return undefined when the element has empty child and aria-label', () => { @@ -574,11 +439,7 @@ describe('aria-required-children', () => { params[1] = { reviewEmpty: ['listbox'] }; - assert.isUndefined( - axe.testUtils - .getCheckEvaluate('aria-required-children') - .apply(checkContext, params) - ); + assert.isUndefined(requiredChildrenCheck.apply(checkContext, params)); }); }); }); 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 14a52d2d10..410434d972 100644 --- a/test/integration/rules/aria-required-children/aria-required-children.html +++ b/test/integration/rules/aria-required-children/aria-required-children.html @@ -24,7 +24,10 @@
- +