Fire UIA_SelectionItem_ElementRemovedFromSelectionEvent when unselected

Currently, when we select an item through SelectionItemProvider, Windows
Narrator announces "selected" but when we remove the selection, we do
not hear "non-selected". This is due to not firing
UIA_SelectionItem_ElementRemovedFromSelectionEvent when we unselect.

This CL adds firing UIA_SelectionItem_ElementRemovedFromSelectionEvent
for non-multiselected selection item, so we can properly notify Windows
Narrator of the unselect action.

Bug: 928949
Change-Id: Idbce6a2ecaaf3ff90231cf1a07d7a3bd06a23475
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2066591Reviewed-by: Ian Prest <iapres@microsoft.com>
Commit-Queue: Victor Fei <vicfei@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#745319}

content/browser/accessibility/browser_accessibility_manager_win.cc

@@ -686,25 +686,26 @@ void BrowserAccessibilityManagerWin::HandleSelectedStateChanged(
   const bool is_selected =
       node->GetBoolAttribute(ax::mojom::BoolAttribute::kSelected);
 
-  bool multiselect = false;
-  auto* selection_container = node->PlatformGetSelectionContainer();
-  if (selection_container &&
-      selection_container->HasState(ax::mojom::State::kMultiselectable))
-    multiselect = true;
-
-  if (multiselect) {
+  // Nodes that have selection container may support multiselect, for such nodes
+  // we add them to |selection_events_|, which FinalizeAccessibilityEvents
+  // handles selection item events firing.
+  // For nodes that do not have selection container, only single select is
+  // supported, selection item events firing are handled here.
+  if (auto* selection_container = node->PlatformGetSelectionContainer()) {
     if (is_selected) {
-      FireWinAccessibilityEvent(EVENT_OBJECT_SELECTIONADD, node);
-      if (::switches::IsExperimentalAccessibilityPlatformUIAEnabled())
       selection_events_[selection_container].added.push_back(node);
     } else {
-      FireWinAccessibilityEvent(EVENT_OBJECT_SELECTIONREMOVE, node);
-      if (::switches::IsExperimentalAccessibilityPlatformUIAEnabled())
       selection_events_[selection_container].removed.push_back(node);
     }
-  } else if (is_selected) {
+  } else {
+    if (is_selected) {
       FireWinAccessibilityEvent(EVENT_OBJECT_SELECTION, node);
       FireUiaAccessibilityEvent(UIA_SelectionItem_ElementSelectedEventId, node);
+    } else {
+      FireWinAccessibilityEvent(EVENT_OBJECT_SELECTIONREMOVE, node);
+      FireUiaAccessibilityEvent(
+          UIA_SelectionItem_ElementRemovedFromSelectionEventId, node);
+    }
   }
 }
 
@@ -730,17 +731,20 @@ void BrowserAccessibilityManagerWin::BeforeAccessibilityEvents() {
 void BrowserAccessibilityManagerWin::FinalizeAccessibilityEvents() {
   BrowserAccessibilityManager::FinalizeAccessibilityEvents();
 
+  // Finalize aria properties events.
   for (auto&& event_node : aria_properties_events_) {
     FireUiaPropertyChangedEvent(UIA_AriaPropertiesPropertyId, event_node);
   }
   aria_properties_events_.clear();
 
+  // Finalize text selection events.
   for (auto&& sel_event_node : text_selection_changed_events_) {
     FireUiaTextContainerEvent(UIA_Text_TextSelectionChangedEventId,
                               sel_event_node);
   }
   text_selection_changed_events_.clear();
 
+  // Finalize selection item events.
   for (auto&& selected : selection_events_) {
     auto* container = selected.first;
     auto&& changes = selected.second;
@@ -760,6 +764,7 @@ void BrowserAccessibilityManagerWin::FinalizeAccessibilityEvents() {
 
     if (selected_count == 1) {
       // Fire 'ElementSelected' on the only selected child
+      FireWinAccessibilityEvent(EVENT_OBJECT_SELECTION, first_selected_child);
       FireUiaAccessibilityEvent(UIA_SelectionItem_ElementSelectedEventId,
                                 first_selected_child);
     } else {
@@ -771,10 +776,18 @@ void BrowserAccessibilityManagerWin::FinalizeAccessibilityEvents() {
         FireUiaAccessibilityEvent(UIA_Selection_InvalidatedEventId, container);
       } else {
         for (auto* item : changes.added) {
+          if (container->HasState(ax::mojom::State::kMultiselectable)) {
+            FireWinAccessibilityEvent(EVENT_OBJECT_SELECTIONADD, item);
             FireUiaAccessibilityEvent(
                 UIA_SelectionItem_ElementAddedToSelectionEventId, item);
+          } else {
+            FireWinAccessibilityEvent(EVENT_OBJECT_SELECTION, item);
+            FireUiaAccessibilityEvent(UIA_SelectionItem_ElementSelectedEventId,
+                                      item);
+          }
         }
         for (auto* item : changes.removed) {
+          FireWinAccessibilityEvent(EVENT_OBJECT_SELECTIONREMOVE, item);
           FireUiaAccessibilityEvent(
               UIA_SelectionItem_ElementRemovedFromSelectionEventId, item);
         }

content/browser/accessibility/dump_accessibility_events_browsertest.cc

@@ -796,18 +796,8 @@ IN_PROC_BROWSER_TEST_P(DumpAccessibilityEventsTest,
   RunEventTest(FILE_PATH_LITERAL("visibility-hidden-changed.html"));
 }
 
-// Even with the deflaking in WaitForAccessibilityTreeToContainNodeWithName,
-// this test is still flaky on Windows.
-// TODO(aboxhall, dmazzoni, meredithl): re-enable with better fix for above.
-#if defined(OS_WIN)
-#define MAYBE_AccessibilityEventsAriaSelectedChanged \
-  DISABLED_AccessibilityEventsAriaSelectedChanged
-#else
-#define MAYBE_AccessibilityEventsAriaSelectedChanged \
-  AccessibilityEventsAriaSelectedChanged
-#endif
 IN_PROC_BROWSER_TEST_P(DumpAccessibilityEventsTest,
-                       MAYBE_AccessibilityEventsAriaSelectedChanged) {
+                       AccessibilityEventsAriaSelectedChanged) {
   RunEventTest(FILE_PATH_LITERAL("aria-selected-changed.html"));
 }
 

content/test/data/accessibility/event/aria-selected-changed-expected-uia-win.txt

+AriaProperties changed on role=gridcell, name=Grid1, GridCell1
+SelectionItem_ElementRemovedFromSelection on role=gridcell, name=Grid1, GridCell1
+=== Start Continuation ===
+AriaProperties changed on role=gridcell, name=Grid2, GridCell2
+SelectionItem_ElementAddedToSelection on role=gridcell, name=Grid2, GridCell2
+=== Start Continuation ===
+AriaProperties changed on role=gridcell, name=Grid3, GridCell1
+SelectionItem_ElementRemovedFromSelection on role=gridcell, name=Grid3, GridCell1
+=== Start Continuation ===
+AriaProperties changed on role=gridcell, name=Grid4, GridCell1
+AriaProperties changed on role=gridcell, name=Grid4, GridCell2
+SelectionItem_ElementRemovedFromSelection on role=gridcell, name=Grid4, GridCell1
+SelectionItem_ElementSelected on role=gridcell, name=Grid4, GridCell2
+=== Start Continuation ===
+=== Start Continuation ===
+AriaProperties changed on role=listitem, name=Select2, Option1
+AriaProperties changed on role=listitem, name=Select2, Option2
+SelectionItem_ElementSelected on role=listitem, name=Select2, Option2
+ValueValue changed on role=combobox
+=== Start Continuation ===
+AriaProperties changed on role=option, name=DivSelect, Option1
+AriaProperties changed on role=option, name=DivSelect, Option2
+SelectionItem_ElementSelected on role=option, name=DivSelect, Option2

content/test/data/accessibility/event/aria-selected-changed-expected-win.txt

@@ -6,10 +6,12 @@ EVENT_OBJECT_SELECTIONADD on <td#GridCell2> role=ROLE_SYSTEM_CELL name="Grid2, G
 EVENT_OBJECT_SELECTIONWITHIN on <table#Grid2> role=ROLE_SYSTEM_TABLE MULTISELECTABLE,EXTSELECTABLE
 EVENT_OBJECT_STATECHANGE on <td#GridCell2> role=ROLE_SYSTEM_CELL name="Grid2, GridCell2" SELECTED,FOCUSABLE,SELECTABLE
 === Start Continuation ===
+EVENT_OBJECT_SELECTIONREMOVE on <td#GridCell1> role=ROLE_SYSTEM_CELL name="Grid3, GridCell1" FOCUSABLE,SELECTABLE
 EVENT_OBJECT_SELECTIONWITHIN on <table#Grid3> role=ROLE_SYSTEM_TABLE
 EVENT_OBJECT_STATECHANGE on <td#GridCell1> role=ROLE_SYSTEM_CELL name="Grid3, GridCell1" FOCUSABLE,SELECTABLE
 === Start Continuation ===
 EVENT_OBJECT_SELECTION on <td#GridCell2> role=ROLE_SYSTEM_CELL name="Grid4, GridCell2" SELECTED,FOCUSABLE,SELECTABLE
+EVENT_OBJECT_SELECTIONREMOVE on <td#GridCell1> role=ROLE_SYSTEM_CELL name="Grid4, GridCell1" FOCUSABLE,SELECTABLE
 EVENT_OBJECT_SELECTIONWITHIN on <table#Grid4> role=ROLE_SYSTEM_TABLE
 EVENT_OBJECT_STATECHANGE on <td#GridCell1> role=ROLE_SYSTEM_CELL name="Grid4, GridCell1" FOCUSABLE,SELECTABLE
 EVENT_OBJECT_STATECHANGE on <td#GridCell2> role=ROLE_SYSTEM_CELL name="Grid4, GridCell2" SELECTED,FOCUSABLE,SELECTABLE
@@ -19,7 +21,7 @@ EVENT_OBJECT_SELECTION on <option#Option2> role=ROLE_SYSTEM_LISTITEM name="Selec
 EVENT_OBJECT_SELECTIONWITHIN on role=ROLE_SYSTEM_LIST INVISIBLE SetSize=2
 EVENT_OBJECT_STATECHANGE on <option#Option1> role=ROLE_SYSTEM_LISTITEM name="Select2, Option1" INVISIBLE,FOCUSABLE,SELECTABLE PosInSet=1 SetSize=2
 EVENT_OBJECT_STATECHANGE on <option#Option2> role=ROLE_SYSTEM_LISTITEM name="Select2, Option2" SELECTED,FOCUSABLE,SELECTABLE PosInSet=2 SetSize=2
-EVENT_OBJECT_VALUECHANGE on <select#Select2> role=ROLE_SYSTEM_COMBOBOX value="Select2, Option2" COLLAPSED,FOCUSABLE,HASPOPUP
+EVENT_OBJECT_VALUECHANGE on <select#Select2> role=ROLE_SYSTEM_COMBOBOX value="Select2, Option2" COLLAPSED,FOCUSABLE,HASPOPUP SetSize=2
 === Start Continuation ===
 EVENT_OBJECT_SELECTION on <div#Option2> role=ROLE_SYSTEM_LISTITEM name="DivSelect, Option2" SELECTED,FOCUSED,FOCUSABLE,SELECTABLE PosInSet=2 SetSize=2
 EVENT_OBJECT_SELECTIONWITHIN on <div#DivSelect> role=ROLE_SYSTEM_LIST IA2_STATE_VERTICAL SetSize=2

content/test/data/accessibility/event/aria-selected-changed.html

+<!--
+  @UIA-WIN-DENY:Text_TextChanged*
+  @UIA-WIN-DENY:Invoke_Invoked*
+-->
 <!DOCTYPE html>
 <table id='Grid1' role='grid' aria-multiselectable='true'><tbody>
   <tr>

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

 EVENT_OBJECT_FOCUS on <option#c> role=ROLE_SYSTEM_LISTITEM name="c" SELECTED,FOCUSED,FOCUSABLE,SELECTABLE PosInSet=3 SetSize=3
-EVENT_OBJECT_SELECTIONADD on <option#c> role=ROLE_SYSTEM_LISTITEM name="c" SELECTED,FOCUSED,FOCUSABLE,SELECTABLE PosInSet=3 SetSize=3
-EVENT_OBJECT_SELECTIONREMOVE on <option> role=ROLE_SYSTEM_LISTITEM name="b" FOCUSABLE,SELECTABLE PosInSet=2 SetSize=3
+EVENT_OBJECT_SELECTION on <option#c> role=ROLE_SYSTEM_LISTITEM name="c" SELECTED,FOCUSED,FOCUSABLE,SELECTABLE PosInSet=3 SetSize=3
 EVENT_OBJECT_SELECTIONWITHIN on <select> role=ROLE_SYSTEM_LIST FOCUSABLE,MULTISELECTABLE,EXTSELECTABLE IA2_STATE_VERTICAL SetSize=3
 EVENT_OBJECT_STATECHANGE on <option#c> role=ROLE_SYSTEM_LISTITEM name="c" SELECTED,FOCUSED,FOCUSABLE,SELECTABLE PosInSet=3 SetSize=3
 EVENT_OBJECT_STATECHANGE on <option> role=ROLE_SYSTEM_LISTITEM name="b" FOCUSABLE,SELECTABLE PosInSet=2 SetSize=3

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

 AriaProperties changed on role=listitem, name=Apple
+SelectionItem_ElementRemovedFromSelection on role=listitem, name=Apple
 ValueValue changed on role=combobox

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

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