From 3acd229b08b806ea359e7e08e37e8721cddc5290 Mon Sep 17 00:00:00 2001 From: Wilco Fiers Date: Sun, 2 May 2021 17:31:27 +0200 Subject: [PATCH] fix(required-parent): Allow *item > group > *item nesting (#2898) * fix(required-parent): Allow *item > group > *item nesting * chore(required-parent): Exclude menuitem from exception * chore(required-parent): Add ownGroupRoles option --- doc/check-options.md | 24 +++++++ .../aria/aria-required-parent-evaluate.js | 18 ++++-- lib/checks/aria/aria-required-parent.json | 3 + lib/standards/aria-roles.js | 2 +- test/checks/aria/required-parent.js | 64 +++++++++++++++++-- .../aria-required-parent.html | 20 ++++++ .../aria-required-parent.json | 15 ++++- test/integration/rules/preprocessor.js | 6 +- 8 files changed, 140 insertions(+), 12 deletions(-) diff --git a/doc/check-options.md b/doc/check-options.md index cd67605590..f1e8a20989 100644 --- a/doc/check-options.md +++ b/doc/check-options.md @@ -7,6 +7,7 @@ - [Global Options](#global-options) - [aria-allowed-role](#aria-allowed-role) - [aria-required-children](#aria-required-children) + - [aria-required-parent](#aria-required-parent) - [aria-roledescription](#aria-roledescription) - [color-contrast](#color-contrast) - [page-has-heading-one](#page-has-heading-one) @@ -107,6 +108,29 @@ All checks allow these global options: +### aria-required-parent + + + + + + + + + + + + + + + + +
OptionDefaultDescription
+ ownGroupRoles + + 
['listitem', 'treeitem']
+ 		List of ARIA roles that when used in a group can have a grand parent with the same role. E.g. list > listitem > group > listitem.
+ ### aria-roledescription diff --git a/lib/checks/aria/aria-required-parent-evaluate.js b/lib/checks/aria/aria-required-parent-evaluate.js index 3efd2edf53..bbc8aa86bc 100644 --- a/lib/checks/aria/aria-required-parent-evaluate.js +++ b/lib/checks/aria/aria-required-parent-evaluate.js @@ -2,7 +2,7 @@ import { getExplicitRole, getRole, requiredContext } from '../../commons/aria'; import { getRootNode } from '../../commons/dom'; import { getNodeFromTree, escapeSelector } from '../../core/utils'; -function getMissingContext(virtualNode, reqContext, includeElement) { +function getMissingContext(virtualNode, ownGroupRoles, reqContext, includeElement) { const explicitRole = getExplicitRole(virtualNode); if (!reqContext) { @@ -20,6 +20,10 @@ function getMissingContext(virtualNode, reqContext, includeElement) { // if parent node has role=group and role=group is an allowed // context, check next parent if (reqContext.includes('group') && parentRole === 'group') { + // Allow the own role; i.e. tree > treeitem > group > treeitem + if (ownGroupRoles.includes(explicitRole)) { + reqContext.push(explicitRole); + } vNode = vNode.parent; continue; } @@ -82,18 +86,24 @@ function getAriaOwners(element) { * @return {Boolean} True if the element has a parent with a required role. False otherwise. */ function ariaRequiredParentEvaluate(node, options, virtualNode) { - var missingParents = getMissingContext(virtualNode); + const ownGroupRoles = ( + options && Array.isArray(options.ownGroupRoles) + ? options.ownGroupRoles + : [] + ); + let missingParents = getMissingContext(virtualNode, ownGroupRoles); if (!missingParents) { return true; } - var owners = getAriaOwners(node); + const owners = getAriaOwners(node); if (owners) { - for (var i = 0, l = owners.length; i < l; i++) { + for (let i = 0, l = owners.length; i < l; i++) { missingParents = getMissingContext( getNodeFromTree(owners[i]), + ownGroupRoles, missingParents, true ); diff --git a/lib/checks/aria/aria-required-parent.json b/lib/checks/aria/aria-required-parent.json index db2a21a72f..68449b346d 100644 --- a/lib/checks/aria/aria-required-parent.json +++ b/lib/checks/aria/aria-required-parent.json @@ -1,6 +1,9 @@ { "id": "aria-required-parent", "evaluate": "aria-required-parent-evaluate", + "options": { + "ownGroupRoles": ["listitem", "treeitem"] + }, "metadata": { "impact": "critical", "messages": { diff --git a/lib/standards/aria-roles.js b/lib/standards/aria-roles.js index 9d9185f2e2..733170dec6 100644 --- a/lib/standards/aria-roles.js +++ b/lib/standards/aria-roles.js @@ -297,7 +297,7 @@ const ariaRoles = { }, listitem: { type: 'structure', - requiredContext: ['list'], + requiredContext: ['list', 'group'], allowedAttrs: [ 'aria-level', 'aria-posinset', diff --git a/test/checks/aria/required-parent.js b/test/checks/aria/required-parent.js index 1c9b0d35be..9d041e7f6b 100644 --- a/test/checks/aria/required-parent.js +++ b/test/checks/aria/required-parent.js @@ -21,7 +21,7 @@ describe('aria-required-parent', function() { .getCheckEvaluate('aria-required-parent') .apply(checkContext, params) ); - assert.deepEqual(checkContext._data, ['list']); + assert.deepEqual(checkContext._data, ['list', 'group']); }); (shadowSupported ? it : xit)( @@ -44,7 +44,7 @@ describe('aria-required-parent', function() { .getCheckEvaluate('aria-required-parent') .apply(checkContext, params) ); - assert.deepEqual(checkContext._data, ['list']); + assert.deepEqual(checkContext._data, ['list', 'group']); } ); @@ -70,7 +70,7 @@ describe('aria-required-parent', function() { .getCheckEvaluate('aria-required-parent') .apply(checkContext, params) ); - assert.deepEqual(checkContext._data, ['list']); + assert.deepEqual(checkContext._data, ['list', 'group']); }); it('should pass when required parent is present in an aria-owns context', function() { @@ -152,7 +152,7 @@ describe('aria-required-parent', function() { it('should fail when intermediate node is role=group but this not an allowed context', function() { var params = checkSetup( - '

Nothing here.

' + '

Nothing here.

' ); assert.isFalse( axe.testUtils @@ -161,6 +161,62 @@ describe('aria-required-parent', function() { ); }); + describe('group with ownGroupRoles', function () { + it('should pass when the role and grand parent role is in ownGroupRoles', function() { + var params = checkSetup( + '
' + + '
' + + '
' + + '
' + + '
', { + ownGroupRoles: ['listitem'] + } + ); + + assert.isTrue( + axe.testUtils + .getCheckEvaluate('aria-required-parent') + .apply(checkContext, params) + ); + }); + + it('should fail when the role and grand parent role is in ownGroupRoles', function() { + var params = checkSetup( + '
' + + '
' + + '
' + + '
', { + ownGroupRoles: ['listitem'] + } + ); + + assert.isFalse( + axe.testUtils + .getCheckEvaluate('aria-required-parent') + .apply(checkContext, params) + ); + }); + + it('should fail when the role is not in a group', function () { + var params = checkSetup( + '
' + + '
' + + '
' + + '
' + + '
', { + ownGroupRoles: ['listitem'] + } + ); + + assert.isFalse( + axe.testUtils + .getCheckEvaluate('aria-required-parent') + .apply(checkContext, params) + ); + }) + }); + it('should pass when intermediate node is role=none', function() { var params = checkSetup( '

Nothing here.

' diff --git a/test/integration/rules/aria-required-parent/aria-required-parent.html b/test/integration/rules/aria-required-parent/aria-required-parent.html index 60f090f844..3c0d5588d1 100644 --- a/test/integration/rules/aria-required-parent/aria-required-parent.html +++ b/test/integration/rules/aria-required-parent/aria-required-parent.html @@ -38,3 +38,23 @@
Item 1
+ +
+
+
+
tree item
+
+
+
+ +
+
tree item
+
+ +
+ +
diff --git a/test/integration/rules/aria-required-parent/aria-required-parent.json b/test/integration/rules/aria-required-parent/aria-required-parent.json index dee6289ce1..df291caf49 100644 --- a/test/integration/rules/aria-required-parent/aria-required-parent.json +++ b/test/integration/rules/aria-required-parent/aria-required-parent.json @@ -1,7 +1,15 @@ { "description": "aria-required-parent test", "rule": "aria-required-parent", - "violations": [["#fail1"], ["#fail2"], ["#fail3"], ["#fail4"], ["#fail5"]], + "violations": [ + ["#fail1"], + ["#fail2"], + ["#fail3"], + ["#fail4"], + ["#fail5"], + ["#fail6"], + ["#fail7"] + ], "passes": [ ["#pass1"], ["#pass2"], @@ -11,6 +19,9 @@ ["#pass6"], ["#pass7"], ["#pass8"], - ["#pass9"] + ["#pass9"], + ["#pass10"], + ["#pass11"], + ["#pass12"] ] } diff --git a/test/integration/rules/preprocessor.js b/test/integration/rules/preprocessor.js index c4857b7635..82ea1bae1e 100644 --- a/test/integration/rules/preprocessor.js +++ b/test/integration/rules/preprocessor.js @@ -20,7 +20,11 @@ var createIntegrationPreprocessor = function(logger) { // and add the test data to it var htmlpath = file.originalPath.replace(extRegex, '.html'); var html = fs.readFileSync(htmlpath, 'utf-8'); - var test = JSON.parse(content); + try { + var test = JSON.parse(content); + } catch (e) { + throw new Error('Unable to parse content of ' + file.originalPath) + } test.content = html; var result = template.replace('{}; /*tests*/', JSON.stringify(test));