Fix for bug 626262 (Don't allow parent's ownerDocument to change during appendChild). r=bz, a=dveditz.
authorPeter Van der Beken <peterv@propagandism.org>
Mon, 07 Feb 2011 15:36:16 +0100
changeset 27423 5eb1fd50c7ad37149f47dcfd916e25ed97b84329
parent 27422 0c002f56fbae4293a35e42f832578664a3642a60
child 27424 d21c370f100927128401abeddfb3d59dabd873b8
push id2728
push userpvanderbeken@mozilla.com
push dateThu, 02 Jun 2011 11:28:04 +0000
reviewersbz, dveditz
bugs626262
milestone1.9.1.20pre
Fix for bug 626262 (Don't allow parent's ownerDocument to change during appendChild). r=bz, a=dveditz.
content/base/src/nsGenericElement.cpp
content/base/test/Makefile.in
content/base/test/test_bug626262.html
--- a/content/base/src/nsGenericElement.cpp
+++ b/content/base/src/nsGenericElement.cpp
@@ -3198,16 +3198,39 @@ nsGenericElement::InsertChildAt(nsIConte
                                 PRBool aNotify)
 {
   NS_PRECONDITION(aKid, "null ptr");
 
   return doInsertChildAt(aKid, aIndex, aNotify, this, GetCurrentDoc(),
                          mAttrsAndChildren);
 }
 
+static nsresult
+AdoptNodeIntoOwnerDoc(nsINode *aParent, nsIDOMNode *aNode)
+{
+  nsIDocument *doc = aParent->GetOwnerDoc();
+
+  nsresult rv;
+  nsCOMPtr<nsIDOM3Document> domDoc = do_QueryInterface(doc, &rv);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  nsCOMPtr<nsIDOMNode> adoptedNode;
+  rv = domDoc->AdoptNode(aNode, getter_AddRefs(adoptedNode));
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  if (doc != aParent->GetOwnerDoc()) {
+    return NS_ERROR_DOM_WRONG_DOCUMENT_ERR;
+  }
+
+  NS_ASSERTION(adoptedNode == aNode, "Uh, adopt node changed nodes?");
+  NS_ASSERTION(aParent->HasSameOwnerDoc(nsCOMPtr<nsINode>(do_QueryInterface(aNode))),
+               "ownerDocument changed again after adopting!");
+
+  return NS_OK;
+}
 
 /* static */
 nsresult
 nsGenericElement::doInsertChildAt(nsIContent* aKid, PRUint32 aIndex,
                                   PRBool aNotify, nsIContent* aParent,
                                   nsIDocument* aDocument,
                                   nsAttrAndChildArray& aChildArray)
 {
@@ -3221,30 +3244,23 @@ nsGenericElement::doInsertChildAt(nsICon
   if (!container->HasSameOwnerDoc(aKid)) {
     nsCOMPtr<nsIDOMNode> kid = do_QueryInterface(aKid, &rv);
     NS_ENSURE_SUCCESS(rv, rv);
  
     PRUint16 nodeType = 0;
     rv = kid->GetNodeType(&nodeType);
     NS_ENSURE_SUCCESS(rv, rv);
 
-    nsCOMPtr<nsIDOM3Document> domDoc =
-      do_QueryInterface(container->GetOwnerDoc());
-
     // DocumentType nodes are the only nodes that can have a null
     // ownerDocument according to the DOM spec, and we need to allow
     // inserting them w/o calling AdoptNode().
 
-    if (domDoc && (nodeType != nsIDOMNode::DOCUMENT_TYPE_NODE ||
-                   aKid->GetOwnerDoc())) {
-      nsCOMPtr<nsIDOMNode> adoptedKid;
-      rv = domDoc->AdoptNode(kid, getter_AddRefs(adoptedKid));
+    if (nodeType != nsIDOMNode::DOCUMENT_TYPE_NODE || aKid->GetOwnerDoc()) {
+      rv = AdoptNodeIntoOwnerDoc(container, kid);
       NS_ENSURE_SUCCESS(rv, rv);
-
-      NS_ASSERTION(adoptedKid == kid, "Uh, adopt node changed nodes?");
     }
   }
 
   nsMutationGuard::DidMutate();
 
   // Do this before checking the child-count since this could cause mutations
   mozAutoDocUpdate updateBatch(aDocument, UPDATE_CONTENT_MODEL, aNotify);
 
@@ -3762,25 +3778,18 @@ nsGenericElement::doReplaceOrInsertBefor
   }
 
   // DocumentType nodes are the only nodes that can have a null
   // ownerDocument according to the DOM spec, and we need to allow
   // inserting them w/o calling AdoptNode().
   if (!container->HasSameOwnerDoc(newContent) &&
       (nodeType != nsIDOMNode::DOCUMENT_TYPE_NODE ||
        newContent->GetOwnerDoc())) {
-    nsCOMPtr<nsIDOM3Document> domDoc = do_QueryInterface(aDocument);
-
-    if (domDoc) {
-      nsCOMPtr<nsIDOMNode> adoptedKid;
-      nsresult rv = domDoc->AdoptNode(aNewChild, getter_AddRefs(adoptedKid));
-      NS_ENSURE_SUCCESS(rv, rv);
-
-      NS_ASSERTION(adoptedKid == aNewChild, "Uh, adopt node changed nodes?");
-    }
+    res = AdoptNodeIntoOwnerDoc(container, aNewChild);
+    NS_ENSURE_SUCCESS(res, res);
   }
 
   // If we're replacing
   if (aReplace) {
     // Getting (and addrefing) the following child here is sort of wasteful
     // in the common case, but really, it's not that expensive. Get over it.
     refContent = container->GetChildAt(insPos + 1);
 
--- a/content/base/test/Makefile.in
+++ b/content/base/test/Makefile.in
@@ -284,16 +284,17 @@ include $(topsrcdir)/config/rules.mk
 		file_htmlserializer_ipv6_out.html \
 		test_bug498433.html \
 		test_bug498897.html \
 		file_bug498897.html \
 		file_bug498897.html^headers^ \
 		file_bug498897.css \
 		test_bug590771.html \
 		test_bug631615.html \
+		test_bug626262.html \
 		$(NULL)
 
 # Disabled for now. Mochitest isn't reliable enough for these.
 # test_bug444546.html \
 # bug444546.sjs \
 
 libs:: $(_TEST_FILES)
 	$(INSTALL) $(foreach f,$^,"$f") $(DEPTH)/_tests/testing/mochitest/tests/$(relativesrcdir)
new file mode 100644
--- /dev/null
+++ b/content/base/test/test_bug626262.html
@@ -0,0 +1,55 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=626262
+-->
+<head>
+  <title>Test for Bug 626262</title>
+  <script type="application/javascript" src="/MochiKit/packed.js"></script>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=626262">Mozilla Bug 626262</a>
+<p id="display"><iframe id="f" src="data:text/html,1"></iframe></p>
+<div id="content" style="display: none">
+  
+</div>
+<pre id="test">
+<script type="application/javascript">
+
+/** Test for Bug 626262 **/
+SimpleTest.waitForExplicitFinish();
+
+addLoadEvent(function() {
+  var iframe = document.getElementById("f");
+  var frameDoc = iframe.contentDocument;
+  var parent = frameDoc.createElementNS("http://www.w3.org/1999/xhtml", "div");
+
+  function a()
+  {
+    window.removeEventListener("DOMNodeRemoved", arguments.callee, false);
+    document.adoptNode(parent);
+  }
+
+  var text = document.createTextNode(" ");
+  document.documentElement.appendChild(text);
+
+  var thrown = false;
+  try {
+    window.addEventListener("DOMNodeRemoved", a, false);
+    parent.appendChild(text);
+  }
+  catch (e) {
+    thrown = true;
+  }
+
+  ok(thrown, "changing ownerDocument during adoptNode should throw");
+
+  SimpleTest.finish();
+});
+
+</script>
+</pre>
+</body>
+</html>