Bug 1571327: Process generic notifications after relocations to fix aria-activedescendant with simultaneous insertion and relocation. r=eeejay

Previously, if a hidden, aria-owned subtree was shown and aria-activedescendant was simultaneously targeted inside it, aria-activedescendant would fail.
This occurred because when we processed insertions, the presence of aria-owns meant we didn't create the subtree.
This meant that when we processed aria-activedescendant (which occurred before relocations), the active descendant didn't exist yet.
To fix this, we now process generic notifications (including aria-activedescendant) *after* relocations.

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

--HG--
extra : moz-landing-system : lando

accessible/base/NotificationController.cpp

@@ -802,18 +802,8 @@ void NotificationController::WillRefresh(mozilla::TimeStamp aTime) {
     }
   }
 
-  // Process only currently queued generic notifications.
-  nsTArray<RefPtr<Notification>> notifications;
-  notifications.SwapElements(mNotifications);
-
-  uint32_t notificationCount = notifications.Length();
-  for (uint32_t idx = 0; idx < notificationCount; idx++) {
-    notifications[idx]->Process();
-    if (!mDocument) return;
-  }
-
   // Process invalidation list of the document after all accessible tree
-  // modification are done.
+  // mutation is done.
   mDocument->ProcessInvalidationList();
 
   // Process relocation list.
@@ -827,6 +817,20 @@ void NotificationController::WillRefresh(mozilla::TimeStamp aTime) {
   }
   mRelocations.Clear();
 
+  // Process only currently queued generic notifications.
+  // These are used for processing aria-activedescendant, DOMMenuItemActive,
+  // etc. Therefore, they must be processed after relocations, since relocated
+  // subtrees might not have been created before relocation processing and the
+  // target might be inside a relocated subtree.
+  nsTArray<RefPtr<Notification>> notifications;
+  notifications.SwapElements(mNotifications);
+
+  uint32_t notificationCount = notifications.Length();
+  for (uint32_t idx = 0; idx < notificationCount; idx++) {
+    notifications[idx]->Process();
+    if (!mDocument) return;
+  }
+
   // If a generic notification occurs after this point then we may be allowed to
   // process it synchronously.  However we do not want to reenter if fireing
   // events causes script to run.

accessible/tests/mochitest/events/test_focus_aria_activedescendant.html

@@ -19,6 +19,10 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=429547
           src="../events.js"></script>
 
   <script type="application/javascript">
+    let PromEvents = {};
+    Services.scriptloader.loadSubScript(
+      "chrome://mochitests/content/a11y/accessible/tests/mochitest/promisified-events.js",
+      PromEvents);
     // gA11yEventDumpToConsole = true; // debugging
 
     function changeARIAActiveDescendant(aContainer, aItem) {
@@ -124,8 +128,15 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=429547
     }
 
     var gQueue = null;
-    function doTest() {
+    async function doTest() {
       gQueue = new eventQueue();
+      // Later tests use await.
+      let queueFinished = new Promise(resolve => {
+        gQueue.onFinish = function() {
+          resolve();
+          return DO_NOT_FINISH_TEST;
+        };
+      });
 
       gQueue.push(new synthFocus("listbox", new focusChecker("item1")));
       gQueue.push(new changeARIAActiveDescendant("listbox", "item2"));
@@ -156,7 +167,24 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=429547
       gQueue.push(new synthFocus(shadowListbox, new focusChecker(shadowItem1)));
       gQueue.push(new changeARIAActiveDescendant(shadowListbox, shadowItem2));
 
-      gQueue.invoke(); // Will call SimpleTest.finish();
+      gQueue.invoke();
+      await queueFinished;
+      // Tests beyond this point use await rather than eventQueue.
+
+      info("Testing simultaneous insertion, relocation and aria-activedescendant");
+      let comboboxWithHiddenList = getNode("comboboxWithHiddenList");
+      let focused = PromEvents.waitForEvent(EVENT_FOCUS, comboboxWithHiddenList);
+      comboboxWithHiddenList.focus();
+      await focused;
+      testStates(comboboxWithHiddenList, STATE_FOCUSED);
+      // hiddenList is owned, so unhiding causes insertion and relocation.
+      getNode("hiddenList").hidden = false;
+      focused = PromEvents.waitForEvent(EVENT_FOCUS, "hiddenListOption");
+      comboboxWithHiddenList.setAttribute("aria-activedescendant", "hiddenListOption");
+      await focused;
+      testStates("hiddenListOption", STATE_FOCUSED);
+
+      SimpleTest.finish();
     }
 
     SimpleTest.waitForExplicitFinish();
@@ -222,5 +250,11 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=429547
     item.setAttribute("role", "option");
     listbox.appendChild(item);
   </script>
+
+  <div id="comboboxWithHiddenList" tabindex="0" role="combobox" aria-owns="hiddenList">
+  </div>
+  <div id="hiddenList" hidden role="listbox">
+    <div id="hiddenListOption" role="option"></div>
+  </div>
 </body>
 </html>