Bug 1260860 - stop illicit accessible stealing, r=yzen
authorAlexander Surkov <surkov.alexander@gmail.com>
Fri, 01 Apr 2016 11:07:57 -0400
changeset 291195 325b2f66c001df47005b17b52477496e650ac12d
parent 291194 e9cff4b182abd31e332762bb0cd0d8c176d4df04
child 291196 1a33b1821e5bbafcd6837c755bc1d8fc6b25325b
push id30130
push userkwierso@gmail.com
push dateFri, 01 Apr 2016 22:29:37 +0000
treeherdermozilla-central@b6ea6a3bb8a6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersyzen
bugs1260860
milestone48.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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();