Bug 1260860 - stop illicit accessible stealing, r=yzen
authorAlexander Surkov <surkov.alexander@gmail.com>
Fri, 01 Apr 2016 11:07:57 -0400
changeset 291254 325b2f66c001df47005b17b52477496e650ac12d
parent 291253 e9cff4b182abd31e332762bb0cd0d8c176d4df04
child 291255 1a33b1821e5bbafcd6837c755bc1d8fc6b25325b
push id19656
push usergwagner@mozilla.com
push dateMon, 04 Apr 2016 13:43:23 +0000
treeherderb2g-inbound@e99061fde28a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersyzen
bugs1260860
milestone48.0a1
Bug 1260860 - stop illicit accessible stealing, r=yzen
accessible/generic/Accessible.cpp
accessible/generic/DocAccessible.cpp
accessible/html/HTMLListAccessible.cpp
accessible/tests/mochitest/events/test_mutation.html
accessible/tests/mochitest/tree/test_aria_owns.html
accessible/xul/XULElementAccessibles.cpp
--- a/accessible/generic/Accessible.cpp
+++ b/accessible/generic/Accessible.cpp
@@ -1958,38 +1958,27 @@ Accessible::NativeName(nsString& aName)
 
   return eNameOK;
 }
 
 // Accessible protected
 void
 Accessible::BindToParent(Accessible* aParent, uint32_t aIndexInParent)
 {
-  NS_PRECONDITION(aParent, "This method isn't used to set null parent!");
-
-  if (mParent) {
-    if (mParent != aParent) {
+  MOZ_ASSERT(aParent, "This method isn't used to set null parent");
+  MOZ_ASSERT(!mParent, "The child was expected to be moved");
+
 #ifdef A11Y_LOG
-      logging::TreeInfo("BindToParent: stealing accessible", 0,
-                        "old parent", mParent.get(),
-                        "new parent", aParent,
-                        "child", this, nullptr);
+  if (mParent) {
+    logging::TreeInfo("BindToParent: stealing accessible", 0,
+                      "old parent", mParent.get(),
+                      "new parent", aParent,
+                      "child", this, nullptr);
+  }
 #endif
-      // XXX: legalize adoption. As long as we don't invalidate the children,
-      // the accessibles start to steal them.
-
-      AutoTreeMutation mt(mParent);
-      mt.BeforeRemoval(this);
-      mParent->RemoveChild(this);
-      mt.Done();
-    } else {
-      NS_ERROR("Binding to the same parent!");
-      return;
-    }
-  }
 
   mParent = aParent;
   mIndexInParent = aIndexInParent;
 
   // Note: this is currently only used for richlistitems and their children.
   if (mParent->HasNameDependentParent() || mParent->IsXULListItem())
     mContextFlags |= eHasNameDependentParent;
   else
--- a/accessible/generic/DocAccessible.cpp
+++ b/accessible/generic/DocAccessible.cpp
@@ -2257,29 +2257,32 @@ DocAccessible::CacheChildrenInSubtree(Ac
                                       Accessible** aFocusedAcc)
 {
   // If the accessible is focused then report a focus event after all related
   // mutation events.
   if (aFocusedAcc && !*aFocusedAcc &&
       FocusMgr()->HasDOMFocus(aRoot->GetContent()))
     *aFocusedAcc = aRoot;
 
-  aRoot->EnsureChildren();
+  Accessible* root = aRoot->IsHTMLCombobox() ? aRoot->FirstChild() : aRoot;
+  if (root->KidsFromDOM()) {
+    AutoTreeMutation mt(root);
+    TreeWalker walker(root);
+    while (Accessible* child = walker.Next()) {
+      if (child->IsBoundToParent()) {
+        MoveChild(child, root, root->ChildCount());
+        continue;
+      }
 
-  // Make sure we create accessible tree defined in DOM only, i.e. if accessible
-  // provides specific tree (like XUL trees) then tree creation is handled by
-  // this accessible.
-  uint32_t count = aRoot->ContentChildCount();
-  for (uint32_t idx = 0; idx < count; idx++) {
-    Accessible* child = aRoot->ContentChildAt(idx);
-    NS_ASSERTION(child, "Illicit tree change while tree is created!");
-    // Don't cross document boundaries.
-    if (child && child->IsContent()) {
+      root->AppendChild(child);
+      mt.AfterInsertion(child);
+
       CacheChildrenInSubtree(child, aFocusedAcc);
     }
+    mt.Done();
   }
 
   // Fire document load complete on ARIA documents.
   // XXX: we should delay an event if the ARIA document has aria-busy.
   if (aRoot->HasARIARole() && !aRoot->IsDoc()) {
     a11y::role role = aRoot->ARIARole();
     if (role == roles::DIALOG || role == roles::DOCUMENT)
       FireDelayedEvent(nsIAccessibleEvent::EVENT_DOCUMENT_LOAD_COMPLETE, aRoot);
--- a/accessible/html/HTMLListAccessible.cpp
+++ b/accessible/html/HTMLListAccessible.cpp
@@ -46,16 +46,17 @@ HTMLLIAccessible::
   HyperTextAccessibleWrap(aContent, aDoc), mBullet(nullptr)
 {
   mType = eHTMLLiType;
 
   nsBlockFrame* blockFrame = do_QueryFrame(GetFrame());
   if (blockFrame && blockFrame->HasBullet()) {
     mBullet = new HTMLListBulletAccessible(mContent, mDoc);
     Document()->BindToDocument(mBullet, nullptr);
+    AppendChild(mBullet);
   }
 }
 
 NS_IMPL_ISUPPORTS_INHERITED0(HTMLLIAccessible, HyperTextAccessible)
 
 void
 HTMLLIAccessible::Shutdown()
 {
--- a/accessible/tests/mochitest/events/test_mutation.html
+++ b/accessible/tests/mochitest/events/test_mutation.html
@@ -356,17 +356,17 @@
       {
         return "insert inaccessible element and then insert referring element to make it accessible";
       }
     }
 
     function showHiddenParentOfVisibleChild()
     {
       this.eventSeq = [
-        new todo_invokerChecker(EVENT_HIDE, getNode("c4_child")),
+        new invokerChecker(EVENT_HIDE, getNode("c4_child")),
         new invokerChecker(EVENT_SHOW, getNode("c4_middle")),
         new invokerChecker(EVENT_REORDER, getNode("c4"))
       ];
 
       this.invoke = function showHiddenParentOfVisibleChild_invoke()
       {
         getNode("c4_middle").style.visibility = 'visible';
       }
--- a/accessible/tests/mochitest/tree/test_aria_owns.html
+++ b/accessible/tests/mochitest/tree/test_aria_owns.html
@@ -139,21 +139,21 @@
   <div id="t1_2" aria-owns="t1_1" role="heading"></div>
 
   <!-- loop -->
   <div id="t2_2" aria-owns="t2_3" role="group"></div>
   <div id="t2_1" aria-owns="t2_2"></div>
   <div id="t2_3" aria-owns="t2_1" role="heading"></div>
 
   <!-- loop #2 -->
+  <div id="t3_3" aria-owns="t3_1"></div>
   <div id="t3_1" aria-owns="t3_2" role="group"></div>
   <div id="t3_2" role="note">
     <div aria-owns="t3_3" role="heading"></div>
   </div>
-  <div id="t3_3" aria-owns="t3_1"></div>
 
   <!-- self loop -->
   <div id="t4_1"><div aria-owns="t4_1" role="group"></div></div>
 
   <!-- natural and aria-owns hierarchy -->
   <div id="t5_1"><div aria-owns="t5_2" role="group"></div></div>
   <div id="t5_2" role="note"><div aria-owns="t5_3" role="heading"></div></div>
   <div id="t5_3" role="form"><div aria-owns="t5_1" role="tooltip"></div></div>
--- a/accessible/xul/XULElementAccessibles.cpp
+++ b/accessible/xul/XULElementAccessibles.cpp
@@ -45,16 +45,17 @@ XULLabelAccessible::
   nsTextBoxFrame* textBoxFrame = do_QueryFrame(mContent->GetPrimaryFrame());
   if (textBoxFrame) {
     mValueTextLeaf = new XULLabelTextLeafAccessible(mContent, mDoc);
     mDoc->BindToDocument(mValueTextLeaf, nullptr);
 
     nsAutoString text;
     textBoxFrame->GetCroppedTitle(text);
     mValueTextLeaf->SetText(text);
+    AppendChild(mValueTextLeaf);
   }
 }
 
 void
 XULLabelAccessible::Shutdown()
 {
   mValueTextLeaf = nullptr;
   HyperTextAccessibleWrap::Shutdown();