Bug 1192108: Fire focus events after mutation events but before any other events. r=eeejay

It's critical that we fire mutation events first because our RemoteAccessible tree is created thus and we can't fire events on RemoteAccessibles we haven't created yet.
Beyond that, though, focus events are of primary importance.
See the comments in EventQueue::ProcessEventQueue for the reasons.

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

accessible/base/EventQueue.cpp

@@ -32,6 +32,11 @@ bool EventQueue::PushEvent(AccEvent* aEvent) {
                    aEvent->Document() == mDocument,
                "Queued event belongs to another document!");
 
+  if (aEvent->mEventType == nsIAccessibleEvent::EVENT_FOCUS) {
+    mFocusEvent = aEvent;
+    return true;
+  }
+
   // XXX(Bug 1631371) Check if this should use a fallible operation as it
   // pretended earlier, or change the return type to void.
   mEvents.AppendElement(aEvent);
@@ -302,28 +307,39 @@ void EventQueue::CoalesceSelChangeEvents(AccSelChangeEvent* aTailEvent,
 void EventQueue::ProcessEventQueue() {
   // Process only currently queued events.
   const nsTArray<RefPtr<AccEvent> > events = std::move(mEvents);
-
   uint32_t eventCount = events.Length();
 #ifdef A11Y_LOG
-  if (eventCount > 0 && logging::IsEnabled(logging::eEvents)) {
+  if ((eventCount > 0 || mFocusEvent) && logging::IsEnabled(logging::eEvents)) {
     logging::MsgBegin("EVENTS", "events processing");
     logging::Address("document", mDocument);
     logging::MsgEnd();
   }
 #endif
 
+  if (mFocusEvent) {
+    // Always fire a pending focus event before all other events. We do this for
+    // two reasons:
+    // 1. It prevents extraneous screen reader speech if the name, states, etc.
+    // of the currently focused item change at the same time as another item is
+    // focused. If aria-activedescendant is used, even setting
+    // aria-activedescendant before changing other properties results in the
+    // property change events being queued before the focus event because we
+    // process  aria-activedescendant async.
+    // 2. It improves perceived performance. Focus changes should be reported as
+    // soon as possible, so clients should handle focus events before they spend
+    // time on anything else.
+    RefPtr<AccEvent> event = std::move(mFocusEvent);
+    if (!event->mAccessible->IsDefunct()) {
+      FocusMgr()->ProcessFocusEvent(event);
+    }
+  }
+
   for (uint32_t idx = 0; idx < eventCount; idx++) {
     AccEvent* event = events[idx];
     if (event->mEventRule != AccEvent::eDoNotEmit) {
       LocalAccessible* target = event->GetAccessible();
       if (!target || target->IsDefunct()) continue;
 
-      // Dispatch the focus event if target is still focused.
-      if (event->mEventType == nsIAccessibleEvent::EVENT_FOCUS) {
-        FocusMgr()->ProcessFocusEvent(event);
-        continue;
-      }
-
       // Dispatch caret moved and text selection change events.
       if (event->mEventType ==
           nsIAccessibleEvent::EVENT_TEXT_SELECTION_CHANGED) {

accessible/base/EventQueue.h

@@ -68,6 +68,9 @@ class EventQueue {
    * SwapElements() on it.
    */
   nsTArray<RefPtr<AccEvent>> mEvents;
+
+  // Pending focus event.
+  RefPtr<AccEvent> mFocusEvent;
 };
 
 }  // namespace a11y

accessible/base/NotificationController.cpp

@@ -71,6 +71,7 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(NotificationController)
       cb.NoteXPCOMChild(list->ElementAt(i));
     }
   }
+  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mFocusEvent)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mEvents)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mRelocations)
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
@@ -103,6 +104,7 @@ void NotificationController::Shutdown() {
   mTextHash.Clear();
   mContentInsertions.Clear();
   mNotifications.Clear();
+  mFocusEvent = nullptr;
   mEvents.Clear();
   mRelocations.Clear();
   mEventTree.Clear();
@@ -976,7 +978,7 @@ void NotificationController::WillRefresh(mozilla::TimeStamp aTime) {
   // Stop further processing if there are no new notifications of any kind or
   // events and document load is processed.
   if (mContentInsertions.Count() == 0 && mNotifications.IsEmpty() &&
-      mEvents.IsEmpty() && mTextHash.Count() == 0 &&
+      !mFocusEvent && mEvents.IsEmpty() && mTextHash.Count() == 0 &&
       mHangingChildDocuments.IsEmpty() &&
       mDocument->HasLoadState(DocAccessible::eCompletelyLoaded) &&
       mPresShell->RemoveRefreshObserver(this, FlushType::Display)) {

accessible/tests/mochitest/events/docload/test_docload_root.html

@@ -37,7 +37,7 @@
       }
 
       this.eventSeq = [
-        new invokerChecker(EVENT_REORDER, gRootAcc),
+        new asyncInvokerChecker(EVENT_REORDER, gRootAcc),
         // We use a function here to get the target because gDialog isn't set
         // yet, but it will be when the function is called.
         new invokerChecker(EVENT_FOCUS, () => gDialog.document)

accessible/tests/mochitest/events/test_focus_aria_activedescendant.html

@@ -27,17 +27,18 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=429547
 
     function changeARIAActiveDescendant(aContainer, aItem, aPrevItemId) {
       let itemID = aItem instanceof Node ? aItem.id : aItem;
-      this.eventSeq = [
-        new stateChangeChecker(EXT_STATE_ACTIVE, true, true, aItem),
-        new focusChecker(aItem),
-      ];
+      this.eventSeq = [new focusChecker(aItem)];
 
       if (aPrevItemId) {
-        this.eventSeq.unshift(
+        this.eventSeq.push(
           new stateChangeChecker(EXT_STATE_ACTIVE, true, false, aPrevItemId)
         );
       }
 
+      this.eventSeq.push(
+        new stateChangeChecker(EXT_STATE_ACTIVE, true, true, aItem)
+      );
+
       this.invoke = function changeARIAActiveDescendant_invoke() {
         getNode(aContainer).setAttribute("aria-activedescendant", itemID);
       };
@@ -53,7 +54,7 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=429547
       ];
 
       if (aPrevItemId) {
-        this.eventSeq.unshift(
+        this.eventSeq.push(
           new stateChangeChecker(EXT_STATE_ACTIVE, true, false, aPrevItemId)
         );
       }
@@ -81,7 +82,7 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=429547
       ];
 
       if (aPrevItemId) {
-        this.eventSeq.unshift(
+        this.eventSeq.push(
           new stateChangeChecker(EXT_STATE_ACTIVE, true, false, aPrevItemId)
         );
       }
@@ -98,17 +99,19 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=429547
     function insertItemNFocus(aID, aNewItemID, aPrevItemId) {
       this.eventSeq = [
         new invokerChecker(EVENT_SHOW, aNewItemID),
-        new stateChangeChecker(EXT_STATE_ACTIVE, true, true, aNewItemID),
         new focusChecker(aNewItemID),
       ];
 
       if (aPrevItemId) {
-        this.eventSeq.splice(
-          1, 0,
+        this.eventSeq.push(
           new stateChangeChecker(EXT_STATE_ACTIVE, true, false, aPrevItemId)
         );
       }
 
+      this.eventSeq.push(
+        new stateChangeChecker(EXT_STATE_ACTIVE, true, true, aNewItemID)
+      );
+
       this.invoke = function insertItemNFocus_invoke() {
         var container  = getNode(aID);
 
@@ -138,8 +141,8 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=429547
     */
     function moveARIAActiveDescendantID(aFromID, aToID) {
       this.eventSeq = [
-        new stateChangeChecker(EXT_STATE_ACTIVE, true, true, aToID),
         new focusChecker(aToID),
+        new stateChangeChecker(EXT_STATE_ACTIVE, true, true, aToID),
       ];
 
       this.invoke = function moveARIAActiveDescendantID_invoke() {
@@ -215,6 +218,7 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=429547
       await evtProm;
       testStates("hiddenListOption", STATE_FOCUSED);
 
+      info("Testing active state changes when not focused");
       testStates("listbox", 0, 0, STATE_FOCUSED);
       evtProm = Promise.all([
         PromEvents.waitForStateChange("roaming3", EXT_STATE_ACTIVE, false, true),
@@ -223,6 +227,20 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=429547
       getNode("listbox").setAttribute("aria-activedescendant", "item1");
       await evtProm;
 
+      info("Testing that focus is always fired first");
+      const listbox = getNode("listbox");
+      evtProm = PromEvents.waitForEvent(EVENT_FOCUS, "item1");
+      listbox.focus();
+      await evtProm;
+      const item1 = getNode("item1");
+      evtProm = PromEvents.waitForOrderedEvents([
+        [EVENT_FOCUS, "item2"],
+        [EVENT_NAME_CHANGE, item1],
+      ], "Focus then name change");
+      item1.setAttribute("aria-label", "changed");
+      listbox.setAttribute("aria-activedescendant", "item2");
+      await evtProm;
+
       SimpleTest.finish();
     }
 

accessible/tests/mochitest/events/test_focus_autocomplete.html

@@ -22,7 +22,7 @@
         EVENT_FOCUS,
         evt => evt.accessible.role == ROLE_COMBOBOX_OPTION
       );
-      if (!event.accessible.name) {
+      while (!event.accessible.name) {
         // Sometimes, the name is null for a very short time after the focus
         // event.
         await waitForEvent(EVENT_NAME_CHANGE, event.accessible);

mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/AccessibilityTest.kt

@@ -385,6 +385,7 @@ class AccessibilityTest : BaseSessionTest() {
             }
         })
 
+        // This focuses the link.
         mainSession.finder.find("sweet", 0)
         sessionRule.waitUntilCalled(object : EventDelegate {
             @AssertCalled(count = 1)
@@ -397,6 +398,9 @@ class AccessibilityTest : BaseSessionTest() {
         // reset caret position
         mainSession.evaluateJS("""
             this.select(document.body, 0, 0);
+            // Changing DOM selection doesn't focus the document! Force focus
+            // here so we can use that to determine when this is done.
+            document.activeElement.blur();
         """.trimIndent())
         sessionRule.waitUntilCalled(object : EventDelegate {
             @AssertCalled(count = 1)