From 35a24c034520e3c6d95514e3b9d9f2ab6ca10e06 Mon Sep 17 00:00:00 2001 From: Adam Cutler Date: Mon, 6 Jan 2020 12:30:55 -0700 Subject: [PATCH] fix(aria-required-children): allow comboboxes with more popup roles (#1950) --- lib/checks/aria/required-children.js | 63 ++++++++++++++++----------- lib/commons/aria/index.js | 2 +- test/checks/aria/required-children.js | 40 +++++++++++++++++ 3 files changed, 78 insertions(+), 27 deletions(-) diff --git a/lib/checks/aria/required-children.js b/lib/checks/aria/required-children.js index 2151908d6e..42239c0e3d 100644 --- a/lib/checks/aria/required-children.js +++ b/lib/checks/aria/required-children.js @@ -10,8 +10,8 @@ function owns(node, virtualTree, role, ariaOwned) { if (node === null) { return false; } - var implicit = implicitNodes(role), - selector = ['[role="' + role + '"]']; + const implicit = implicitNodes(role); + let selector = ['[role="' + role + '"]']; if (implicit) { selector = selector.concat( @@ -27,14 +27,13 @@ function owns(node, virtualTree, role, ariaOwned) { } function ariaOwns(nodes, role) { - var index, length; - - for (index = 0, length = nodes.length; index < length; index++) { - if (nodes[index] === null) { + for (let index = 0; index < nodes.length; index++) { + const node = nodes[index]; + if (node === null) { continue; } - const virtualTree = axe.utils.getNodeFromTree(nodes[index]); - if (owns(nodes[index], virtualTree, role, true)) { + const virtualTree = axe.utils.getNodeFromTree(node); + if (owns(node, virtualTree, role, true)) { return true; } } @@ -42,13 +41,11 @@ function ariaOwns(nodes, role) { } function missingRequiredChildren(node, childRoles, all, role) { - var index, - length = childRoles.length, - missing = [], + const missing = [], ownedElements = idrefs(node, 'aria-owns'); - for (index = 0; index < length; index++) { - var childRole = childRoles[index]; + for (let index = 0; index < childRoles.length; index++) { + const childRole = childRoles[index]; if ( owns(node, virtualNode, childRole) || ariaOwns(ownedElements, childRole) @@ -66,8 +63,8 @@ function missingRequiredChildren(node, childRoles, all, role) { // combobox exceptions if (role === 'combobox') { // remove 'textbox' from missing roles if combobox is a native text-type input or owns a 'searchbox' - var textboxIndex = missing.indexOf('textbox'); - var textTypeInputs = ['text', 'search', 'email', 'url', 'tel']; + const textboxIndex = missing.indexOf('textbox'); + const textTypeInputs = ['text', 'search', 'email', 'url', 'tel']; if ( (textboxIndex >= 0 && (node.nodeName.toUpperCase() === 'INPUT' && @@ -78,11 +75,25 @@ function missingRequiredChildren(node, childRoles, all, role) { missing.splice(textboxIndex, 1); } - // remove 'listbox' from missing roles if combobox is collapsed - var listboxIndex = missing.indexOf('listbox'); - var expanded = node.getAttribute('aria-expanded'); - if (listboxIndex >= 0 && (!expanded || expanded === 'false')) { - missing.splice(listboxIndex, 1); + const expandedChildRoles = ['listbox', 'tree', 'grid', 'dialog']; + const expandedValue = node.getAttribute('aria-expanded'); + const expanded = expandedValue && expandedValue !== 'false'; + const popupRole = ( + node.getAttribute('aria-haspopup') || 'listbox' + ).toLowerCase(); + + for (let index = 0; index < expandedChildRoles.length; index++) { + const expandedChildRole = expandedChildRoles[index]; + // keep the specified popup type required if expanded + if (expanded && expandedChildRole === popupRole) { + continue; + } + + // remove 'listbox' and company from missing roles if combobox is collapsed + const missingIndex = missing.indexOf(expandedChildRole); + if (missingIndex >= 0) { + missing.splice(missingIndex, 1); + } } } @@ -108,21 +119,21 @@ function hasDecendantWithRole(node) { ); } -var role = node.getAttribute('role'); -var required = requiredOwned(role); +const role = node.getAttribute('role'); +const required = requiredOwned(role); if (!required) { return true; } -var all = false; -var childRoles = required.one; +let all = false; +let childRoles = required.one; if (!childRoles) { - var all = true; + all = true; childRoles = required.all; } -var missing = missingRequiredChildren(node, childRoles, all, role); +const missing = missingRequiredChildren(node, childRoles, all, role); if (!missing) { return true; diff --git a/lib/commons/aria/index.js b/lib/commons/aria/index.js index 46b47f4db8..9d4cacfb46 100644 --- a/lib/commons/aria/index.js +++ b/lib/commons/aria/index.js @@ -436,7 +436,7 @@ lookupTable.role = { required: ['aria-expanded'] }, owned: { - all: ['listbox', 'textbox'] + all: ['listbox', 'tree', 'grid', 'dialog', 'textbox'] }, nameFrom: ['author'], context: null, diff --git a/test/checks/aria/required-children.js b/test/checks/aria/required-children.js index e447590bd4..a7132dd693 100644 --- a/test/checks/aria/required-children.js +++ b/test/checks/aria/required-children.js @@ -206,6 +206,46 @@ describe('aria-required-children', function() { ); }); + it('should pass an expanded combobox when the required popup role matches', function() { + var params = checkSetup( + '

Textbox

' + ); + assert.isTrue( + checks['aria-required-children'].evaluate.apply(checkContext, params) + ); + }); + + it('should fail an expanded combobox when the required role is missing on children', function() { + var params = checkSetup( + '

Textbox

' + ); + assert.isFalse( + checks['aria-required-children'].evaluate.apply(checkContext, params) + ); + + assert.deepEqual(checkContext._data, ['grid']); + }); + + it('should pass an expanded combobox when the required popup role matches regarless of case', function() { + var params = checkSetup( + '

Textbox

' + ); + assert.isTrue( + checks['aria-required-children'].evaluate.apply(checkContext, params) + ); + }); + + it('should fail when combobox child isnt default listbox', function() { + var params = checkSetup( + '

Textbox

' + ); + assert.isFalse( + checks['aria-required-children'].evaluate.apply(checkContext, params) + ); + + assert.deepEqual(checkContext._data, ['listbox']); + }); + it('should pass one indirectly aria-owned child when one required', function() { var params = checkSetup( '
Nothing here.
'