Bug 1357645 - Clone attributes rather than reparsing when possible draft
authorKirk Steuber <ksteuber@mozilla.com>
Thu, 15 Jun 2017 09:56:18 -0700
changeset 595690 f70d370f8572f56c28b484b4652f12d7f0f502de
parent 594702 035c25bef7b5e4175006e63eff10c61c2eef73f1
child 595848 e93bd80bd0236dbe870c321630446a8141a65774
child 596718 43f0d5326124a7fe5370eeb7db9eeaa2fee2b325
push id64417
push userksteuber@mozilla.com
push dateFri, 16 Jun 2017 17:49:24 +0000
bugs1357645, 1365092
milestone56.0a1
Bug 1357645 - Clone attributes rather than reparsing when possible Now that the side effects of parsing have been relocated to BeforeSetAttr and AfterSetAttr (Bug 1365092), we can easily switch nsGenericHTMLElement::CopyInnerTo from reparsing attributes to cloning them. They are still reparsed, however, in the case where the owning document is changing since Base URIs may have changed. MozReview-Commit-ID: 2TlUUyBx6bL
dom/html/nsGenericHTMLElement.cpp
--- a/dom/html/nsGenericHTMLElement.cpp
+++ b/dom/html/nsGenericHTMLElement.cpp
@@ -172,47 +172,60 @@ NS_INTERFACE_MAP_BEGIN(nsGenericHTMLElem
   NS_INTERFACE_MAP_ENTRY(nsIDOMHTMLElement)
   NS_INTERFACE_MAP_ENTRY(nsIDOMElement)
   NS_INTERFACE_MAP_ENTRY(nsIDOMNode)
 NS_INTERFACE_MAP_END_INHERITING(nsGenericHTMLElementBase)
 
 nsresult
 nsGenericHTMLElement::CopyInnerTo(Element* aDst, bool aPreallocateChildren)
 {
+  MOZ_ASSERT(!aDst->GetUncomposedDoc(),
+             "Should not CopyInnerTo an Element in a document");
   nsresult rv;
 
+  bool reparse = (aDst->OwnerDoc() != OwnerDoc());
+
   rv = static_cast<nsGenericHTMLElement*>(aDst)->mAttrsAndChildren.
        EnsureCapacityToClone(mAttrsAndChildren, aPreallocateChildren);
   NS_ENSURE_SUCCESS(rv, rv);
 
   int32_t i, count = GetAttrCount();
   for (i = 0; i < count; ++i) {
     const nsAttrName *name = mAttrsAndChildren.AttrNameAt(i);
     const nsAttrValue *value = mAttrsAndChildren.AttrAt(i);
 
-    nsAutoString valStr;
-    value->ToString(valStr);
-
     if (name->Equals(nsGkAtoms::style, kNameSpaceID_None) &&
         value->Type() == nsAttrValue::eCSSDeclaration) {
+      // We still clone CSS attributes, even in the cross-document case.
+      // https://github.com/w3c/webappsec-csp/issues/212
+
+      nsAutoString valStr;
+      value->ToString(valStr);
+
       DeclarationBlock* decl = value->GetCSSDeclarationValue();
       // We can't just set this as a string, because that will fail
       // to reparse the string into style data until the node is
       // inserted into the document.  Clone the Rule instead.
       RefPtr<DeclarationBlock> declClone = decl->Clone();
 
       rv = aDst->SetInlineStyleDeclaration(declClone, &valStr, false);
       NS_ENSURE_SUCCESS(rv, rv);
-
-      continue;
+    } else if (reparse) {
+      nsAutoString valStr;
+      value->ToString(valStr);
+
+      rv = aDst->SetAttr(name->NamespaceID(), name->LocalName(),
+                         name->GetPrefix(), valStr, false);
+      NS_ENSURE_SUCCESS(rv, rv);
+    } else {
+      nsAttrValue valueCopy(*value);
+      rv = aDst->SetParsedAttr(name->NamespaceID(), name->LocalName(),
+                               name->GetPrefix(), valueCopy, false);
+      NS_ENSURE_SUCCESS(rv, rv);
     }
-
-    rv = aDst->SetAttr(name->NamespaceID(), name->LocalName(),
-                       name->GetPrefix(), valStr, false);
-    NS_ENSURE_SUCCESS(rv, rv);
   }
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsGenericHTMLElement::GetDataset(nsISupports** aDataset)
 {