Don't parse "style" attributes in data documents unless someone asks for .style. Bug 418214, r+sr=peterv, a=schrep
authorbzbarsky@mit.edu
Tue, 19 Feb 2008 09:52:00 -0800
changeset 11871 1b077683d70b14688c48150d452d9bf84f4f38ad
parent 11870 ad0c45d553ef4859dcc56343d1cc33c32c7a0e45
child 11872 27ca28934b283b6102e9f14ef1eebfe9b3a79ae5
push id1
push userroot
push dateMon, 20 Oct 2014 17:29:22 +0000
reviewersschrep
bugs418214
milestone1.9b4pre
Don't parse "style" attributes in data documents unless someone asks for .style. Bug 418214, r+sr=peterv, a=schrep
content/base/src/nsStyledElement.cpp
content/base/src/nsStyledElement.h
content/base/test/Makefile.in
content/base/test/test_bug418214.html
content/svg/content/src/nsSVGElement.cpp
content/xul/content/src/nsXULElement.cpp
--- a/content/base/src/nsStyledElement.cpp
+++ b/content/base/src/nsStyledElement.cpp
@@ -81,17 +81,17 @@ nsStyledElement::GetClasses() const
 
 PRBool
 nsStyledElement::ParseAttribute(PRInt32 aNamespaceID, nsIAtom* aAttribute,
                                 const nsAString& aValue, nsAttrValue& aResult)
 {
   if (aNamespaceID == kNameSpaceID_None) {
     if (aAttribute == nsGkAtoms::style) {
       SetFlags(NODE_MAY_HAVE_STYLE);
-      ParseStyleAttribute(this, aValue, aResult);
+      ParseStyleAttribute(this, aValue, aResult, PR_FALSE);
       return PR_TRUE;
     }
     if (aAttribute == nsGkAtoms::_class) {
       SetFlags(NODE_MAY_HAVE_CLASS);
 #ifdef MOZ_SVG
       NS_ASSERTION(!nsCOMPtr<nsIDOMSVGStylable>(do_QueryInterface(this)),
                    "SVG code should have handled this 'class' attribute!");
 #endif
@@ -160,17 +160,17 @@ nsStyledElement::BindToTree(nsIDocument*
 {
   nsresult rv = nsStyledElementBase::BindToTree(aDocument, aParent,
                                                 aBindingParent,
                                                 aCompileEventHandlers);
   NS_ENSURE_SUCCESS(rv, rv);
 
   // XXXbz if we already have a style attr parsed, this won't do
   // anything... need to fix that.
-  ReparseStyleAttribute();
+  ReparseStyleAttribute(PR_FALSE);
 
   return rv;
 }
 
 // ---------------------------------------------------------------
 // Others and helpers
 
 static nsICSSOMFactory* gCSSOMFactory = nsnull;
@@ -179,17 +179,17 @@ static NS_DEFINE_CID(kCSSOMFactoryCID, N
 nsresult
 nsStyledElement::GetStyle(nsIDOMCSSStyleDeclaration** aStyle)
 {
   nsGenericElement::nsDOMSlots *slots = GetDOMSlots();
   NS_ENSURE_TRUE(slots, NS_ERROR_OUT_OF_MEMORY);
 
   if (!slots->mStyle) {
     // Just in case...
-    ReparseStyleAttribute();
+    ReparseStyleAttribute(PR_TRUE);
 
     nsresult rv;
     if (!gCSSOMFactory) {
       rv = CallGetService(kCSSOMFactoryCID, &gCSSOMFactory);
       NS_ENSURE_SUCCESS(rv, rv);
     }
 
     rv = gCSSOMFactory->CreateDOMCSSAttributeDeclaration(this,
@@ -199,46 +199,47 @@ nsStyledElement::GetStyle(nsIDOMCSSStyle
   }
 
   // Why bother with QI?
   NS_ADDREF(*aStyle = slots->mStyle);
   return NS_OK;
 }
 
 nsresult
-nsStyledElement::ReparseStyleAttribute()
+nsStyledElement::ReparseStyleAttribute(PRBool aForceInDataDoc)
 {
   if (!HasFlag(NODE_MAY_HAVE_STYLE)) {
     return NS_OK;
   }
   const nsAttrValue* oldVal = mAttrsAndChildren.GetAttr(nsGkAtoms::style);
   
   if (oldVal && oldVal->Type() != nsAttrValue::eCSSStyleRule) {
     nsAttrValue attrValue;
     nsAutoString stringValue;
     oldVal->ToString(stringValue);
-    ParseStyleAttribute(this, stringValue, attrValue);
+    ParseStyleAttribute(this, stringValue, attrValue, aForceInDataDoc);
     // Don't bother going through SetInlineStyleRule, we don't want to fire off
     // mutation events or document notifications anyway
     nsresult rv = mAttrsAndChildren.SetAndTakeAttr(nsGkAtoms::style, attrValue);
     NS_ENSURE_SUCCESS(rv, rv);
   }
   
   return NS_OK;
 }
 
 void
 nsStyledElement::ParseStyleAttribute(nsIContent* aContent,
                                      const nsAString& aValue,
-                                     nsAttrValue& aResult)
+                                     nsAttrValue& aResult,
+                                     PRBool aForceInDataDoc)
 {
   nsresult result = NS_OK;
   nsIDocument* doc = aContent->GetOwnerDoc();
 
-  if (doc) {
+  if (doc && (aForceInDataDoc || !doc->IsLoadedAsData())) {
     PRBool isCSS = PR_TRUE; // assume CSS until proven otherwise
 
     if (!aContent->IsNativeAnonymous()) {  // native anonymous content
                                            // always assumes CSS
       nsAutoString styleType;
       doc->GetHeaderData(nsGkAtoms::headerContentStyleType, styleType);
       if (!styleType.IsEmpty()) {
         static const char textCssStr[] = "text/css";
--- a/content/base/src/nsStyledElement.h
+++ b/content/base/src/nsStyledElement.h
@@ -72,36 +72,39 @@ public:
   virtual nsICSSStyleRule* GetInlineStyleRule();
   NS_IMETHOD SetInlineStyleRule(nsICSSStyleRule* aStyleRule, PRBool aNotify);
 
   virtual nsresult BindToTree(nsIDocument* aDocument, nsIContent* aParent,
                               nsIContent* aBindingParent,
                               PRBool aCompileEventHandlers);
 
   /**
-   * Create the style struct from the style attr.  Used when an element is first
-   * put into a document.  Only has an effect if the old value is a string.
-   */
-  nsresult  ReparseStyleAttribute(void);
-  /**
    * Parse a style attr value into a CSS rulestruct (or, if there is no
    * document, leave it as a string) and return as nsAttrValue.
    * Note: this function is used by other classes than nsStyledElement
    *
    * @param aValue the value to parse
    * @param aResult the resulting HTMLValue [OUT]
    */
   static void ParseStyleAttribute(nsIContent* aContent,
                                   const nsAString& aValue,
-                                  nsAttrValue& aResult);
+                                  nsAttrValue& aResult,
+                                  PRBool aForceInDataDoc);
 
   static void Shutdown();
   
 protected:
 
   virtual PRBool ParseAttribute(PRInt32 aNamespaceID, nsIAtom* aAttribute,
                                 const nsAString& aValue, nsAttrValue& aResult);
 
   nsresult GetStyle(nsIDOMCSSStyleDeclaration** aStyle);
 
+  /**
+   * Create the style struct from the style attr.  Used when an element is
+   * first put into a document.  Only has an effect if the old value is a
+   * string.  If aForceInDataDoc is true, will reparse even if we're in a data
+   * document.
+   */
+  nsresult  ReparseStyleAttribute(PRBool aForceInDataDoc);
 };
 
 #endif // __NS_STYLEDELEMENT_H_
--- a/content/base/test/Makefile.in
+++ b/content/base/test/Makefile.in
@@ -139,12 +139,13 @@ include $(topsrcdir)/config/rules.mk
 		test_bug405182.html \
 		test_bug403841.html \
 		test_bug410229.html \
 		test_bug415860.html \
 		test_bug414190.html \
 		test_bug414796.html \
 		test_bug416383.html \
 		test_bug417384.html \
+		test_bug418214.html \
 		$(NULL)
 
 libs:: $(_TEST_FILES)
 	$(INSTALL) $(foreach f,$^,"$f") $(DEPTH)/_tests/testing/mochitest/tests/$(relativesrcdir)
new file mode 100644
--- /dev/null
+++ b/content/base/test/test_bug418214.html
@@ -0,0 +1,84 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=418214
+-->
+<head>
+  <title>Test for Bug 418214</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=418214">Mozilla Bug 418214</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+  
+</div>
+<pre id="test">
+<script class="testbody" type="text/javascript">
+
+var str = '<root xmlns:html="http://www.w3.org/1999/xhtml" xmlns:svg="http://www.w3.org/2000/svg" xmlns:math="http://www.w3.org/1998/Math/MathML"><html:div id="d" style="border:: invalid"/><svg:svg id="s" style="border:: invalid"/><math:math id="m" style="border:: invalid"/></root>';
+
+/** Test for Bug 418214 **/
+var doc = (new DOMParser()).parseFromString(str, "text/xml");
+var d = doc.getElementById("d");
+var s = doc.getElementById("s");
+var m = doc.getElementById("m");
+
+is(d.getAttribute("style"), "border:: invalid",
+   "Shouldn't be parsing style on HTML in data documents");
+is(s.getAttribute("style"), "border:: invalid",
+   "Shouldn't be parsing style on SVG in data documents");
+is(m.getAttribute("style"), "border:: invalid",
+   "Shouldn't be parsing style on MathML in data documents");
+
+var d2 = d.cloneNode(true);
+var s2 = s.cloneNode(true);
+var m2 = m.cloneNode(true);
+
+is(d2.getAttribute("style"), "border:: invalid",
+   "Shouldn't be parsing style on HTML on clone");
+is(s2.getAttribute("style"), "border:: invalid",
+   "Shouldn't be parsing style on SVG on clone");
+is(m2.getAttribute("style"), "border:: invalid",
+   "Shouldn't be parsing style on MathML on clone");
+
+d2.style;
+s2.style;
+m2.style;
+   
+is(d2.getAttribute("style"), "",
+   "Getting .style should parse style on HTML");
+is(s2.getAttribute("style"), "",
+   "Getting .style should parse style on SVG");
+is(m2.getAttribute("style"), "border:: invalid",
+   "Getting .style shouldn't parse style on MathML");
+
+d = document.adoptNode(d);
+s = document.adoptNode(s);
+m = document.adoptNode(m);
+
+is(d.getAttribute("style"), "border:: invalid",
+   "Adopting should not parse style on HTML");
+is(s.getAttribute("style"), "border:: invalid",
+   "Adopting should not parse style on SVG");
+is(m.getAttribute("style"), "border:: invalid",
+   "Adopting should not parse style on MathML");
+
+$("display").appendChild(d);
+$("display").appendChild(s);
+$("display").appendChild(m);
+
+is(d.getAttribute("style"), "",
+   "Insertion should parse style on HTML");
+is(s.getAttribute("style"), "",
+   "Insertion should parse style on SVG");
+is(m.getAttribute("style"), "",
+   "Insertion should parse style on MathML");
+
+</script>
+</pre>
+</body>
+</html>
+
--- a/content/svg/content/src/nsSVGElement.cpp
+++ b/content/svg/content/src/nsSVGElement.cpp
@@ -190,17 +190,18 @@ nsSVGElement::BindToTree(nsIDocument* aD
   const nsAttrValue* oldVal = mAttrsAndChildren.GetAttr(nsGkAtoms::style);
 
   if (oldVal && oldVal->Type() == nsAttrValue::eCSSStyleRule) {
     // we need to force a reparse because the baseURI of the document
     // may have changed
     nsAttrValue attrValue;
     nsAutoString stringValue;
     oldVal->ToString(stringValue);
-    ParseStyleAttribute(this, stringValue, attrValue);
+    // Force in data doc, since we already have a style rule
+    ParseStyleAttribute(this, stringValue, attrValue, PR_TRUE);
     // Don't bother going through SetInlineStyleRule, we don't want to fire off
     // mutation events or document notifications anyway
     rv = mAttrsAndChildren.SetAndTakeAttr(nsGkAtoms::style, attrValue);
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
   return NS_OK;
 }
--- a/content/xul/content/src/nsXULElement.cpp
+++ b/content/xul/content/src/nsXULElement.cpp
@@ -1083,17 +1083,17 @@ nsXULElement::ParseAttribute(PRInt32 aNa
     // Parse into a nsAttrValue
 
     // WARNING!!
     // This code is largely duplicated in nsXULPrototypeElement::SetAttrAt.
     // Any changes should be made to both functions.
     if (aNamespaceID == kNameSpaceID_None) {
         if (aAttribute == nsGkAtoms::style) {
             SetFlags(NODE_MAY_HAVE_STYLE);
-            nsStyledElement::ParseStyleAttribute(this, aValue, aResult);
+            nsStyledElement::ParseStyleAttribute(this, aValue, aResult, PR_FALSE);
             return PR_TRUE;
         }
 
         if (aAttribute == nsGkAtoms::_class) {
             SetFlags(NODE_MAY_HAVE_CLASS);
             aResult.ParseAtomArray(aValue);
             return PR_TRUE;
         }