Bug 1040735 - DOM node reinsertion under anonymous content may trigger a11y child adoption, r=bz, tbdaunde, davidb
authorAlexander Surkov <surkov.alexander@gmail.com>
Fri, 19 Sep 2014 20:02:30 -0400
changeset 206330 478ea4b85c00bfcbebf221295145c963b7dcaf0c
parent 206329 8e6e924ff00bbfc6654e5364ae894b4d9a717880
child 206331 da1f707ddef9164b3aea7499acb2c0226692d7ac
push id27524
push usercbook@mozilla.com
push dateMon, 22 Sep 2014 10:59:09 +0000
treeherdermozilla-central@53f7f5b6d7bf [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz, tbdaunde, davidb
bugs1040735
milestone35.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 1040735 - DOM node reinsertion under anonymous content may trigger a11y child adoption, r=bz, tbdaunde, davidb
accessible/base/nsAccessibilityService.cpp
accessible/base/nsAccessibilityService.h
accessible/generic/DocAccessible.cpp
accessible/generic/DocAccessible.h
accessible/tests/mochitest/treeupdate/a11y.ini
accessible/tests/mochitest/treeupdate/test_bug1040735.html
accessible/tests/mochitest/treeupdate/test_optgroup.html
accessible/tests/mochitest/treeupdate/test_select.html
layout/base/RestyleManager.cpp
layout/base/nsCSSFrameConstructor.cpp
layout/xul/nsListBoxBodyFrame.cpp
--- a/accessible/base/nsAccessibilityService.cpp
+++ b/accessible/base/nsAccessibilityService.cpp
@@ -29,16 +29,17 @@
 #include "Platform.h"
 #include "Role.h"
 #ifdef MOZ_ACCESSIBILITY_ATK
 #include "RootAccessibleWrap.h"
 #endif
 #include "States.h"
 #include "Statistics.h"
 #include "TextLeafAccessibleWrap.h"
+#include "TreeWalker.h"
 
 #ifdef MOZ_ACCESSIBILITY_ATK
 #include "AtkSocketAccessible.h"
 #endif
 
 #ifdef XP_WIN
 #include "mozilla/a11y/Compatibility.h"
 #include "HTMLWin32ObjectAccessible.h"
@@ -386,32 +387,54 @@ nsAccessibilityService::ContentRangeInse
 
   DocAccessible* docAccessible = GetDocAccessible(aPresShell);
   if (docAccessible)
     docAccessible->ContentInserted(aContainer, aStartChild, aEndChild);
 }
 
 void
 nsAccessibilityService::ContentRemoved(nsIPresShell* aPresShell,
-                                       nsIContent* aContainer,
-                                       nsIContent* aChild)
+                                       nsIContent* aChildNode)
 {
 #ifdef A11Y_LOG
   if (logging::IsEnabled(logging::eTree)) {
     logging::MsgBegin("TREE", "content removed");
-    logging::Node("container", aContainer);
-    logging::Node("content", aChild);
+    logging::Node("container", aChildNode->GetFlattenedTreeParent());
+    logging::Node("content", aChildNode);
+  }
+#endif
+
+  DocAccessible* document = GetDocAccessible(aPresShell);
+  if (document) {
+    // Flatten hierarchy may be broken at this point so we cannot get a true
+    // container by traversing up the DOM tree. Find a parent of first accessible
+    // from the subtree of the given DOM node, that'll be a container. If no
+    // accessibles in subtree then we don't care about the change.
+    Accessible* child = document->GetAccessible(aChildNode);
+    if (!child) {
+      a11y::TreeWalker walker(document->GetContainerAccessible(aChildNode),
+                              aChildNode, a11y::TreeWalker::eWalkCache);
+      child = walker.NextChild();
+    }
+
+    if (child) {
+      document->ContentRemoved(child->Parent(), aChildNode);
+#ifdef A11Y_LOG
+      if (logging::IsEnabled(logging::eTree))
+        logging::AccessibleNNode("real container", child->Parent());
+#endif
+    }
+  }
+
+#ifdef A11Y_LOG
+  if (logging::IsEnabled(logging::eTree)) {
     logging::MsgEnd();
     logging::Stack();
   }
 #endif
-
-  DocAccessible* docAccessible = GetDocAccessible(aPresShell);
-  if (docAccessible)
-    docAccessible->ContentRemoved(aContainer, aChild);
 }
 
 void
 nsAccessibilityService::UpdateText(nsIPresShell* aPresShell,
                                    nsIContent* aContent)
 {
   DocAccessible* document = GetDocAccessible(aPresShell);
   if (document)
--- a/accessible/base/nsAccessibilityService.h
+++ b/accessible/base/nsAccessibilityService.h
@@ -85,18 +85,17 @@ public:
    * inserted.
    */
   void ContentRangeInserted(nsIPresShell* aPresShell, nsIContent* aContainer,
                             nsIContent* aStartChild, nsIContent* aEndChild);
 
   /**
    * Notification used to update the accessible tree when content is removed.
    */
-  void ContentRemoved(nsIPresShell* aPresShell, nsIContent* aContainer,
-                      nsIContent* aChild);
+  void ContentRemoved(nsIPresShell* aPresShell, nsIContent* aChild);
 
   virtual void UpdateText(nsIPresShell* aPresShell, nsIContent* aContent);
 
   /**
    * Update XUL:tree accessible tree when treeview is changed.
    */
   void TreeViewChanged(nsIPresShell* aPresShell, nsIContent* aContent,
                        nsITreeView* aView);
--- a/accessible/generic/DocAccessible.cpp
+++ b/accessible/generic/DocAccessible.cpp
@@ -1413,28 +1413,16 @@ DocAccessible::ContentInserted(nsIConten
 
     mNotificationController->ScheduleContentInsertion(container,
                                                       aStartChildNode,
                                                       aEndChildNode);
   }
 }
 
 void
-DocAccessible::ContentRemoved(nsIContent* aContainerNode,
-                              nsIContent* aChildNode)
-{
-  // Update the whole tree of this document accessible when the container is
-  // null (document element is removed).
-  Accessible* container = aContainerNode ?
-    GetAccessibleOrContainer(aContainerNode) : this;
-
-  UpdateTree(container, aChildNode, false);
-}
-
-void
 DocAccessible::RecreateAccessible(nsIContent* aContent)
 {
 #ifdef A11Y_LOG
   if (logging::IsEnabled(logging::eTree)) {
     logging::MsgBegin("TREE", "accessible recreated");
     logging::Node("content", aContent);
     logging::MsgEnd();
   }
--- a/accessible/generic/DocAccessible.h
+++ b/accessible/generic/DocAccessible.h
@@ -291,17 +291,26 @@ public:
    */
   void ContentInserted(nsIContent* aContainerNode,
                        nsIContent* aStartChildNode,
                        nsIContent* aEndChildNode);
 
   /**
    * Notify the document accessible that content was removed.
    */
-  void ContentRemoved(nsIContent* aContainerNode, nsIContent* aChildNode);
+  void ContentRemoved(Accessible* aContainer, nsIContent* aChildNode)
+  {
+    // Update the whole tree of this document accessible when the container is
+    // null (document element is removed).
+    UpdateTree((aContainer ? aContainer : this), aChildNode, false);
+  }
+  void ContentRemoved(nsIContent* aContainerNode, nsIContent* aChildNode)
+  {
+    ContentRemoved(GetAccessibleOrContainer(aContainerNode), aChildNode);
+  }
 
   /**
    * Updates accessible tree when rendered text is changed.
    */
   void UpdateText(nsIContent* aTextNode);
 
   /**
    * Recreate an accessible, results in hide/show events pair.
--- a/accessible/tests/mochitest/treeupdate/a11y.ini
+++ b/accessible/tests/mochitest/treeupdate/a11y.ini
@@ -1,15 +1,16 @@
 [DEFAULT]
 
 [test_ariadialog.html]
 [test_bug852150.xhtml]
 [test_bug883708.xhtml]
 [test_bug884251.xhtml]
 [test_bug895082.html]
+[test_bug1040735.html]
 [test_canvas.html]
 [test_colorpicker.xul]
 [test_contextmenu.xul]
 [test_cssoverflow.html]
 [test_deck.xul]
 [test_doc.html]
 [test_gencontent.html]
 [test_hidden.html]
new file mode 100644
--- /dev/null
+++ b/accessible/tests/mochitest/treeupdate/test_bug1040735.html
@@ -0,0 +1,42 @@
+<!DOCTYPE html>
+<html>
+<head>
+  <title>Adopt DOM node from anonymous subtree</title>
+
+  <link rel="stylesheet" type="text/css"
+        href="chrome://mochikit/content/tests/SimpleTest/test.css" />
+
+  <script type="application/javascript"
+          src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
+  <script type="application/javascript"
+          src="../common.js"></script>
+
+  <script type="application/javascript">
+    function doTest()
+    {
+      document.body.appendChild(document.getElementById("mw_a"));
+      setTimeout(function() { ok(true, "no crash and assertions"); SimpleTest.finish(); } , 0);
+    }
+
+    SimpleTest.waitForExplicitFinish();
+    addA11yLoadEvent(doTest);
+  </script>
+</head>
+
+<body>
+  <a target="_blank"
+     href="https://bugzilla.mozilla.org/show_bug.cgi?id=1040735"
+     title="Bug 1040735 - DOM node reinsertion under anonymous content may trigger a11y child adoption">
+    Bug 1040735</a>
+  <p id="display"></p>
+  <div id="content" style="display: none"></div>
+  <pre id="test">
+  </pre>
+
+  <marquee>
+    <div id="mw_a" style="visibility: hidden;">
+      <div style="visibility: visible;" id="mw_inside"></div>
+    </div>
+  </marquee>
+</body>
+</html>
--- a/accessible/tests/mochitest/treeupdate/test_optgroup.html
+++ b/accessible/tests/mochitest/treeupdate/test_optgroup.html
@@ -46,17 +46,17 @@
       {
         var tree =
           { COMBOBOX: [
             { COMBOBOX_LIST: [
               { GROUPING: [
                 { COMBOBOX_OPTION: [
                   { TEXT_LEAF: [] }
                 ] },
-              { COMBOBOX_OPTION: [
+                { COMBOBOX_OPTION: [
                   { TEXT_LEAF: [] }
                 ] },
               ]},
               { COMBOBOX_OPTION: [] }
             ] }
           ] };
         testAccessibleTree(this.select, tree);
       }
@@ -66,25 +66,26 @@
         return "test optgroup's insertion into a select";
       }
     }
 
     function removeOptGroup(aID)
     {
       this.selectNode = getNode(aID);
       this.select = getAccessible(this.selectNode);
+      this.selectList = this.select.firstChild;
 
       this.invoke = function removeOptGroup_invoke()
       {
         this.option1Node = this.selectNode.firstChild.firstChild;
         this.selectNode.removeChild(this.selectNode.firstChild);
       }
 
       this.eventSeq = [
-        new invokerChecker(EVENT_REORDER, this.select)
+        new invokerChecker(EVENT_REORDER, this.selectList)
       ];
 
       this.finalCheck = function removeOptGroup_finalCheck()
       {
         var tree =
           { COMBOBOX: [
             { COMBOBOX_LIST: [
               { COMBOBOX_OPTION: [] }
--- a/accessible/tests/mochitest/treeupdate/test_select.html
+++ b/accessible/tests/mochitest/treeupdate/test_select.html
@@ -58,25 +58,26 @@
         return "test elements insertion into a select";
       }
     }
 
     function removeOptions(aID)
     {
       this.selectNode = getNode(aID);
       this.select = getAccessible(this.selectNode);
+      this.selectList = this.select.firstChild;
 
       this.invoke = function removeOptions_invoke()
       {
         while (this.selectNode.length)
           this.selectNode.remove(0);
       }
 
       this.eventSeq = [
-        new invokerChecker(EVENT_REORDER, this.select)
+        new invokerChecker(EVENT_REORDER, this.selectList)
       ];
 
       this.finalCheck = function removeOptions_finalCheck()
       {
         var tree =
           { COMBOBOX: [
             { COMBOBOX_LIST: [] }
           ] };
--- a/layout/base/RestyleManager.cpp
+++ b/layout/base/RestyleManager.cpp
@@ -3404,17 +3404,17 @@ ElementRestyler::SendAccessibilityNotifi
                                        content,
                                        content->GetNextSibling());
     }
   } else if (mOurA11yNotification == eNotifyHidden) {
     nsAccessibilityService* accService = nsIPresShell::AccService();
     if (accService) {
       nsIPresShell* presShell = mFrame->PresContext()->GetPresShell();
       nsIContent* content = mFrame->GetContent();
-      accService->ContentRemoved(presShell, content->GetParent(), content);
+      accService->ContentRemoved(presShell, content);
 
       // Process children staying shown.
       uint32_t visibleContentCount = mVisibleKidsOfHiddenElement.Length();
       for (uint32_t idx = 0; idx < visibleContentCount; idx++) {
         nsIContent* childContent = mVisibleKidsOfHiddenElement[idx];
         accService->ContentRangeInserted(presShell, childContent->GetParent(),
                                          childContent,
                                          childContent->GetNextSibling());
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@@ -7798,17 +7798,17 @@ nsCSSFrameConstructor::ContentRemoved(ns
       nsresult rv = RecreateFramesForContent(grandparentFrame->GetContent(), true);
       LAYOUT_PHASE_TEMP_REENTER();
       return rv;
     }
 
 #ifdef ACCESSIBILITY
     nsAccessibilityService* accService = nsIPresShell::AccService();
     if (accService) {
-      accService->ContentRemoved(mPresShell, aContainer, aChild);
+      accService->ContentRemoved(mPresShell, aChild);
     }
 #endif
 
     // Examine the containing-block for the removed content and see if
     // :first-letter style applies.
     nsIFrame* inflowChild = childFrame;
     if (childFrame->GetStateBits() & NS_FRAME_OUT_OF_FLOW) {
       inflowChild = GetPlaceholderFrameFor(childFrame);
--- a/layout/xul/nsListBoxBodyFrame.cpp
+++ b/layout/xul/nsListBoxBodyFrame.cpp
@@ -1507,18 +1507,17 @@ nsListBoxBodyFrame::RemoveChildFrame(nsB
 {
   MOZ_ASSERT(mFrames.ContainsFrame(aFrame));
   MOZ_ASSERT(aFrame != GetContentInsertionFrame());
 
 #ifdef ACCESSIBILITY
   nsAccessibilityService* accService = nsIPresShell::AccService();
   if (accService) {
     nsIContent* content = aFrame->GetContent();
-    accService->ContentRemoved(PresContext()->PresShell(), content->GetParent(),
-                               content);
+    accService->ContentRemoved(PresContext()->PresShell(), content);
   }
 #endif
 
   mFrames.RemoveFrame(aFrame);
   if (mLayoutManager)
     mLayoutManager->ChildrenRemoved(this, aState, aFrame);
   aFrame->Destroy();
 }