Bug 420700 - Calling createContextualFragment affects subsequent setting of innerHTML. r+sr=jst, a=blocking1.9+.
authorbent.mozilla@gmail.com
Thu, 06 Mar 2008 12:14:33 -0800
changeset 12676 274130eafe8550aed4348c8edbcf66ba97571f40
parent 12675 546d38e52abf52b572ad8bcf7d37c3a63e80f7da
child 12677 d1f43445f740a5e11c03a0ab7f57790f69add4f4
push id1
push userbsmedberg@mozilla.com
push dateThu, 20 Mar 2008 16:49:24 +0000
treeherdermozilla-central@61007906a1f8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersblocking1.9
bugs420700
milestone1.9b5pre
Bug 420700 - Calling createContextualFragment affects subsequent setting of innerHTML. r+sr=jst, a=blocking1.9+.
content/base/public/nsContentUtils.h
content/base/src/nsContentUtils.cpp
content/base/src/nsRange.cpp
content/base/test/Makefile.in
content/base/test/test_bug420700.html
content/html/content/src/nsGenericHTMLElement.cpp
content/html/document/src/nsHTMLFragmentContentSink.cpp
content/xml/document/src/nsXMLFragmentContentSink.cpp
parser/htmlparser/public/nsIFragmentContentSink.h
--- a/content/base/public/nsContentUtils.h
+++ b/content/base/public/nsContentUtils.h
@@ -896,20 +896,23 @@ public:
                                 PRInt32 aNamespaceID);
 
   /**
    * Creates a DocumentFragment from text using a context node to resolve
    * namespaces.
    *
    * @param aContextNode the node which is used to resolve namespaces
    * @param aFragment the string which is parsed to a DocumentFragment
+   * @param aWillOwnFragment is PR_TRUE if ownership of the fragment should be
+   *                         transferred to the caller.
    * @param aReturn [out] the created DocumentFragment
    */
   static nsresult CreateContextualFragment(nsIDOMNode* aContextNode,
                                            const nsAString& aFragment,
+                                           PRBool aWillOwnFragment,
                                            nsIDOMDocumentFragment** aReturn);
 
   /**
    * Creates a new XML document, which is marked to be loaded as data.
    *
    * @param aNamespaceURI Namespace for the root element to create and insert in
    *                      the document. Only used if aQualifiedName is not
    *                      empty.
--- a/content/base/src/nsContentUtils.cpp
+++ b/content/base/src/nsContentUtils.cpp
@@ -3349,16 +3349,17 @@ nsContentUtils::IsValidNodeName(nsIAtom 
   return aPrefix != nsGkAtoms::xmlns &&
          (aNamespaceID == kNameSpaceID_XML || aPrefix != nsGkAtoms::xml);
 }
 
 /* static */
 nsresult
 nsContentUtils::CreateContextualFragment(nsIDOMNode* aContextNode,
                                          const nsAString& aFragment,
+                                         PRBool aWillOwnFragment,
                                          nsIDOMDocumentFragment** aReturn)
 {
   NS_ENSURE_ARG(aContextNode);
   *aReturn = nsnull;
 
   nsresult rv;
   nsCOMPtr<nsINode> node = do_QueryInterface(aContextNode);
   NS_ENSURE_TRUE(node, NS_ERROR_NOT_AVAILABLE);
@@ -3494,17 +3495,17 @@ nsContentUtils::CreateContextualFragment
       NS_NOTREACHED("unknown mode");
       break;
   }
 
   // XXX Shouldn't we be returning rv if it's a failure code?
   rv = parser->ParseFragment(aFragment, nsnull, tagStack,
                              !bHTML, contentType, mode);
   if (NS_SUCCEEDED(rv)) {
-    rv = sink->GetFragment(aReturn);
+    rv = sink->GetFragment(aWillOwnFragment, aReturn);
   }
 
   document->SetFragmentParser(parser);
 
   return NS_OK;
 }
 
 /* static */
--- a/content/base/src/nsRange.cpp
+++ b/content/base/src/nsRange.cpp
@@ -1789,13 +1789,14 @@ nsRange::Detach()
 }
 
 // nsIDOMNSRange interface
 NS_IMETHODIMP    
 nsRange::CreateContextualFragment(const nsAString& aFragment,
                                   nsIDOMDocumentFragment** aReturn)
 {
   nsCOMPtr<nsIDOMNode> start = do_QueryInterface(mStartParent);
-  return
-    mIsPositioned
-    ? nsContentUtils::CreateContextualFragment(start, aFragment, aReturn)
-    : NS_ERROR_FAILURE;
+  if (mIsPositioned) {
+    return nsContentUtils::CreateContextualFragment(start, aFragment, PR_TRUE,
+                                                    aReturn);
+  }
+  return NS_ERROR_FAILURE;
 }
--- a/content/base/test/Makefile.in
+++ b/content/base/test/Makefile.in
@@ -170,16 +170,17 @@ include $(topsrcdir)/config/rules.mk
 		test_bug415860.html \
 		test_bug414190.html \
 		test_bug414796.html \
 		test_bug416383.html \
 		test_bug417255.html \
 		test_bug417384.html \
 		test_bug418214.html \
 		test_bug420609.xhtml \
+		test_bug420700.html \
 		$(NULL)
 
 libs:: $(_TEST_FILES)
 	$(INSTALL) $(foreach f,$^,"$f") $(DEPTH)/_tests/testing/mochitest/tests/$(relativesrcdir)
 
 check::
 	@$(EXIT_ON_ERROR) \
 	for f in $(subst .cpp,,$(CPP_UNIT_TESTS)); do \
new file mode 100644
--- /dev/null
+++ b/content/base/test/test_bug420700.html
@@ -0,0 +1,36 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=420700
+-->
+<head>
+  <title>Test for Bug 420700</title>
+  <script type="text/javascript" src="/MochiKit/MochiKit.js"></script>
+  <script type="text/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=420700">Mozilla Bug 420700</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+  
+</div>
+<pre id="test">
+<script class="testbody" type="text/javascript">
+
+  var r = document.createRange();
+  r.selectNode(document.documentElement);
+
+  var df = r.createContextualFragment("<p>BAD</p>");
+
+  var display = document.getElementById("display");
+  display.innerHTML = "<p>GOOD</p>";
+
+  var p = display.firstChild;
+
+  is(p.textContent, "GOOD", "createContextualFragment tests");
+</script>
+</pre>
+</body>
+</html>
+
--- a/content/html/content/src/nsGenericHTMLElement.cpp
+++ b/content/html/content/src/nsGenericHTMLElement.cpp
@@ -741,16 +741,17 @@ nsGenericHTMLElement::SetInnerHTML(const
     loader = doc->ScriptLoader();
     scripts_enabled = loader->GetEnabled();
     loader->SetEnabled(PR_FALSE);
   }
 
   nsCOMPtr<nsIDOMNode> thisNode(do_QueryInterface(static_cast<nsIContent *>
                                                              (this)));
   nsresult rv = nsContentUtils::CreateContextualFragment(thisNode, aInnerHTML,
+                                                         PR_FALSE,
                                                          getter_AddRefs(df));
   if (NS_SUCCEEDED(rv)) {
     nsCOMPtr<nsIDOMNode> tmpNode;
     rv = thisNode->AppendChild(df, getter_AddRefs(tmpNode));
   }
 
   if (scripts_enabled) {
     // If we disabled scripts, re-enable them now that we're
--- a/content/html/document/src/nsHTMLFragmentContentSink.cpp
+++ b/content/html/document/src/nsHTMLFragmentContentSink.cpp
@@ -114,17 +114,18 @@ public:
   NS_IMETHOD OpenContainer(const nsIParserNode& aNode);
   NS_IMETHOD CloseContainer(const nsHTMLTag aTag);
   NS_IMETHOD AddLeaf(const nsIParserNode& aNode);
   NS_IMETHOD AddComment(const nsIParserNode& aNode);
   NS_IMETHOD AddProcessingInstruction(const nsIParserNode& aNode);
   NS_IMETHOD AddDocTypeDecl(const nsIParserNode& aNode);
 
   // nsIFragmentContentSink
-  NS_IMETHOD GetFragment(nsIDOMDocumentFragment** aFragment);
+  NS_IMETHOD GetFragment(PRBool aWillOwnFragment,
+                         nsIDOMDocumentFragment** aFragment);
   NS_IMETHOD SetTargetDocument(nsIDocument* aDocument);
   NS_IMETHOD WillBuildContent();
   NS_IMETHOD DidBuildContent();
   NS_IMETHOD IgnoreFirstContainer();
 
   nsIContent* GetCurrentContent();
   PRInt32 PushContent(nsIContent *aContent);
   nsIContent* PopContent();
@@ -603,20 +604,25 @@ nsHTMLFragmentContentSink::AddProcessing
 
 NS_IMETHODIMP
 nsHTMLFragmentContentSink::AddDocTypeDecl(const nsIParserNode& aNode)
 {
   return NS_OK;
 }
 
 NS_IMETHODIMP
-nsHTMLFragmentContentSink::GetFragment(nsIDOMDocumentFragment** aFragment)
+nsHTMLFragmentContentSink::GetFragment(PRBool aWillOwnFragment,
+                                       nsIDOMDocumentFragment** aFragment)
 {
   if (mRoot) {
-    return CallQueryInterface(mRoot, aFragment);
+    nsresult rv = CallQueryInterface(mRoot, aFragment);
+    if (NS_SUCCEEDED(rv) && aWillOwnFragment) {
+      mRoot = nsnull;
+    }
+    return rv;
   }
 
   *aFragment = nsnull;
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
--- a/content/xml/document/src/nsXMLFragmentContentSink.cpp
+++ b/content/xml/document/src/nsXMLFragmentContentSink.cpp
@@ -97,17 +97,18 @@ public:
   NS_IMETHOD DidBuildModel();
   NS_IMETHOD SetDocumentCharset(nsACString& aCharset);
   virtual nsISupports *GetTarget();
   NS_IMETHOD DidProcessATokenImpl();
 
   // nsIXMLContentSink
 
   // nsIFragmentContentSink
-  NS_IMETHOD GetFragment(nsIDOMDocumentFragment** aFragment);
+  NS_IMETHOD GetFragment(PRBool aWillOwnFragment,
+                         nsIDOMDocumentFragment** aFragment);
   NS_IMETHOD SetTargetDocument(nsIDocument* aDocument);
   NS_IMETHOD WillBuildContent();
   NS_IMETHOD DidBuildContent();
   NS_IMETHOD IgnoreFirstContainer();
 
 protected:
   virtual PRBool SetDocElement(PRInt32 aNameSpaceID, 
                                nsIAtom *aTagName,
@@ -395,24 +396,29 @@ void
 nsXMLFragmentContentSink::StartLayout()
 {
   NS_NOTREACHED("fragments shouldn't layout");
 }
 
 ////////////////////////////////////////////////////////////////////////
 
 NS_IMETHODIMP 
-nsXMLFragmentContentSink::GetFragment(nsIDOMDocumentFragment** aFragment)
+nsXMLFragmentContentSink::GetFragment(PRBool aWillOwnFragment,
+                                      nsIDOMDocumentFragment** aFragment)
 {
   *aFragment = nsnull;
   if (mParseError) {
     //XXX PARSE_ERR from DOM3 Load and Save would be more appropriate
     return NS_ERROR_DOM_SYNTAX_ERR;
   } else if (mRoot) {
-    return CallQueryInterface(mRoot, aFragment);
+    nsresult rv = CallQueryInterface(mRoot, aFragment);
+    if (NS_SUCCEEDED(rv) && aWillOwnFragment) {
+      mRoot = nsnull;
+    }
+    return rv;
   } else {
     return NS_OK;
   }
 }
 
 NS_IMETHODIMP
 nsXMLFragmentContentSink::SetTargetDocument(nsIDocument* aTargetDocument)
 {
--- a/parser/htmlparser/public/nsIFragmentContentSink.h
+++ b/parser/htmlparser/public/nsIFragmentContentSink.h
@@ -38,35 +38,38 @@
 #define nsIFragmentContentSink_h___
 
 #include "nsISupports.h"
 
 class nsIDOMDocumentFragment;
 class nsIDocument;
 
 #define NS_I_FRAGMENT_CONTENT_SINK_IID \
-  { 0x2cec7263, 0x9dd0, 0x4413, \
-    { 0xb6, 0x68, 0x6f, 0xf0, 0xa1, 0x40, 0xc1, 0xbe } }
+  { 0x1ecdb30d, 0x1f10, 0x45d2, \
+    { 0xa4, 0xf4, 0xec, 0xbc, 0x03, 0x52, 0x9a, 0x7e } }
 
 /**
  * The fragment sink allows a client to parse a fragment of sink, possibly
  * surrounded in context. Also see nsIParser::ParseFragment().
  * Note: once you've parsed a fragment, the fragment sink must be re-set on
  * the parser in order to parse another fragment.
  */
 class nsIFragmentContentSink : public nsISupports {
 public:
   NS_DECLARE_STATIC_IID_ACCESSOR(NS_I_FRAGMENT_CONTENT_SINK_IID)
   /**
    * This method is used to obtain the fragment created by
    * a fragment content sink. The value returned will be null
    * if the content sink hasn't yet received parser notifications.
    *
+   * If aWillOwnFragment is PR_TRUE then the sink should drop its
+   * ownership of the fragment.
    */
-  NS_IMETHOD GetFragment(nsIDOMDocumentFragment** aFragment) = 0;
+  NS_IMETHOD GetFragment(PRBool aWillOwnFragment,
+                         nsIDOMDocumentFragment** aFragment) = 0;
 
   /**
    * This method is used to set the target document for this fragment
    * sink.  This document's nodeinfo manager will be used to create
    * the content objects.  This MUST be called before the sink is used.
    *
    * @param aDocument the document the new nodes will belong to
    * (should not be null)