From 0aca195721f11a20c20947e87b75b0254a6ec8f7 Mon Sep 17 00:00:00 2001 From: Steven Lambert Date: Wed, 10 Aug 2022 15:56:42 -0600 Subject: [PATCH 1/8] fix(aria-required-children): fail for child elements which are not listed as required --- .../aria/aria-required-children-evaluate.js | 31 +++-- lib/checks/aria/aria-required-children.json | 3 +- test/checks/aria/required-children.js | 118 ++++++++++++------ .../aria-required-children.html | 12 +- .../aria-required-children.json | 8 +- 5 files changed, 120 insertions(+), 52 deletions(-) diff --git a/lib/checks/aria/aria-required-children-evaluate.js b/lib/checks/aria/aria-required-children-evaluate.js index e4d2c845c9..55a5bef86a 100644 --- a/lib/checks/aria/aria-required-children-evaluate.js +++ b/lib/checks/aria/aria-required-children-evaluate.js @@ -4,7 +4,8 @@ import { getExplicitRole, getOwnedVirtual } from '../../commons/aria'; -import { hasContentVirtual, idrefs } from '../../commons/dom'; +import { getGlobalAriaAttrs } from '../../commons/standards'; +import { hasContentVirtual, idrefs, isFocusable } from '../../commons/dom'; /** * Get all owned roles of an element @@ -16,18 +17,24 @@ function getOwnedRoles(virtualNode, required) { const ownedElement = ownedElements[i]; const role = getRole(ownedElement, { noPresentational: true }); + const hasGlobalAria = getGlobalAriaAttrs().some(attr => + ownedElement.hasAttr(attr) + ); + const includedInAccessibilityTree = + hasGlobalAria || isFocusable(ownedElement); + // 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 || + (!role && !includedInAccessibilityTree) || (['group', 'rowgroup'].includes(role) && required.some(requiredRole => requiredRole === role)) ) { ownedElements.push(...ownedElement.children); - } else if (role) { - ownedRoles.push(role); + } else if (role || includedInAccessibilityTree) { + ownedRoles.push({ role, ownedElement }); } } @@ -39,10 +46,10 @@ function getOwnedRoles(virtualNode, required) { */ function missingRequiredChildren(virtualNode, role, required, ownedRoles) { for (let i = 0; i < ownedRoles.length; i++) { - var ownedRole = ownedRoles[i]; + const { role } = ownedRoles[i]; - if (required.includes(ownedRole)) { - required = required.filter(requiredRole => requiredRole !== ownedRole); + if (required.includes(role)) { + required = required.filter(requiredRole => requiredRole !== role); return null; } } @@ -74,6 +81,16 @@ function ariaRequiredChildrenEvaluate(node, options, virtualNode) { } const ownedRoles = getOwnedRoles(virtualNode, required); + const unallowed = ownedRoles.filter(({ role }) => !required.includes(role)); + + if (unallowed.length) { + this.relatedNodes(unallowed.map(({ ownedElement }) => ownedElement)); + this.data({ + messageKey: 'unallowed' + }); + return false; + } + const missing = missingRequiredChildren( virtualNode, role, diff --git a/lib/checks/aria/aria-required-children.json b/lib/checks/aria/aria-required-children.json index 6191d6361f..700bd39c71 100644 --- a/lib/checks/aria/aria-required-children.json +++ b/lib/checks/aria/aria-required-children.json @@ -21,7 +21,8 @@ "pass": "Required ARIA children are present", "fail": { "singular": "Required ARIA child role not present: ${data.values}", - "plural": "Required ARIA children role not present: ${data.values}" + "plural": "Required ARIA children role not present: ${data.values}", + "unallowed": "Element has children which are not allowed" }, "incomplete": { "singular": "Expecting ARIA child role to be added: ${data.values}", diff --git a/test/checks/aria/required-children.js b/test/checks/aria/required-children.js index c5fba9de19..126426370d 100644 --- a/test/checks/aria/required-children.js +++ b/test/checks/aria/required-children.js @@ -1,4 +1,4 @@ -describe('aria-required-children', function() { +describe('aria-required-children', function () { 'use strict'; var fixture = document.getElementById('fixture'); @@ -6,13 +6,13 @@ describe('aria-required-children', function() { var checkContext = axe.testUtils.MockCheckContext(); var checkSetup = axe.testUtils.checkSetup; - afterEach(function() { + afterEach(function () { fixture.innerHTML = ''; axe._tree = undefined; checkContext.reset(); }); - it('should detect missing sole required child', function() { + it('should detect missing sole required child', function () { var params = checkSetup( '

Nothing here.

' ); @@ -27,7 +27,7 @@ describe('aria-required-children', function() { (shadowSupported ? it : xit)( 'should detect missing sole required child in shadow tree', - function() { + function () { fixture.innerHTML = '
'; var target = document.querySelector('#target'); @@ -47,7 +47,7 @@ describe('aria-required-children', function() { } ); - it('should detect multiple missing required children when one required', function() { + it('should detect multiple missing required children when one required', function () { var params = checkSetup( '

Nothing here.

' ); @@ -62,7 +62,7 @@ describe('aria-required-children', function() { (shadowSupported ? it : xit)( 'should detect missing multiple required children in shadow tree when one required', - function() { + function () { fixture.innerHTML = '
'; var target = document.querySelector('#target'); @@ -82,7 +82,7 @@ describe('aria-required-children', function() { } ); - it('should pass all existing required children when all required', function() { + it('should pass all existing required children when all required', function () { var params = checkSetup( '' ); @@ -93,7 +93,7 @@ describe('aria-required-children', function() { ); }); - it('should return undefined when element is empty and is in reviewEmpty options', function() { + it('should return undefined when element is empty and is in reviewEmpty options', function () { var params = checkSetup('
', { reviewEmpty: ['list'] }); @@ -104,7 +104,7 @@ describe('aria-required-children', function() { ); }); - it('should return false when children do not have correct role and is in reviewEmpty options', function() { + it('should return false when children do not have correct role and is in reviewEmpty options', function () { var params = checkSetup( '
', { reviewEmpty: ['list'] } @@ -116,7 +116,7 @@ describe('aria-required-children', function() { ); }); - it('should return false when owned children do not have correct role and is in reviewEmpty options', function() { + it('should return false when owned children do not have correct role and is in reviewEmpty options', function () { var params = checkSetup( '
', { reviewEmpty: ['list'] } @@ -128,7 +128,7 @@ describe('aria-required-children', function() { ); }); - it('should fail when list does not have required children listitem', function() { + it('should fail when list does not have required children listitem', function () { var params = checkSetup( '
Item 1
' ); @@ -141,7 +141,7 @@ describe('aria-required-children', function() { assert.deepEqual(checkContext._data, ['group', 'listitem']); }); - it('should fail when list has intermediate child with role that is not a required role', function() { + it('should fail when list has intermediate child with role that is not a required role', function () { var params = checkSetup( '
List item 1
' ); @@ -151,10 +151,48 @@ describe('aria-required-children', function() { .apply(checkContext, params) ); - assert.deepEqual(checkContext._data, ['group', 'listitem']); + var unallowed = axe.utils.querySelectorAll( + axe._tree, + '[role="tabpanel"]' + )[0]; + assert.deepEqual(checkContext._data, { messageKey: 'unallowed' }); + assert.deepEqual(checkContext._relatedNodes, [unallowed]); + }); + + it('should fail when list has child with global aria attribute but no role', function () { + var params = checkSetup( + '
List item 1
' + ); + assert.isFalse( + axe.testUtils + .getCheckEvaluate('aria-required-children') + .apply(checkContext, params) + ); + + var unallowed = axe.utils.querySelectorAll( + axe._tree, + '[aria-live="polite"]' + )[0]; + assert.deepEqual(checkContext._data, { messageKey: 'unallowed' }); + assert.deepEqual(checkContext._relatedNodes, [unallowed]); + }); + + it('should fail when list has child with tabindex but no role', function () { + var params = checkSetup( + '
List item 1
' + ); + assert.isFalse( + axe.testUtils + .getCheckEvaluate('aria-required-children') + .apply(checkContext, params) + ); + + var 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() { + it('should fail when nested child with role row does not have required child role cell', function () { var params = checkSetup( '
Item 1
' ); @@ -167,7 +205,7 @@ describe('aria-required-children', function() { assert.includeMembers(checkContext._data, ['cell']); }); - it('should pass one indirectly aria-owned child when one required', function() { + it('should pass one indirectly aria-owned child when one required', function () { var params = checkSetup( '
Nothing here.
' ); @@ -178,7 +216,7 @@ describe('aria-required-children', function() { ); }); - it('should not break if aria-owns points to non-existent node', function() { + it('should not break if aria-owns points to non-existent node', function () { var params = checkSetup( '
' ); @@ -189,7 +227,7 @@ describe('aria-required-children', function() { ); }); - it('should pass one existing aria-owned child when one required', function() { + it('should pass one existing aria-owned child when one required', function () { var params = checkSetup( '

Nothing here.

' ); @@ -200,7 +238,7 @@ 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() { + 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( '
' ); @@ -211,7 +249,7 @@ describe('aria-required-children', function() { ); }); - it('should pass one existing required child when one required (has explicit role of tab)', function() { + it('should pass one existing required child when one required (has explicit role of tab)', function () { var params = checkSetup( '' ); @@ -222,7 +260,7 @@ describe('aria-required-children', function() { ); }); - it('should pass required child roles (grid contains row, which contains cell)', function() { + it('should pass required child roles (grid contains row, which contains cell)', function () { var params = checkSetup( '
Item 1
' ); @@ -233,7 +271,7 @@ describe('aria-required-children', function() { ); }); - it('should pass one existing required child when one required', function() { + it('should pass one existing required child when one required', function () { var params = checkSetup( '

Nothing here.

' ); @@ -244,7 +282,7 @@ describe('aria-required-children', function() { ); }); - it('should pass one existing required child when one required because of implicit role', function() { + it('should pass one existing required child when one required because of implicit role', function () { var params = checkSetup( '

Nothing here.

' ); @@ -255,7 +293,7 @@ describe('aria-required-children', function() { ); }); - it('should pass when a child with an implicit role is present', function() { + it('should pass when a child with an implicit role is present', function () { var params = checkSetup( '
Nothing here.
' ); @@ -266,7 +304,7 @@ describe('aria-required-children', function() { ); }); - it('should pass direct existing required children', function() { + it('should pass direct existing required children', function () { var params = checkSetup( '

Nothing here.

' ); @@ -277,7 +315,7 @@ describe('aria-required-children', function() { ); }); - it('should pass indirect required children', function() { + it('should pass indirect required children', function () { var params = checkSetup( '

Just a regular ol p that contains a...

Nothing here.

' ); @@ -288,7 +326,7 @@ describe('aria-required-children', function() { ); }); - it('should return true when a role has no required owned', function() { + it('should return true when a role has no required owned', function () { var params = checkSetup( '

Nothing here.

' ); @@ -299,7 +337,7 @@ describe('aria-required-children', function() { ); }); - it('should pass when role allows group and group has required child', function() { + it('should pass when role allows group and group has required child', function () { var params = checkSetup( '' ); @@ -310,7 +348,7 @@ describe('aria-required-children', function() { ); }); - it('should fail when role allows group and group does not have required child', function() { + it('should fail when role allows group and group does not have required child', function () { var params = checkSetup( '' ); @@ -321,7 +359,7 @@ describe('aria-required-children', function() { ); }); - it('should fail when role does not allow group', function() { + it('should fail when role does not allow group', function () { var params = checkSetup( '
' ); @@ -332,7 +370,7 @@ describe('aria-required-children', function() { ); }); - it('should pass when role allows rowgroup and rowgroup has required child', function() { + it('should pass when role allows rowgroup and rowgroup has required child', function () { var params = checkSetup( '
' ); @@ -343,7 +381,7 @@ describe('aria-required-children', function() { ); }); - it('should fail when role allows rowgroup and rowgroup does not have required child', function() { + it('should fail when role allows rowgroup and rowgroup does not have required child', function () { var params = checkSetup( '
' ); @@ -354,7 +392,7 @@ describe('aria-required-children', function() { ); }); - it('should fail when role does not allow rowgroup', function() { + it('should fail when role does not allow rowgroup', function () { var params = checkSetup( '
' ); @@ -365,8 +403,8 @@ describe('aria-required-children', function() { ); }); - describe('options', function() { - it('should return undefined instead of false when the role is in options.reviewEmpty', function() { + describe('options', function () { + it('should return undefined instead of false when the role is in options.reviewEmpty', function () { var params = checkSetup('
', { reviewEmpty: [] }); @@ -387,7 +425,7 @@ describe('aria-required-children', function() { ); }); - it('should not throw when options is incorrect', function() { + it('should not throw when options is incorrect', function () { var params = checkSetup(''); // Options: (incorrect) @@ -415,7 +453,7 @@ describe('aria-required-children', function() { ); }); - it('should return undefined when the element has empty children', function() { + it('should return undefined when the element has empty children', function () { var params = checkSetup( '
' ); @@ -429,7 +467,7 @@ describe('aria-required-children', function() { ); }); - it('should return false when the element has empty child with role', function() { + it('should return false when the element has empty child with role', function () { var params = checkSetup( '
' ); @@ -443,7 +481,7 @@ describe('aria-required-children', function() { ); }); - it('should return undefined when the element has empty child with role=presentation', function() { + it('should return undefined when the element has empty child with role=presentation', function () { var params = checkSetup( '
' ); @@ -457,7 +495,7 @@ describe('aria-required-children', function() { ); }); - it('should return undefined when the element has empty child with role=none', function() { + it('should return undefined when the element has empty child with role=none', function () { var params = checkSetup( '
' ); @@ -471,7 +509,7 @@ describe('aria-required-children', function() { ); }); - it('should return undefined when the element has empty child and aria-label', function() { + it('should return undefined when the element has empty child and aria-label', 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 8d0305a492..7ef6ee0507 100644 --- a/test/integration/rules/aria-required-children/aria-required-children.html +++ b/test/integration/rules/aria-required-children/aria-required-children.html @@ -76,8 +76,18 @@
+
+
  • Item 1
  • + Item 2 +
    +
    +
    +
    List item 1
    +
    List item 2
    +
    +
    -
    +
    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 c2f736160e..29732160a8 100644 --- a/test/integration/rules/aria-required-children/aria-required-children.json +++ b/test/integration/rules/aria-required-children/aria-required-children.json @@ -11,7 +11,10 @@ ["#fail7"], ["#fail8"], ["#fail9"], - ["#fail10"] + ["#fail10"], + ["#fail11"], + ["#fail12"], + ["#fail13"] ], "passes": [ ["#pass1"], @@ -37,7 +40,6 @@ ["#incomplete6"], ["#incomplete7"], ["#incomplete8"], - ["#incomplete9"], - ["#incomplete10"] + ["#incomplete9"] ] } From 8ea9f86401aef45d30f49ba0aa076145e33a733b Mon Sep 17 00:00:00 2001 From: Steven Lambert <2433219+straker@users.noreply.github.com> Date: Mon, 15 Aug 2022 10:43:21 -0600 Subject: [PATCH 2/8] Update lib/checks/aria/aria-required-children-evaluate.js --- lib/checks/aria/aria-required-children-evaluate.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/checks/aria/aria-required-children-evaluate.js b/lib/checks/aria/aria-required-children-evaluate.js index 55a5bef86a..ae85044b85 100644 --- a/lib/checks/aria/aria-required-children-evaluate.js +++ b/lib/checks/aria/aria-required-children-evaluate.js @@ -20,7 +20,7 @@ function getOwnedRoles(virtualNode, required) { const hasGlobalAria = getGlobalAriaAttrs().some(attr => ownedElement.hasAttr(attr) ); - const includedInAccessibilityTree = + const hasGlobalAriaOrFocusable = hasGlobalAria || isFocusable(ownedElement); // if owned node has no role or is presentational, or if role From 6449f1dfe21f123f3ac7d92c0133fd6f17679211 Mon Sep 17 00:00:00 2001 From: Steven Lambert <2433219+straker@users.noreply.github.com> Date: Mon, 15 Aug 2022 10:43:26 -0600 Subject: [PATCH 3/8] Update lib/checks/aria/aria-required-children-evaluate.js --- lib/checks/aria/aria-required-children-evaluate.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/checks/aria/aria-required-children-evaluate.js b/lib/checks/aria/aria-required-children-evaluate.js index ae85044b85..b66da12d31 100644 --- a/lib/checks/aria/aria-required-children-evaluate.js +++ b/lib/checks/aria/aria-required-children-evaluate.js @@ -28,7 +28,7 @@ function getOwnedRoles(virtualNode, required) { // this means intermediate roles between a required parent and // child will fail the check if ( - (!role && !includedInAccessibilityTree) || + (!role && ! hasGlobalAriaOrFocusable) || (['group', 'rowgroup'].includes(role) && required.some(requiredRole => requiredRole === role)) ) { From 281109c059ad573263d794a23d7208ec17bfe678 Mon Sep 17 00:00:00 2001 From: Steven Lambert <2433219+straker@users.noreply.github.com> Date: Mon, 15 Aug 2022 10:43:30 -0600 Subject: [PATCH 4/8] Update lib/checks/aria/aria-required-children-evaluate.js --- lib/checks/aria/aria-required-children-evaluate.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/checks/aria/aria-required-children-evaluate.js b/lib/checks/aria/aria-required-children-evaluate.js index b66da12d31..bebe639da2 100644 --- a/lib/checks/aria/aria-required-children-evaluate.js +++ b/lib/checks/aria/aria-required-children-evaluate.js @@ -33,7 +33,7 @@ function getOwnedRoles(virtualNode, required) { required.some(requiredRole => requiredRole === role)) ) { ownedElements.push(...ownedElement.children); - } else if (role || includedInAccessibilityTree) { + } else if (role || hasGlobalAriaOrFocusable) { ownedRoles.push({ role, ownedElement }); } } From 4fb7bae245816b130aedba88b6a0412f681bcd07 Mon Sep 17 00:00:00 2001 From: Steven Lambert Date: Mon, 15 Aug 2022 16:27:21 -0600 Subject: [PATCH 5/8] fix apg --- lib/standards/aria-roles.js | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/lib/standards/aria-roles.js b/lib/standards/aria-roles.js index f2f76fff3c..adbaaccfed 100644 --- a/lib/standards/aria-roles.js +++ b/lib/standards/aria-roles.js @@ -342,7 +342,14 @@ const ariaRoles = { }, menubar: { type: 'composite', - requiredOwned: ['group', 'menuitemradio', 'menuitem', 'menuitemcheckbox'], + // Note: spec difference (menu as required owned) + requiredOwned: [ + 'group', + 'menuitemradio', + 'menuitem', + 'menuitemcheckbox', + 'menu' + ], allowedAttrs: [ 'aria-activedescendant', 'aria-expanded', @@ -477,7 +484,7 @@ const ariaRoles = { }, radiogroup: { type: 'composite', - requiredOwned: ['radio'], + // Note: spec difference (no required owned) allowedAttrs: [ 'aria-readonly', 'aria-required', From 760dcd3dd1a91ec07cb71e94ddea8eba404c7079 Mon Sep 17 00:00:00 2001 From: Steven Lambert Date: Mon, 15 Aug 2022 16:30:49 -0600 Subject: [PATCH 6/8] update tests --- .../aria-required-children.html | 17 +++++++++++++++++ .../aria-required-children.json | 3 ++- 2 files changed, 19 insertions(+), 1 deletion(-) 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 7ef6ee0507..fc2f4595ff 100644 --- a/test/integration/rules/aria-required-children/aria-required-children.html +++ b/test/integration/rules/aria-required-children/aria-required-children.html @@ -72,6 +72,16 @@
    +
    + + + +
    Item 1
    +
    item 2
    +
    item 2
    +
    item 2
    +
    +
    @@ -93,3 +103,10 @@
    +
    +
    Heading
    +
      +
    • +
    • +
    +
    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 29732160a8..81df8bf5f7 100644 --- a/test/integration/rules/aria-required-children/aria-required-children.json +++ b/test/integration/rules/aria-required-children/aria-required-children.json @@ -29,7 +29,8 @@ ["#pass10"], ["#pass11"], ["#pass12"], - ["#pass13"] + ["#pass13"], + ["#pass14"] ], "incomplete": [ ["#incomplete1"], From 4610e42f80375c9c72621f3c74cb20bdb5d8df1c Mon Sep 17 00:00:00 2001 From: Steven Lambert Date: Mon, 15 Aug 2022 16:33:01 -0600 Subject: [PATCH 7/8] test --- .../aria-required-children/aria-required-children.html | 8 +++++++- .../aria-required-children/aria-required-children.json | 3 ++- 2 files changed, 9 insertions(+), 2 deletions(-) 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 fc2f4595ff..565cfce232 100644 --- a/test/integration/rules/aria-required-children/aria-required-children.html +++ b/test/integration/rules/aria-required-children/aria-required-children.html @@ -96,8 +96,14 @@
    List item 2
    -
    +
    +
    List item 1
    +
    List item 2
    +
    +
    + +
    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 81df8bf5f7..6af207cf7e 100644 --- a/test/integration/rules/aria-required-children/aria-required-children.json +++ b/test/integration/rules/aria-required-children/aria-required-children.json @@ -14,7 +14,8 @@ ["#fail10"], ["#fail11"], ["#fail12"], - ["#fail13"] + ["#fail13"], + ["#fail14"] ], "passes": [ ["#pass1"], From f72652dfbf2ab427890be38ab0f5d70228dcd4be Mon Sep 17 00:00:00 2001 From: Steven Lambert <2433219+straker@users.noreply.github.com> Date: Wed, 17 Aug 2022 09:18:41 -0600 Subject: [PATCH 8/8] Update lib/checks/aria/aria-required-children.json Co-authored-by: Wilco Fiers --- lib/checks/aria/aria-required-children.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/checks/aria/aria-required-children.json b/lib/checks/aria/aria-required-children.json index 700bd39c71..7f89cbeca6 100644 --- a/lib/checks/aria/aria-required-children.json +++ b/lib/checks/aria/aria-required-children.json @@ -22,7 +22,7 @@ "fail": { "singular": "Required ARIA child role not present: ${data.values}", "plural": "Required ARIA children role not present: ${data.values}", - "unallowed": "Element has children which are not allowed" + "unallowed": "Element has children which are not allowed (see related nodes)" }, "incomplete": { "singular": "Expecting ARIA child role to be added: ${data.values}",