From 5a264e48e800bb9a783b2597e723ec9529d6d798 Mon Sep 17 00:00:00 2001 From: Steven Lambert <2433219+straker@users.noreply.github.com> Date: Mon, 30 Nov 2020 08:27:33 -0700 Subject: [PATCH] fix(aria-required-children): allow group and rowgroup roles (#2661) * fix(aria-required-children): allow group and rowgroup roles * fix --- .../aria/aria-required-children-evaluate.js | 21 ++++-- lib/standards/aria-roles.js | 8 +- test/checks/aria/required-children.js | 74 ++++++++++++++++++- .../aria-required-children.html | 9 +++ .../aria-required-children.json | 7 +- 5 files changed, 102 insertions(+), 17 deletions(-) diff --git a/lib/checks/aria/aria-required-children-evaluate.js b/lib/checks/aria/aria-required-children-evaluate.js index 045c7b3a75..1b235a4b03 100644 --- a/lib/checks/aria/aria-required-children-evaluate.js +++ b/lib/checks/aria/aria-required-children-evaluate.js @@ -9,17 +9,22 @@ import { hasContentVirtual, idrefs } from '../../commons/dom'; /** * Get all owned roles of an element */ -function getOwnedRoles(virtualNode) { +function getOwnedRoles(virtualNode, required) { const ownedRoles = []; const ownedElements = getOwnedVirtual(virtualNode); for (let i = 0; i < ownedElements.length; i++) { let ownedElement = ownedElements[i]; - let role = getRole(ownedElement); + let role = getRole(ownedElement, { noPresentational: true }); - // if owned node has no role or is presentational we keep - // parsing the descendant tree. this means intermediate roles - // between a required parent and child will fail the check - if (['presentation', 'none', null].includes(role)) { + // if owned node has no role or is presentational, or if role + // allows group or rowgroup, we keep parsing the descendant tree. + // this means intermediate roles between a required parent and + // child will fail the check + if ( + !role || + (['group', 'rowgroup'].includes(role) && + required.some(requiredRole => requiredRole === role)) + ) { ownedElements.push(...ownedElement.children); } else if (role) { ownedRoles.push(role); @@ -99,11 +104,11 @@ function ariaRequiredChildrenEvaluate(node, options, virtualNode) { options && Array.isArray(options.reviewEmpty) ? options.reviewEmpty : []; const role = getExplicitRole(virtualNode, { dpub: true }); const required = requiredOwned(role); - if (!required) { + if (required === null) { return true; } - const ownedRoles = getOwnedRoles(virtualNode); + const ownedRoles = getOwnedRoles(virtualNode, required); const missing = missingRequiredChildren( virtualNode, role, diff --git a/lib/standards/aria-roles.js b/lib/standards/aria-roles.js index 456231b603..c67d47009a 100644 --- a/lib/standards/aria-roles.js +++ b/lib/standards/aria-roles.js @@ -269,7 +269,7 @@ const ariaRoles = { }, list: { type: 'structure', - requiredOwned: ['listitem'], + requiredOwned: ['group', 'listitem'], allowedAttrs: ['aria-expanded'], superclassRole: ['section'] }, @@ -322,7 +322,7 @@ const ariaRoles = { }, menu: { type: 'composite', - requiredOwned: ['menuitemradio', 'menuitem', 'menuitemcheckbox'], + requiredOwned: ['group', 'menuitemradio', 'menuitem', 'menuitemcheckbox'], allowedAttrs: [ 'aria-activedescendant', 'aria-expanded', @@ -332,7 +332,7 @@ const ariaRoles = { }, menubar: { type: 'composite', - requiredOwned: ['menuitemradio', 'menuitem', 'menuitemcheckbox'], + requiredOwned: ['group', 'menuitemradio', 'menuitem', 'menuitemcheckbox'], allowedAttrs: [ 'aria-activedescendant', 'aria-expanded', @@ -744,7 +744,7 @@ const ariaRoles = { }, tree: { type: 'composite', - requiredOwned: ['treeitem'], + requiredOwned: ['group', 'treeitem'], allowedAttrs: [ 'aria-multiselectable', 'aria-required', diff --git a/test/checks/aria/required-children.js b/test/checks/aria/required-children.js index aa16023982..617067610c 100644 --- a/test/checks/aria/required-children.js +++ b/test/checks/aria/required-children.js @@ -22,7 +22,7 @@ describe('aria-required-children', function() { .getCheckEvaluate('aria-required-children') .apply(checkContext, params) ); - assert.deepEqual(checkContext._data, ['listitem']); + assert.deepEqual(checkContext._data, ['group', 'listitem']); }); (shadowSupported ? it : xit)( @@ -43,7 +43,7 @@ describe('aria-required-children', function() { .getCheckEvaluate('aria-required-children') .apply(checkContext, params) ); - assert.deepEqual(checkContext._data, ['listitem']); + assert.deepEqual(checkContext._data, ['group', 'listitem']); } ); @@ -309,7 +309,7 @@ describe('aria-required-children', function() { .apply(checkContext, params) ); - assert.deepEqual(checkContext._data, ['listitem']); + assert.deepEqual(checkContext._data, ['group', 'listitem']); }); it('should fail when list has intermediate child with role that is not a required role', function() { @@ -322,7 +322,7 @@ describe('aria-required-children', function() { .apply(checkContext, params) ); - assert.deepEqual(checkContext._data, ['listitem']); + assert.deepEqual(checkContext._data, ['group', 'listitem']); }); it('should fail when nested child with role row does not have required child role cell', function() { @@ -492,6 +492,72 @@ describe('aria-required-children', function() { ); }); + it('should pass when role allows group and group has required child', function() { + var params = checkSetup( + '' + ); + assert.isTrue( + axe.testUtils + .getCheckEvaluate('aria-required-children') + .apply(checkContext, params) + ); + }); + + it('should fail when role allows group and group does not have required child', function() { + var params = checkSetup( + '' + ); + assert.isFalse( + axe.testUtils + .getCheckEvaluate('aria-required-children') + .apply(checkContext, params) + ); + }); + + it('should fail when role does not allow group', function() { + var params = checkSetup( + '
' + ); + assert.isFalse( + axe.testUtils + .getCheckEvaluate('aria-required-children') + .apply(checkContext, params) + ); + }); + + it('should pass when role allows rowgroup and rowgroup has required child', function() { + var params = checkSetup( + '
' + ); + assert.isTrue( + axe.testUtils + .getCheckEvaluate('aria-required-children') + .apply(checkContext, params) + ); + }); + + it('should fail when role allows rowgroup and rowgroup does not have required child', function() { + var params = checkSetup( + '
' + ); + assert.isFalse( + axe.testUtils + .getCheckEvaluate('aria-required-children') + .apply(checkContext, params) + ); + }); + + it('should fail when role does not allow rowgroup', function() { + var params = checkSetup( + '
' + ); + assert.isFalse( + axe.testUtils + .getCheckEvaluate('aria-required-children') + .apply(checkContext, params) + ); + }); + describe('options', function() { it('should return undefined instead of false when the role is in options.reviewEmpty', function() { var params = checkSetup('
', { 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 45247fcd9e..fafeae32ad 100644 --- a/test/integration/rules/aria-required-children/aria-required-children.html +++ b/test/integration/rules/aria-required-children/aria-required-children.html @@ -47,3 +47,12 @@ + + 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 1d55257d04..e77e8aa4e6 100644 --- a/test/integration/rules/aria-required-children/aria-required-children.json +++ b/test/integration/rules/aria-required-children/aria-required-children.json @@ -26,7 +26,12 @@ ["#pass13"], ["#pass14"], ["#pass15"], - ["#pass16"] + ["#pass16"], + ["#pass17"], + ["#pass18"], + ["#pass19"], + ["#pass20"], + ["#pass21"] ], "incomplete": [ ["#incomplete1"],