644728 - nsGenericHTMLElement::SetInnerHTML() will silently fail if target node's owner doc has no document URI

Status: NEW

(Core :: DOM: HTML Parser, defect, P5)

Description

[Seth Spitzer]

nsGenericHTMLElement::SetInnerHTML() will silently fail if target node's owner doc has no document URI

cc'ing Henri as he touched this code in bug #599588 and jonas because he reviewed it and may have thoughts.

Note that before Henri changed this code we could have silently failed before, but what I am doing used to work with the old parser.

some links to code:

http://mxr.mozilla.org/mozilla-central/source/content/html/content/src/nsGenericHTMLElement.cpp#766

766     asFragmentParser->ParseHtml5Fragment(aInnerHTML,
767                                          this,
768                                          Tag(),
769                                          GetNameSpaceID(),
770                                          doc->GetCompatibilityMode() ==
771                                              eCompatibility_NavQuirks,
772                                          PR_TRUE);


http://mxr.mozilla.org/mozilla-central/source/parser/html/nsHtml5Parser.cpp#485

485 nsHtml5Parser::ParseHtml5Fragment(const nsAString& aSourceBuffer,
486                                   nsIContent* aTargetNode,
487                                   nsIAtom* aContextLocalName,
488                                   PRInt32 aContextNamespace,
489                                   PRBool aQuirks,
490                                   PRBool aPreventScriptExecution)
491 {
492   nsIDocument* doc = aTargetNode->GetOwnerDoc();
493   NS_ENSURE_TRUE(doc, NS_ERROR_NOT_AVAILABLE);
494   
495   nsIURI* uri = doc->GetDocumentURI();
496   NS_ENSURE_TRUE(uri, NS_ERROR_NOT_AVAILABLE);

My abridged call stack:

#0  nsHtml5Parser::ParseHtml5Fragment (this=0x132894ee0,
aSourceBuffer=@0x7fff5fbf93c0, aTargetNode=0x132fd0750,
aContextLocalName=0x100c44480, aContextNamespace=3, aQuirks=1,
aPreventScriptExecution=1) at
/Users/sspitzerpostbox/fp/pb3/src/mozilla/parser/html/nsHtml5Parser.cpp:496
#1  0x000000011404a134 in nsGenericHTMLElement::SetInnerHTML (this=0x132fd0750,
aInnerHTML=@0x7fff5fbf93c0) at
/Users/sspitzerpostbox/fp/pb3/src/mozilla/content/html/content/src/nsGenericHTMLElement.cpp:772
#2  0x000000011404ccf0 in nsGenericHTMLElementTearoff::SetInnerHTML
(this=0x132fd07c0, aInnerHTML=@0x7fff5fbf93c0) at
/Users/sspitzerpostbox/fp/pb3/src/mozilla/content/html/content/src/nsGenericHTMLElement.cpp:251

<snip>

Should nsGenericHTMLElement::SetInnerHTML() return the error (NS_ERROR_NOT_AVAILABLE) to the caller in this scenario?

Comment 1

[Seth Spitzer]

To fix the problem I'm having, I did this in my code:

             nsCOMPtr<nsIDOMDocument> ownerDocument;
             rv = outNode->GetOwnerDocument(getter_AddRefs(ownerDocument));
             NS_ENSURE_SUCCESS(rv, rv);
 
+            // set the document uri
+            nsCOMPtr<nsIDocument> doc = do_QueryInterface(ownerDocument, &rv);
+            NS_ENSURE_SUCCESS(rv, rv);
+
+            nsCOMPtr<nsIURI> uri;
+            NS_NewURI(getter_AddRefs(uri), NS_LITERAL_CSTRING("about:blank"));
+            NS_ENSURE_TRUE(uri, NS_ERROR_UNEXPECTED);
+            doc->SetDocumentURI(uri);

             nsCOMPtr<nsIDOMElement> divElement;
             rv = ownerDocument->CreateElement(NS_LITERAL_STRING("div"), getter_AddRefs(divElement));
             NS_ENSURE_SUCCESS(rv, rv);

             nsCOMPtr<nsIDOMNSHTMLElement> divNSElement = do_QueryInterface(divElement, &rv);
             NS_ENSURE_SUCCESS(rv, rv);
              
             rv = divNSElement->SetInnerHTML(innerHTML);
             NS_ENSURE_SUCCESS(rv, rv);

Comment 2

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Henri, why does the parser care about a URI at all here?

Comment 3

[Henri Sivonen (:hsivonen)]

(In reply to comment #2)
> Henri, why does the parser care about a URI at all here?

It cares because nsContentSink::Init wants a URI.

Comment 4

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Can we change that? Or make it possible to use null as URI?

Comment 5

[Jens Stutte [:jstutte]]

Bulk-downgrade of unassigned, >=5 years untouched DOM/Storage bugs' priority.

If you have reason to believe this is wrong (especially for the severity), please write a comment and ni :jstutte.