Bug 1455416: Relocate aria-owned accessibles on (aria) parent removal, r=Jamie

This revision modifies UncacheChildrenInSubtree such that removed but relocated
accessibles that are aria-owned are actually relocated to their proper parent
after the removal of the formerly-aria-owning parent. On the way to
accomplishing that goal, this revision adds PutChildBack, a method called by
PutChildrenBack and now also called by UncacheChildrenInSubtree. We don't always
want to put all the children back - this function lets us manage it one at a time.
Finally, this revision adds tests which verify that the functionality works
as intended.

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

accessible/generic/DocAccessible.cpp

@@ -2405,6 +2405,11 @@ void DocAccessible::PutChildrenBack(
     LocalAccessible* origContainer =
         AccessibleOrTrueContainer(content->GetFlattenedTreeParentNode());
     if (origContainer) {
+      // If the target container isn't in the document, there's no need to
+      // determine where the child should go for relocation. We can move on.
+      if (!origContainer->IsInDocument()) {
+        continue;
+      }
       TreeWalker walker(origContainer);
       if (walker.Seek(content)) {
         LocalAccessible* prevChild = walker.Prev();
@@ -2596,19 +2601,24 @@ void DocAccessible::UncacheChildrenInSubtree(LocalAccessible* aRoot) {
     CachedTableAccessible::Invalidate(aRoot);
   }
 
+  // Put relocated children back in their original places instead of removing
+  // them from the tree.
   nsTArray<RefPtr<LocalAccessible>>* owned = mARIAOwnsHash.Get(aRoot);
-  uint32_t count = aRoot->ContentChildCount();
-  for (uint32_t idx = 0; idx < count; idx++) {
+  if (owned) {
+    PutChildrenBack(owned, 0);
+    MOZ_ASSERT(owned->IsEmpty(),
+               "Owned Accessibles should be cleared after PutChildrenBack.");
+    mARIAOwnsHash.Remove(aRoot);
+    owned = nullptr;
+  }
+
+  const uint32_t count = aRoot->ContentChildCount();
+  for (uint32_t idx = 0; idx < count; ++idx) {
     LocalAccessible* child = aRoot->ContentChildAt(idx);
 
-    if (child->IsRelocated()) {
-      MOZ_ASSERT(owned, "IsRelocated flag is out of sync with mARIAOwnsHash");
-      owned->RemoveElement(child);
-      if (owned->Length() == 0) {
-        mARIAOwnsHash.Remove(aRoot);
-        owned = nullptr;
-      }
-    }
+    MOZ_ASSERT(!child->IsRelocated(),
+               "No children should be relocated here. They should all have "
+               "been relocated by PutChildrenBack.");
 
     // Removing this accessible from the document doesn't mean anything about
     // accessibles for subdocuments, so skip removing those from the tree.

accessible/tests/browser/e10s/browser_treeupdate_ariaowns.js

@@ -323,3 +323,99 @@ addAccessibleTask(
     });
   }
 );
+
+// Verify that removing the parent of a DOM-sibling aria-owned child keeps the
+// formerly-owned child in the tree.
+addAccessibleTask(
+  `<input id='x'></input><div aria-owns='x'></div>`,
+  async function(browser, accDoc) {
+    testAccessibleTree(accDoc, {
+      DOCUMENT: [{ SECTION: [{ ENTRY: [] }] }],
+    });
+
+    info("Removing the div that aria-owns a DOM sibling");
+    let onReorder = waitForEvent(EVENT_REORDER, accDoc);
+    await invokeContentTask(browser, [], () => {
+      content.document.querySelector("div").remove();
+    });
+    await onReorder;
+
+    info("Verifying that the formerly-owned child is still present");
+    testAccessibleTree(accDoc, {
+      DOCUMENT: [{ ENTRY: [] }],
+    });
+  },
+  { chrome: true, iframe: true, remoteIframe: true }
+);
+
+// Verify that removing the parent of multiple DOM-sibling aria-owned children
+// keeps all formerly-owned children in the tree.
+addAccessibleTask(
+  `<input id='x'></input><input id='y'><div aria-owns='x y'></div>`,
+  async function(browser, accDoc) {
+    testAccessibleTree(accDoc, {
+      DOCUMENT: [
+        {
+          SECTION: [{ ENTRY: [] }, { ENTRY: [] }],
+        },
+      ],
+    });
+
+    info("Removing the div that aria-owns DOM siblings");
+    let onReorder = waitForEvent(EVENT_REORDER, accDoc);
+    await invokeContentTask(browser, [], () => {
+      content.document.querySelector("div").remove();
+    });
+    await onReorder;
+
+    info("Verifying that the formerly-owned children are still present");
+    testAccessibleTree(accDoc, {
+      DOCUMENT: [{ ENTRY: [] }, { ENTRY: [] }],
+    });
+  },
+  { chrome: true, iframe: true, remoteIframe: true }
+);
+
+// Verify that reordering owned elements by changing the aria-owns attribute
+// properly reorders owned elements.
+addAccessibleTask(
+  `
+<div id="container" aria-owns="b d c a">
+  <div id="a" role="button"></div>
+  <div id="b" role="checkbox"></div>
+</div>
+<div id="c" role="radio"></div>
+<div id="d"></div>`,
+  async function(browser, accDoc) {
+    testAccessibleTree(accDoc, {
+      DOCUMENT: [
+        {
+          SECTION: [
+            { CHECKBUTTON: [] }, // b
+            { SECTION: [] }, // d
+            { RADIOBUTTON: [] }, // c
+            { PUSHBUTTON: [] }, // a
+          ],
+        },
+      ],
+    });
+
+    info("Removing the div that aria-owns other elements");
+    let onReorder = waitForEvent(EVENT_REORDER, accDoc);
+    await invokeContentTask(browser, [], () => {
+      content.document.querySelector("#container").remove();
+    });
+    await onReorder;
+
+    info(
+      "Verify DOM children are removed, order of remaining elements is correct"
+    );
+    testAccessibleTree(accDoc, {
+      DOCUMENT: [
+        { RADIOBUTTON: [] }, // c
+        { SECTION: [] }, // d
+      ],
+    });
+  },
+  { chrome: true, iframe: true, remoteIframe: true }
+);

accessible/tests/mochitest/treeupdate/test_ariaowns.html

@@ -654,7 +654,7 @@
       this.finalCheck = () => {
         let tree =
           { SECTION: [ // t10_container
-            // { ENTRY: [] }, // t10_child
+            { ENTRY: [] }, // t10_child
             { PARAGRAPH: [] },
           ] };
         testAccessibleTree("t10_container", tree);