Bug 1372985 - ARIA owned children ordering may be incorrect. r=eeejay, a=gchang
authorAlexander Surkov <surkov.alexander@gmail.com>
Mon, 24 Jul 2017 12:13:46 -0400
changeset 356142 8fc70b2b57a8837ba295c23663c4874600f1a313
parent 356141 3856be3d222f6eade39f69c7c3bad32e53ae58be
child 356143 47dd26630291f3ffffc602d98273713b80ef18d9
push id7226
push userryanvm@gmail.com
push dateMon, 24 Jul 2017 16:17:12 +0000
treeherdermozilla-esr52@7b79969a18ee [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerseeejay, gchang
bugs1372985
milestone52.2.1
Bug 1372985 - ARIA owned children ordering may be incorrect. r=eeejay, a=gchang
accessible/generic/DocAccessible.cpp
--- a/accessible/generic/DocAccessible.cpp
+++ b/accessible/generic/DocAccessible.cpp
@@ -2047,72 +2047,68 @@ DocAccessible::ValidateARIAOwned()
       it.Remove();
     }
   }
 }
 
 void
 DocAccessible::DoARIAOwnsRelocation(Accessible* aOwner)
 {
-  nsTArray<RefPtr<Accessible> >* children = mARIAOwnsHash.LookupOrAdd(aOwner);
-
   MOZ_ASSERT(aOwner, "aOwner must be a valid pointer");
   MOZ_ASSERT(aOwner->Elm(), "aOwner->Elm() must be a valid pointer");
 
 #ifdef A11Y_LOG
   logging::TreeInfo("aria owns relocation", logging::eVerbose, aOwner);
 #endif
 
+  nsTArray<RefPtr<Accessible> >* owned = mARIAOwnsHash.LookupOrAdd(aOwner);
   IDRefsIterator iter(this, aOwner->Elm(), nsGkAtoms::aria_owns);
-  uint32_t arrayIdx = 0, insertIdx = aOwner->ChildCount() - children->Length();
+  uint32_t idx = 0;
   while (nsIContent* childEl = iter.NextElem()) {
     Accessible* child = GetAccessible(childEl);
+    auto insertIdx = aOwner->ChildCount() - owned->Length() + idx;
 
     // Make an attempt to create an accessible if it wasn't created yet.
     if (!child) {
       if (aOwner->IsAcceptableChild(childEl)) {
         child = GetAccService()->CreateAccessible(childEl, aOwner);
         if (child) {
           TreeMutation imut(aOwner);
           aOwner->InsertChildAt(insertIdx, child);
           imut.AfterInsertion(child);
           imut.Done();
 
           child->SetRelocated(true);
-          children->InsertElementAt(arrayIdx, child);
+          owned->InsertElementAt(idx, child);
+          idx++;
 
           // Create subtree before adjusting the insertion index, since subtree
           // creation may alter children in the container.
           CreateSubtree(child);
           FireEventsOnInsertion(aOwner);
-
-          insertIdx = child->IndexInParent() + 1;
-          arrayIdx++;
         }
       }
       continue;
     }
 
 #ifdef A11Y_LOG
   logging::TreeInfo("aria owns traversal", logging::eVerbose,
                     "candidate", child, nullptr);
 #endif
 
     // Same child on same position, no change.
     if (child->Parent() == aOwner &&
         child->IndexInParent() == static_cast<int32_t>(insertIdx)) {
-      NS_ASSERTION(child == children->ElementAt(arrayIdx), "Not in sync!");
-      insertIdx++; arrayIdx++;
+      MOZ_ASSERT(owned->ElementAt(idx) == child, "Not in sync!");
+      idx++;
       continue;
     }
 
-    NS_ASSERTION(children->SafeElementAt(arrayIdx) != child, "Already in place!");
-
-    nsTArray<RefPtr<Accessible> >::index_type idx = children->IndexOf(child);
-    if (idx < arrayIdx) {
+    MOZ_ASSERT(owned->SafeElementAt(idx) != child, "Already in place!");
+    if (owned->IndexOf(child) < idx) {
       continue; // ignore second entry of same ID
     }
 
     // A new child is found, check for loops.
     if (child->Parent() != aOwner) {
       Accessible* parent = aOwner;
       while (parent && parent != child && !parent->IsDoc()) {
         parent = parent->Parent();
@@ -2120,33 +2116,34 @@ DocAccessible::DoARIAOwnsRelocation(Acce
       // A referred child cannot be a parent of the owner.
       if (parent == child) {
         continue;
       }
     }
 
     if (MoveChild(child, aOwner, insertIdx)) {
       child->SetRelocated(true);
-      children->InsertElementAt(arrayIdx, child);
-      arrayIdx++;
-      insertIdx = child->IndexInParent() + 1;
+      owned->InsertElementAt(idx, child);
+      idx++;
     }
   }
 
   // Put back children that are not seized anymore.
-  PutChildrenBack(children, arrayIdx);
-  if (children->Length() == 0) {
+  PutChildrenBack(owned, idx);
+  if (owned->Length() == 0) {
     mARIAOwnsHash.Remove(aOwner);
   }
 }
 
 void
 DocAccessible::PutChildrenBack(nsTArray<RefPtr<Accessible> >* aChildren,
                                uint32_t aStartIdx)
 {
+  MOZ_ASSERT(aStartIdx <= aChildren->Length(), "Wrong removal index");
+
   nsTArray<RefPtr<Accessible> > containers;
   for (auto idx = aStartIdx; idx < aChildren->Length(); idx++) {
     Accessible* child = aChildren->ElementAt(idx);
     if (!child->IsInDocument()) {
       continue;
     }
 
     // Remove the child from the owner
@@ -2187,27 +2184,31 @@ DocAccessible::PutChildrenBack(nsTArray<
 }
 
 bool
 DocAccessible::MoveChild(Accessible* aChild, Accessible* aNewParent,
                          int32_t aIdxInParent)
 {
   MOZ_ASSERT(aChild, "No child");
   MOZ_ASSERT(aChild->Parent(), "No parent");
+  MOZ_ASSERT(aIdxInParent <= static_cast<int32_t>(aNewParent->ChildCount()),
+             "Wrong insertion point for a moving child");
 
   Accessible* curParent = aChild->Parent();
 
 #ifdef A11Y_LOG
   logging::TreeInfo("move child", 0,
                     "old parent", curParent, "new parent", aNewParent,
                     "child", aChild, nullptr);
 #endif
 
-  // If the child was taken from from an ARIA owns element.
+  // Forget aria-owns info in case of ARIA owned element. The caller is expected
+  // to update it if needed.
   if (aChild->IsRelocated()) {
+    aChild->SetRelocated(false);
     nsTArray<RefPtr<Accessible> >* children = mARIAOwnsHash.Get(curParent);
     children->RemoveElement(aChild);
   }
 
   NotificationController::MoveGuard mguard(mNotificationController);
 
   if (curParent == aNewParent) {
     MOZ_ASSERT(aChild->IndexInParent() != aIdxInParent, "No move case");