Bug 1837315: Avoid queueing a hide event on an accessible that's already being moved, r=Jamie

If we move an accessible via an aria-owns relocation on a grandparent, then move
it again via an aria-owns relocation on a parent, we can end up with multiple
hide events in the queue: one for the shallowest hide (of the grandparent), and
one for the hide of the grandchild accessible. We can't always coalesce (drop)
the hide event of the grandchild since the first move severs the relationship
between its parent and its grandparent. To address this, this revision stops us
from queueing a hide event on an accessible that's already being moved. This
revision also adds a test to verify we're getting the proper events.

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

accessible/base/NotificationController.cpp

@@ -139,6 +139,13 @@ bool NotificationController::QueueMutationEvent(AccTreeMutationEvent* aEvent) {
       return false;
     }
 
+    // Don't queue a hide event on an accessible that's already being moved. It
+    // or an ancestor should already have a hide event queued.
+    if (mDocument &&
+        mDocument->IsAccessibleBeingMoved(aEvent->GetAccessible())) {
+      return false;
+    }
+
     // If this is an additional hide event, the accessible may be hidden, or
     // moved again after a move. Preserve the original hide event since
     // its properties are consistent with the tree that existed before

accessible/generic/DocAccessible.h

@@ -415,6 +415,13 @@ class DocAccessible : public HyperTextAccessibleWrap,
    */
   std::pair<nsPoint, nsRect> ComputeScrollData(LocalAccessible* aAcc);
 
+  /**
+   * Only works in content process documents.
+   */
+  bool IsAccessibleBeingMoved(LocalAccessible* aAcc) {
+    return mMovedAccessibles.Contains(aAcc);
+  }
+
  protected:
   virtual ~DocAccessible();
 
@@ -535,13 +542,6 @@ class DocAccessible : public HyperTextAccessibleWrap,
    */
   void ProcessQueuedCacheUpdates();
 
-  /**
-   * Only works in content process documents.
-   */
-  bool IsAccessibleBeingMoved(LocalAccessible* aAcc) {
-    return mMovedAccessibles.Contains(aAcc);
-  }
-
   /**
    * Called from NotificationController before mutation events are processed to
    * notify the parent process which Accessibles are being moved (if any).

accessible/tests/browser/e10s/browser_treeupdate_ariaowns.js

@@ -419,3 +419,39 @@ addAccessibleTask(
   },
   { chrome: true, iframe: true, remoteIframe: true }
 );
+
+// Verify that we avoid sending unwanted hide events when doing multiple
+// aria-owns relocations in a single tick. Note that we're avoiding testing
+// chrome here since parent process locals don't track moves in the same way,
+// meaning our mechanism for avoiding duplicate hide events doesn't work.
+addAccessibleTask(
+  `
+<div id='b' aria-owns='a'></div>
+<div id='d'></div>
+<dd id='f'>
+  <div id='a' aria-owns='d'></div>
+</dd>
+  `,
+  async function (browser, accDoc) {
+    const b = findAccessibleChildByID(accDoc, "b");
+    const waitFor = {
+      expected: [
+        [EVENT_HIDE, b],
+        [EVENT_SHOW, "d"],
+        [EVENT_REORDER, accDoc],
+      ],
+      unexpected: [
+        [EVENT_HIDE, "d"],
+        [EVENT_REORDER, "a"],
+      ],
+    };
+    info(
+      "Verifying that events are fired properly after doing two aria-owns relocations"
+    );
+    await contentSpawnMutation(browser, waitFor, function () {
+      content.document.querySelector("#b").remove();
+      content.document.querySelector("#f").remove();
+    });
+  },
+  { chrome: false, iframe: true, remoteIframe: true }
+);