Bug 1192108: Fire focus events after mutation events but before any o…

…ther 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 @@
 
     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 @@
       ];
 
       if (aPrevItemId) {
-        this.eventSeq.unshift(
+        this.eventSeq.push(
           new stateChangeChecker(EXT_STATE_ACTIVE, true, false, aPrevItemId)
         );
       }
@@ -81,7 +82,7 @@
       ];
 
       if (aPrevItemId) {
-        this.eventSeq.unshift(
+        this.eventSeq.push(
           new stateChangeChecker(EXT_STATE_ACTIVE, true, false, aPrevItemId)
         );
       }
@@ -98,17 +99,19 @@
     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 @@
     */
     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 @@
       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 @@
       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);