Bug 1788143 - Rely on selection state to determine combo box value, r=Jamie

Previously, we were not firing active state change events for the options in
<select> elements when the drop-down was expanded. This leads to possibly-stale
cached 'active' state in the parent process, which can cause Firefox to report
incorrect combo box values based on stale state. Rather than fire more 'active'
state change events to fix the problem, this revision addresses the problem by
not firing 'active' state change events for combo boxes at all, and instead
relying on the selection state when determining the combo box value in the
parent process. This revision also adds a test to verify that the behavior is
as expected.

Differential Revision: https://phabricator.services.mozilla.com/D156627

accessible/html/HTMLSelectAccessible.cpp

@@ -221,23 +221,6 @@ nsRect HTMLSelectOptionAccessible::RelativeBounds(
   return HyperTextAccessibleWrap::RelativeBounds(aBoundingFrame);
 }
 
-nsresult HTMLSelectOptionAccessible::HandleAccEvent(AccEvent* aEvent) {
-  nsresult rv = HyperTextAccessibleWrap::HandleAccEvent(aEvent);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  AccStateChangeEvent* event = downcast_accEvent(aEvent);
-  if (event && (event->GetState() == states::SELECTED)) {
-    LocalAccessible* widget = ContainerWidget();
-    if (widget && !widget->AreItemsOperable()) {
-      // Collapsed options' ACTIVE state reflects their SELECT state.
-      nsEventShell::FireEvent(this, states::ACTIVE, event->IsStateEnabled(),
-                              true);
-    }
-  }
-
-  return NS_OK;
-}
-
 void HTMLSelectOptionAccessible::ActionNameAt(uint8_t aIndex,
                                               nsAString& aName) {
   if (aIndex == eAction_Select) aName.AssignLiteral("select");

accessible/html/HTMLSelectAccessible.h

@@ -70,8 +70,6 @@ class HTMLSelectOptionAccessible : public HyperTextAccessibleWrap {
   virtual nsRect RelativeBounds(nsIFrame** aBoundingFrame) const override;
   virtual void SetSelected(bool aSelect) override;
 
-  nsresult HandleAccEvent(AccEvent* aEvent) override;
-
   // ActionAccessible
   virtual bool HasPrimaryAction() const override;
   virtual void ActionNameAt(uint8_t aIndex, nsAString& aName) override;

accessible/ipc/RemoteAccessibleBase.cpp

@@ -264,15 +264,9 @@ void RemoteAccessibleBase<Derived>::Value(nsString& aValue) const {
     }
 
     if (IsCombobox()) {
-      Pivot p = Pivot(const_cast<RemoteAccessibleBase<Derived>*>(this));
-      PivotStateRule rule(states::ACTIVE);
-      Accessible* option = p.First(rule);
-      if (!option) {
-        option =
-            const_cast<RemoteAccessibleBase<Derived>*>(this)->GetSelectedItem(
-                0);
-      }
-
+      // For combo boxes, rely on selection state to determine the value.
+      const Accessible* option =
+          const_cast<RemoteAccessibleBase<Derived>*>(this)->GetSelectedItem(0);
       if (option) {
         option->Name(aValue);
       }

accessible/tests/browser/e10s/browser_caching_value.js

@@ -4,8 +4,12 @@
 
 "use strict";
 
+/* import-globals-from ../../mochitest/states.js */
 /* import-globals-from ../../mochitest/value.js */
-loadScripts({ name: "value.js", dir: MOCHITESTS_DIR });
+loadScripts(
+  { name: "states.js", dir: MOCHITESTS_DIR },
+  { name: "value.js", dir: MOCHITESTS_DIR }
+);
 
 /**
  * Test data has the format of:
@@ -246,3 +250,71 @@ addAccessibleTask(
   },
   { chrome: true, topLevel: true, iframe: true, remoteIframe: true }
 );
+
+/**
+ * Test caching of active state for select options - see bug 1788143.
+ */
+addAccessibleTask(
+  `
+  <select id="select">
+    <option id="first_option">First</option>
+    <option id="second_option">Second</option>
+  </select>`,
+  async function(browser, docAcc) {
+    const select = findAccessibleChildByID(docAcc, "select");
+    is(select.value, "First", "Select initial value correct");
+
+    // Focus the combo box.
+    await invokeFocus(browser, "select");
+
+    // Select the second option (drop-down collapsed).
+    let p = waitForEvents({
+      expected: [
+        [EVENT_SELECTION, "second_option"],
+        [EVENT_TEXT_VALUE_CHANGE, "select"],
+      ],
+      unexpected: [
+        stateChangeEventArgs("second_option", EXT_STATE_ACTIVE, true, true),
+        stateChangeEventArgs("first_option", EXT_STATE_ACTIVE, false, true),
+      ],
+    });
+    await invokeContentTask(browser, [], () => {
+      content.document.getElementById("select").selectedIndex = 1;
+    });
+    await p;
+
+    is(select.value, "Second", "Select value correct after changing option");
+
+    // Expand the combobox dropdown.
+    p = waitForEvent(EVENT_STATE_CHANGE, "ContentSelectDropdown");
+    EventUtils.synthesizeKey("VK_SPACE");
+    await p;
+
+    p = waitForEvents({
+      expected: [
+        [EVENT_SELECTION, "first_option"],
+        [EVENT_TEXT_VALUE_CHANGE, "select"],
+        [EVENT_HIDE, "ContentSelectDropdown"],
+      ],
+      unexpected: [
+        stateChangeEventArgs("first_option", EXT_STATE_ACTIVE, true, true),
+        stateChangeEventArgs("second_option", EXT_STATE_ACTIVE, false, true),
+      ],
+    });
+
+    // Press the up arrow to select the first option (drop-down expanded).
+    // Then, press Enter to confirm the selection and close the dropdown.
+    // We do both of these together to unify testing across platforms, since
+    // events are not entirely consistent on Windows vs. Linux + macOS.
+    EventUtils.synthesizeKey("VK_UP");
+    EventUtils.synthesizeKey("VK_RETURN");
+    await p;
+
+    is(
+      select.value,
+      "First",
+      "Select value correct after changing option back"
+    );
+  },
+  { chrome: true, topLevel: true, iframe: true, remoteIFrame: true }
+);

accessible/tests/mochitest/events/test_focus_selects.html

@@ -90,11 +90,11 @@
       await p;
 
       p = waitForEvents({
-        expected: [
-          [EVENT_SELECTION, "cb_apple"],
+        expected: [[EVENT_SELECTION, "cb_apple"]],
+        unexpected: [
+          [EVENT_FOCUS],
           stateChangeEventArgs("cb_apple", EXT_STATE_ACTIVE, true, true),
         ],
-        unexpected: [[EVENT_FOCUS]],
       });
       // collapsed combobox keeps a focus
       synthesizeKey("VK_DOWN");
@@ -120,14 +120,14 @@
       await p;
 
       p = waitForEvents({
-        expected: [
-          [EVENT_SELECTION, "cb_orange"],
+        expected: [[EVENT_SELECTION, "cb_orange"]],
+        unexpected: [
+          [EVENT_FOCUS],
           stateChangeEventArgs("cb_orange", EXT_STATE_ACTIVE, true, true),
         ],
-        unexpected: [[EVENT_FOCUS]],
       });
       // An unfocused selectable combobox gets selection change events,
-      // and active state change events, but not focus.
+      // but not focus events nor active state change events.
       getNode("cb_orange").selected = true;
       await p;