Normalize <select> focus events with other accessibility implementations

When a <select size=1> receives focus, a focus event should be fired on
the combobox/menupopupbutton object.

When the value changes in a collapsed state, only the value change event
should be fired, but not a focus or activedescendantchanged event.

When the value changes in an expanded state, then the focus and/or
activedescendantchanged events can be fired.

This fixes some strange behavior with NVDA when Alt+Down is used to open
the select.

Bug: 621510
Change-Id: I30fd744157ecbb5180d5c77f2713a73fa168397a
Reviewed-on: https://chromium-review.googlesource.com/1175034
Commit-Queue: Aaron Leventhal <aleventhal@chromium.org>
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Reviewed-by: Nektarios Paisios <nektar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583367}

content/browser/accessibility/browser_accessibility_manager.cc

@@ -500,11 +500,16 @@ BrowserAccessibility* BrowserAccessibilityManager::GetActiveDescendant(
 
   if (focus->GetRole() == ax::mojom::Role::kPopUpButton) {
     BrowserAccessibility* child = focus->InternalGetChild(0);
-    if (child && child->GetRole() == ax::mojom::Role::kMenuListPopup) {
+    if (child && child->GetRole() == ax::mojom::Role::kMenuListPopup &&
+        !child->GetData().HasState(ax::mojom::State::kInvisible)) {
       // The active descendant is found on the menu list popup, i.e. on the
       // actual list and not on the button that opens it.
       // If there is no active descendant, focus should stay on the button so
       // that Windows screen readers would enable their virtual cursor.
+      // Do not expose an activedescendant in a hidden/collapsed list, as
+      // screen readers expect the focus event to go to the button itself.
+      // Note that the AX hierarchy in this case is strange -- the active
+      // option is the only visible option, and is inside an invisible list.
       if (child->GetIntAttribute(ax::mojom::IntAttribute::kActivedescendantId,
                                  &active_descendant_id)) {
         active_descendant = child->manager()->GetFromID(active_descendant_id);
@@ -512,7 +517,8 @@ BrowserAccessibility* BrowserAccessibilityManager::GetActiveDescendant(
     }
   }
 
-  if (active_descendant)
+  if (active_descendant &&
+      !active_descendant->GetData().HasState(ax::mojom::State::kInvisible))
     return active_descendant;
 
   return focus;

content/browser/accessibility/dump_accessibility_events_browsertest.cc

@@ -359,6 +359,11 @@ IN_PROC_BROWSER_TEST_F(DumpAccessibilityEventsTest,
   RunEventTest(FILE_PATH_LITERAL("menulist-collapse.html"));
 }
 
+IN_PROC_BROWSER_TEST_F(DumpAccessibilityEventsTest,
+                       AccessibilityEventsMenuListCollapseNext) {
+  RunEventTest(FILE_PATH_LITERAL("menulist-collapse-next.html"));
+}
+
 // https://crbug.com/719030
 IN_PROC_BROWSER_TEST_F(DumpAccessibilityEventsTest,
                        DISABLED_AccessibilityEventsMenuListExpand) {

content/test/data/accessibility/event/menulist-collapse-expected-win.txt

-EVENT_OBJECT_STATECHANGE on <option> role=ROLE_SYSTEM_LISTITEM name="Apple" FOCUSED,INVISIBLE,FOCUSABLE,SELECTABLE 1 of 3
-EVENT_OBJECT_VALUECHANGE on <select> role=ROLE_SYSTEM_COMBOBOX COLLAPSED,FOCUSABLE,HASPOPUP
+EVENT_OBJECT_STATECHANGE on <option> role=ROLE_SYSTEM_LISTITEM name="Apple" INVISIBLE,FOCUSABLE,SELECTABLE 1 of 3
+EVENT_OBJECT_VALUECHANGE on <select> role=ROLE_SYSTEM_COMBOBOX FOCUSED,COLLAPSED,FOCUSABLE,HASPOPUP
\ No newline at end of file

content/test/data/accessibility/event/menulist-collapse-next-expected-mac.txt

+AXValueChanged on AXPopUpButton AXValue="Orange"

content/test/data/accessibility/event/menulist-collapse-next-expected-win.txt

+EVENT_OBJECT_SELECTION on <option> role=ROLE_SYSTEM_LISTITEM name="Orange" SELECTED,FOCUSABLE,SELECTABLE 2 of 3
+EVENT_OBJECT_STATECHANGE on <option> role=ROLE_SYSTEM_LISTITEM name="Apple" INVISIBLE,FOCUSABLE,SELECTABLE 1 of 3
+EVENT_OBJECT_STATECHANGE on <option> role=ROLE_SYSTEM_LISTITEM name="Orange" SELECTED,FOCUSABLE,SELECTABLE 2 of 3
+EVENT_OBJECT_VALUECHANGE on <select> role=ROLE_SYSTEM_COMBOBOX value="Orange" FOCUSED,COLLAPSED,FOCUSABLE,HASPOPUP
\ No newline at end of file

content/test/data/accessibility/event/menulist-collapse-next.html

+<!DOCTYPE html>
+<html>
+<body>
+<select>
+  <option selected>Apple</option>
+  <option>Orange</option>
+  <option>Banana</option>
+</select>
+<script>
+  document.querySelector('select').focus();
+  function go() {
+    document.querySelector('select').selectedIndex = 1;
+  }
+</script>
+</body>
+</html>

content/test/data/accessibility/event/menulist-focus-expected-mac.txt

-AXFocusedUIElementChanged on AXPopUpButton AXValue="Apple"
+AXFocusedUIElementChanged on AXPopUpButton AXValue="Apple"
\ No newline at end of file

content/test/data/accessibility/event/menulist-focus-expected-win.txt

-EVENT_OBJECT_FOCUS on <select> role=ROLE_SYSTEM_COMBOBOX value="Apple" FOCUSED,COLLAPSED,FOCUSABLE,HASPOPUP
+EVENT_OBJECT_FOCUS on <select> role=ROLE_SYSTEM_COMBOBOX value="Apple" FOCUSED,COLLAPSED,FOCUSABLE,HASPOPUP
\ No newline at end of file

ui/accessibility/ax_event_generator.cc

@@ -221,8 +221,12 @@ void AXEventGenerator::OnIntAttributeChanged(AXTree* tree,
 
   switch (attr) {
     case ax::mojom::IntAttribute::kActivedescendantId:
-      AddEvent(node, Event::ACTIVE_DESCENDANT_CHANGED);
-      active_descendant_changed_.push_back(node);
+      // Don't fire on invisible containers, as it confuses some screen readers,
+      // such as NVDA.
+      if (!node->data().HasState(ax::mojom::State::kInvisible)) {
+        AddEvent(node, Event::ACTIVE_DESCENDANT_CHANGED);
+        active_descendant_changed_.push_back(node);
+      }
       break;
     case ax::mojom::IntAttribute::kCheckedState:
       AddEvent(node, Event::CHECKED_STATE_CHANGED);