From 5ab026d1bc9932fdf6cd2b135df32dd80051b5c9 Mon Sep 17 00:00:00 2001
From: Cassey Lottman <clottman@users.noreply.github.com>
Date: Tue, 6 Jul 2021 10:20:52 -0500
Subject: [PATCH] fix(combobox): support aria 1.2 pattern for comboboxes
 (#3033)

* remove textbox from aria-required-children for combobox

* aria-required-attr changes for combobox

* address PR comments

* hardcode combobox owned roles

* fix another test

* use ariaAttrs.has-popup.values instead of hardcoded  list

* add comment

* add aria-owns attribute

* fix test
---
 .../aria/aria-required-attr-evaluate.js       |  15 +-
 .../aria/aria-required-children-evaluate.js   |  37 +---
 .../scrollable-region-focusable-matches.js    |   4 +-
 lib/standards/aria-roles.js                   |   8 +-
 test/checks/aria/aria-required-attr.js        |  48 +++++
 test/checks/aria/required-children.js         | 193 ------------------
 .../aria-required-attr/required-attr.html     |  17 +-
 .../aria-required-attr/required-attr.json     |   3 +-
 8 files changed, 83 insertions(+), 242 deletions(-)

diff --git a/lib/checks/aria/aria-required-attr-evaluate.js b/lib/checks/aria/aria-required-attr-evaluate.js
index 2d4a11b65c..fb9db7481e 100644
--- a/lib/checks/aria/aria-required-attr-evaluate.js
+++ b/lib/checks/aria/aria-required-attr-evaluate.js
@@ -29,9 +29,8 @@ import { uniqueArray } from '../../core/utils';
 function ariaRequiredAttrEvaluate(node, options = {}, virtualNode) {
   const missing = [];
   const attrs = virtualNode.attrNames;
-
+  const role = getExplicitRole(virtualNode);
   if (attrs.length) {
-    const role = getExplicitRole(virtualNode);
     let required = requiredAttr(role);
     const elmSpec = getElementSpec(virtualNode);
 
@@ -56,11 +55,21 @@ function ariaRequiredAttrEvaluate(node, options = {}, virtualNode) {
     }
   }
 
+  // aria 1.2 combobox requires aria-controls, but aria-owns is acceptable instead in earlier versions of the guidelines
+  // https://github.com/dequelabs/axe-core/issues/2505#issuecomment-788703942
+  if (
+    missing.length === 1 &&
+    role === 'combobox' &&
+    missing[0] === 'aria-controls' &&
+    virtualNode.attr('aria-owns')
+  ) {
+    return true;
+  }
+
   if (missing.length) {
     this.data(missing);
     return false;
   }
-
   return true;
 }
 
diff --git a/lib/checks/aria/aria-required-children-evaluate.js b/lib/checks/aria/aria-required-children-evaluate.js
index c6d5204263..e4d2c845c9 100644
--- a/lib/checks/aria/aria-required-children-evaluate.js
+++ b/lib/checks/aria/aria-required-children-evaluate.js
@@ -38,47 +38,12 @@ function getOwnedRoles(virtualNode, required) {
  * Get missing children roles
  */
 function missingRequiredChildren(virtualNode, role, required, ownedRoles) {
-  const isCombobox = role === 'combobox';
-
-  // combobox exceptions
-  if (isCombobox) {
-    // remove 'textbox' from missing roles if combobox is a native
-    // text-type input or owns a 'searchbox'
-    const textTypeInputs = ['text', 'search', 'email', 'url', 'tel'];
-    if (
-      (virtualNode.props.nodeName === 'input' &&
-        textTypeInputs.includes(virtualNode.props.type)) ||
-      ownedRoles.includes('searchbox')
-    ) {
-      required = required.filter(requiredRole => requiredRole !== 'textbox');
-    }
-
-    // combobox only needs one of [listbox, tree, grid, dialog] and
-    // only the type that matches the aria-popup value. remove
-    // all the other popup roles from the list of required
-    const expandedChildRoles = ['listbox', 'tree', 'grid', 'dialog'];
-    const expandedValue = virtualNode.attr('aria-expanded');
-    const expanded = expandedValue && expandedValue.toLowerCase() !== 'false';
-    const popupRole = (
-      virtualNode.attr('aria-haspopup') || 'listbox'
-    ).toLowerCase();
-    required = required.filter(
-      requiredRole =>
-        !expandedChildRoles.includes(requiredRole) ||
-        (expanded && requiredRole === popupRole)
-    );
-  }
-
   for (let i = 0; i < ownedRoles.length; i++) {
     var ownedRole = ownedRoles[i];
 
     if (required.includes(ownedRole)) {
       required = required.filter(requiredRole => requiredRole !== ownedRole);
-
-      // combobox requires all the roles not just any one of them
-      if (!isCombobox) {
-        return null;
-      }
+      return null;
     }
   }
 
diff --git a/lib/rules/scrollable-region-focusable-matches.js b/lib/rules/scrollable-region-focusable-matches.js
index b9f7732057..71acf09d24 100644
--- a/lib/rules/scrollable-region-focusable-matches.js
+++ b/lib/rules/scrollable-region-focusable-matches.js
@@ -1,6 +1,5 @@
 import { hasContentVirtual } from '../commons/dom';
 import { getExplicitRole } from '../commons/aria';
-import standards from '../standards';
 import {
   querySelectorAll,
   getScroll,
@@ -8,6 +7,7 @@ import {
   getRootNode,
   tokenList
 } from '../core/utils';
+import ariaAttrs from '../standards/aria-attrs';
 
 function scrollableRegionFocusableMatches(node, virtualNode) {
   /**
@@ -29,7 +29,7 @@ function scrollableRegionFocusableMatches(node, virtualNode) {
    * @see https://github.com/dequelabs/axe-core/issues/1763
    */
   const role = getExplicitRole(virtualNode);
-  if (standards.ariaRoles.combobox.requiredOwned.includes(role)) {
+  if (ariaAttrs['aria-haspopup'].values.includes(role)) {
     // in ARIA 1.1 the container has role=combobox
     if (closest(virtualNode, '[role~="combobox"]')) {
       return false;
diff --git a/lib/standards/aria-roles.js b/lib/standards/aria-roles.js
index ed74edf429..de372b3dd5 100644
--- a/lib/standards/aria-roles.js
+++ b/lib/standards/aria-roles.js
@@ -117,13 +117,9 @@ const ariaRoles = {
   },
   combobox: {
     type: 'composite',
-    requiredOwned: ['listbox', 'tree', 'grid', 'dialog', 'textbox'],
-    requiredAttrs: ['aria-expanded'],
-    // Note: because aria-controls is not well supported we will not
-    // make it a required attribute even though it is required in the
-    // spec
+    requiredAttrs: ['aria-expanded', 'aria-controls'],
     allowedAttrs: [
-      'aria-controls',
+      'aria-owns',
       'aria-autocomplete',
       'aria-readonly',
       'aria-required',
diff --git a/test/checks/aria/aria-required-attr.js b/test/checks/aria/aria-required-attr.js
index 2ab9e7a939..9474715053 100644
--- a/test/checks/aria/aria-required-attr.js
+++ b/test/checks/aria/aria-required-attr.js
@@ -57,6 +57,54 @@ describe('aria-required-attr', function() {
     );
   });
 
+  describe('combobox special case', function() {
+    it('should pass comboboxes that have aria-owns and aria-expanded', function() {
+      var vNode = queryFixture(
+        '<div id="target" role="combobox" aria-expanded="false" aria-owns="ownedchild"></div>'
+      );
+
+      assert.isTrue(
+        axe.testUtils
+          .getCheckEvaluate('aria-required-attr')
+          .call(checkContext, vNode.actualNode, options, vNode)
+      );
+    });
+
+    it('should pass comboboxes that have aria-controls and aria-expanded', function() {
+      var vNode = queryFixture(
+        '<div id="target" role="combobox" aria-expanded="false" aria-controls="test"></div>'
+      );
+
+      assert.isTrue(
+        axe.testUtils
+          .getCheckEvaluate('aria-required-attr')
+          .call(checkContext, vNode.actualNode, options, vNode)
+      );
+    });
+
+    it('should fail comboboxes that have aria-expanded only', function() {
+      var vNode = queryFixture(
+        '<div id="target" role="combobox" aria-expanded="false"></div>'
+      );
+
+      assert.isFalse(
+        axe.testUtils
+          .getCheckEvaluate('aria-required-attr')
+          .call(checkContext, vNode.actualNode, options, vNode)
+      );
+    });
+
+    it('should fail comboboxes that have no required attributes', function() {
+      var vNode = queryFixture('<div id="target" role="combobox"></div>');
+
+      assert.isFalse(
+        axe.testUtils
+          .getCheckEvaluate('aria-required-attr')
+          .call(checkContext, vNode.actualNode, options, vNode)
+      );
+    });
+  });
+
   describe('options', function() {
     it('should require provided attribute names for a role', function() {
       axe.configure({
diff --git a/test/checks/aria/required-children.js b/test/checks/aria/required-children.js
index 617067610c..38e1ccaae9 100644
--- a/test/checks/aria/required-children.js
+++ b/test/checks/aria/required-children.js
@@ -82,41 +82,6 @@ describe('aria-required-children', function() {
     }
   );
 
-  it('should detect multiple missing required children when all required', function() {
-    var params = checkSetup(
-      '<div role="combobox" id="target" aria-expanded="true"><p>Nothing here.</p></div>'
-    );
-    assert.isFalse(
-      axe.testUtils
-        .getCheckEvaluate('aria-required-children')
-        .apply(checkContext, params)
-    );
-    assert.deepEqual(checkContext._data, ['listbox', 'textbox']);
-  });
-
-  it('should detect single missing required child when all required', function() {
-    var params = checkSetup(
-      '<div role="combobox" id="target" aria-expanded="true"><p role="listbox">Nothing here.</p></div>'
-    );
-    assert.isFalse(
-      axe.testUtils
-        .getCheckEvaluate('aria-required-children')
-        .apply(checkContext, params)
-    );
-    assert.deepEqual(checkContext._data, ['textbox']);
-  });
-
-  it('should pass all existing required children when all required', function() {
-    var params = checkSetup(
-      '<div role="combobox" id="target"><p role="listbox">Nothing here.</p><p role="textbox">Textbox</p></div>'
-    );
-    assert.isTrue(
-      axe.testUtils
-        .getCheckEvaluate('aria-required-children')
-        .apply(checkContext, params)
-    );
-  });
-
   it('should pass all existing required children when all required', function() {
     var params = checkSetup(
       '<div id="target" role="menu"><li role="none"></li><li role="menuitem">Item 1</li><div role="menuitemradio">Item 2</div><div role="menuitemcheckbox">Item 3</div></div>'
@@ -163,142 +128,6 @@ describe('aria-required-children', function() {
     );
   });
 
-  (shadowSupported ? it : xit)(
-    'should pass all existing required children in shadow tree when all required',
-    function() {
-      fixture.innerHTML = '<div role="combobox" id="target"></div>';
-
-      var target = document.querySelector('#target');
-      var shadowRoot = target.attachShadow({ mode: 'open' });
-      shadowRoot.innerHTML =
-        '<p role="listbox">Nothing here.</p><p role="textbox">Textbox</p>';
-
-      axe.testUtils.flatTreeSetup(fixture);
-      var virtualTarget = axe.utils.getNodeFromTree(target);
-
-      var params = [target, undefined, virtualTarget];
-      assert.isTrue(
-        axe.testUtils
-          .getCheckEvaluate('aria-required-children')
-          .apply(checkContext, params)
-      );
-    }
-  );
-
-  it('should pass a native "text" type input with role comboxbox when missing child is role textbox', function() {
-    var params = checkSetup(
-      '<input type="text" role="combobox" aria-owns="listbox" id="target"><p role="listbox" id="listbox">Nothing here.</p>'
-    );
-    assert.isTrue(
-      axe.testUtils
-        .getCheckEvaluate('aria-required-children')
-        .apply(checkContext, params)
-    );
-  });
-
-  it('should pass a native "search" type input with role comboxbox when missing child is role textbox', function() {
-    var params = checkSetup(
-      '<input type="search" role="combobox" aria-owns="listbox1" id="target"><p role="listbox" id="listbox1">Nothing here.</p>'
-    );
-    assert.isTrue(
-      axe.testUtils
-        .getCheckEvaluate('aria-required-children')
-        .apply(checkContext, params)
-    );
-  });
-
-  it('should pass a native "email" type input with role comboxbox when missing child is role textbox', function() {
-    var params = checkSetup(
-      '<input type="email" role="combobox" aria-owns="listbox" id="target"><p role="listbox" id="listbox">Nothing here.</p>'
-    );
-    assert.isTrue(
-      axe.testUtils
-        .getCheckEvaluate('aria-required-children')
-        .apply(checkContext, params)
-    );
-  });
-
-  it('should pass a native "url" type input with role comboxbox when missing child is role textbox', function() {
-    var params = checkSetup(
-      '<input type="url" role="combobox" aria-owns="listbox" id="target"><p role="listbox" id="listbox">Nothing here.</p>'
-    );
-    assert.isTrue(
-      axe.testUtils
-        .getCheckEvaluate('aria-required-children')
-        .apply(checkContext, params)
-    );
-  });
-
-  it('should pass a native "tel" type input with role comboxbox when missing child is role textbox', function() {
-    var params = checkSetup(
-      '<input type="tel" role="combobox" aria-owns="listbox" id="target"><p role="listbox" id="listbox">Nothing here.</p>'
-    );
-    assert.isTrue(
-      axe.testUtils
-        .getCheckEvaluate('aria-required-children')
-        .apply(checkContext, params)
-    );
-  });
-
-  it('should pass a collapsed comboxbox when missing child is role listbox', function() {
-    var params = checkSetup(
-      '<div role="combobox" id="target"><p role="textbox">Textbox</p></div>'
-    );
-    assert.isTrue(
-      axe.testUtils
-        .getCheckEvaluate('aria-required-children')
-        .apply(checkContext, params)
-    );
-  });
-
-  it('should pass an expanded combobox when the required popup role matches', function() {
-    var params = checkSetup(
-      '<div role="combobox" aria-haspopup="grid" aria-expanded="true" id="target"><p role="textbox">Textbox</p><div role="grid"></div></div>'
-    );
-    assert.isTrue(
-      axe.testUtils
-        .getCheckEvaluate('aria-required-children')
-        .apply(checkContext, params)
-    );
-  });
-
-  it('should fail an expanded combobox when the required role is missing on children', function() {
-    var params = checkSetup(
-      '<div role="combobox" aria-haspopup="grid" aria-expanded="true" id="target"><p role="textbox">Textbox</p><div role="listbox"></div></div>'
-    );
-    assert.isFalse(
-      axe.testUtils
-        .getCheckEvaluate('aria-required-children')
-        .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(
-      '<div role="combobox" aria-haspopup="gRiD" aria-expanded="true" id="target"><p role="textbox">Textbox</p><div role="grid"></div></div>'
-    );
-    assert.isTrue(
-      axe.testUtils
-        .getCheckEvaluate('aria-required-children')
-        .apply(checkContext, params)
-    );
-  });
-
-  it('should fail when combobox child isnt default listbox', function() {
-    var params = checkSetup(
-      '<div role="combobox" aria-expanded="true" id="target"><p role="textbox">Textbox</p><div role="grid"></div></div>'
-    );
-    assert.isFalse(
-      axe.testUtils
-        .getCheckEvaluate('aria-required-children')
-        .apply(checkContext, params)
-    );
-
-    assert.deepEqual(checkContext._data, ['listbox']);
-  });
-
   it('should fail when list does not have required children listitem', function() {
     var params = checkSetup(
       '<div id="target" role="list"><span>Item 1</span></div>'
@@ -470,28 +299,6 @@ describe('aria-required-children', function() {
     );
   });
 
-  it('should pass role comboxbox when child is native "search" input type', function() {
-    var params = checkSetup(
-      '<div role="combobox" id="target"><input type="search"><p role="listbox">Textbox</p></div>'
-    );
-    assert.isTrue(
-      axe.testUtils
-        .getCheckEvaluate('aria-required-children')
-        .apply(checkContext, params)
-    );
-  });
-
-  it('should not accept implicit nodes with a different role', function() {
-    var params = checkSetup(
-      '<div role="combobox" id="target"><input type="search" role="spinbutton"><p role="listbox">Textbox</p></div>'
-    );
-    assert.isFalse(
-      axe.testUtils
-        .getCheckEvaluate('aria-required-children')
-        .apply(checkContext, params)
-    );
-  });
-
   it('should pass when role allows group and group has required child', function() {
     var params = checkSetup(
       '<div role="menu" id="target"><ul role="group"><li role="menuitem">Menuitem</li></ul></div>'
diff --git a/test/integration/rules/aria-required-attr/required-attr.html b/test/integration/rules/aria-required-attr/required-attr.html
index db11d68924..12d699b34b 100644
--- a/test/integration/rules/aria-required-attr/required-attr.html
+++ b/test/integration/rules/aria-required-attr/required-attr.html
@@ -28,7 +28,22 @@
   ok
 </div>
 <div role="heading" id="pass4" aria-level="1">ok</div>
-<div role="combobox" id="pass5" aria-expanded="true">ok</div>
+<div
+  role="combobox"
+  id="pass5"
+  aria-expanded="true"
+  aria-controls="controlsElement"
+>
+  ok
+</div>
+<div
+  role="combobox"
+  id="comboboxWithOwns"
+  aria-expanded="true"
+  aria-owns="ownsElement"
+>
+  ok
+</div>
 <span role="radio" id="pass6">I am BLUE!</span>
 <span role="radio" id="pass7" aria-checked="false">I am RED!</span>
 <span role="radio" id="pass8" aria-checked="true">I am GREEN!</span>
diff --git a/test/integration/rules/aria-required-attr/required-attr.json b/test/integration/rules/aria-required-attr/required-attr.json
index d6fcc345fa..bf44895d68 100644
--- a/test/integration/rules/aria-required-attr/required-attr.json
+++ b/test/integration/rules/aria-required-attr/required-attr.json
@@ -18,6 +18,7 @@
     ["#pass7"],
     ["#pass8"],
     ["#pass9"],
-    ["#pass10"]
+    ["#pass10"],
+    ["#comboboxWithOwns"]
   ]
 }