Bug 1330051; Reparse style attribute when adopting across style backends; draft
authorManish Goregaokar <manishearth@gmail.com>
Fri, 24 Mar 2017 15:28:19 -0700
changeset 551243 0215a0acb67b30a31bf5ec09b71817a3254f9372
parent 504149 01d1dedf400d4be413b1a0d48090dca7acf29637
child 621492 1af0a8a67ac7f0eacb6d9f514e326e4604847cb3
push id50997
push userbmo:manishearth@gmail.com
push dateSat, 25 Mar 2017 07:02:14 +0000
bugs1330051
milestone55.0a1
Bug 1330051; Reparse style attribute when adopting across style backends; MozReview-Commit-ID: LWN57KApiMu
dom/base/Element.cpp
dom/base/Element.h
dom/base/nsStyledElement.cpp
dom/base/nsStyledElement.h
dom/html/HTMLImageElement.cpp
dom/html/nsGenericHTMLElement.cpp
dom/svg/nsSVGElement.cpp
dom/svg/nsSVGElement.h
layout/reftests/bugs/1330051-ref.svg
layout/reftests/bugs/1330051.svg
layout/reftests/bugs/reftest-stylo.list
layout/reftests/bugs/reftest.list
--- a/dom/base/Element.cpp
+++ b/dom/base/Element.cpp
@@ -1668,17 +1668,17 @@ Element::BindToTree(nsIDocument* aDocume
   if (HasID()) {
     AddToIdTable(DoGetID());
   }
 
   if (MayHaveStyle() && !IsXULElement()) {
     // XXXbz if we already have a style attr parsed, this won't do
     // anything... need to fix that.
     // If MayHaveStyle() is true, we must be an nsStyledElement
-    static_cast<nsStyledElement*>(this)->ReparseStyleAttribute(false);
+    static_cast<nsStyledElement*>(this)->ReparseStyleAttribute(false, false);
   }
 
   if (aDocument) {
     // If we're in a document now, let our mapped attrs know what their new
     // sheet is.  This is safe to run for non-mapped-attribute elements too;
     // it'll just do a small bit of unnecessary work.  But most elements in
     // practice are mapped-attribute elements.
     nsHTMLStyleSheet* sheet = aDocument->GetAttributeStyleSheet();
--- a/dom/base/Element.h
+++ b/dom/base/Element.h
@@ -1149,20 +1149,21 @@ public:
    */
   virtual BorrowedAttrInfo GetAttrInfo(int32_t aNamespaceID, nsIAtom* aName) const;
 
   /**
    * Called when we have been adopted, and the information of the
    * node has been changed.
    *
    * The new document can be reached via OwnerDoc().
+   *
+   * If you override this method,
+   * please call up to the parent NodeInfoChanged.
    */
-  virtual void NodeInfoChanged(nsIDocument* aOldDoc)
-  {
-  }
+  virtual void NodeInfoChanged(nsIDocument* aOldDoc) {}
 
   /**
    * Parse a string into an nsAttrValue for a CORS attribute.  This
    * never fails.  The resulting value is an enumerated value whose
    * GetEnumValue() returns one of the above constants.
    */
   static void ParseCORSValue(const nsAString& aValue, nsAttrValue& aResult);
 
--- a/dom/base/nsStyledElement.cpp
+++ b/dom/base/nsStyledElement.cpp
@@ -96,46 +96,55 @@ nsStyledElement::SetInlineStyleDeclarati
 
 nsICSSDeclaration*
 nsStyledElement::Style()
 {
   Element::nsDOMSlots *slots = DOMSlots();
 
   if (!slots->mStyle) {
     // Just in case...
-    ReparseStyleAttribute(true);
+    ReparseStyleAttribute(true, false);
 
     slots->mStyle = new nsDOMCSSAttributeDeclaration(this, false);
     SetMayHaveStyle();
   }
 
   return slots->mStyle;
 }
 
 nsresult
-nsStyledElement::ReparseStyleAttribute(bool aForceInDataDoc)
+nsStyledElement::ReparseStyleAttribute(bool aForceInDataDoc, bool aForceIfAlreadyParsed)
 {
   if (!MayHaveStyle()) {
     return NS_OK;
   }
   const nsAttrValue* oldVal = mAttrsAndChildren.GetAttr(nsGkAtoms::style);
-  if (oldVal && oldVal->Type() != nsAttrValue::eCSSDeclaration) {
+  if (oldVal && (aForceIfAlreadyParsed || oldVal->Type() != nsAttrValue::eCSSDeclaration)) {
     nsAttrValue attrValue;
     nsAutoString stringValue;
     oldVal->ToString(stringValue);
     ParseStyleAttribute(stringValue, attrValue, aForceInDataDoc);
     // Don't bother going through SetInlineStyleDeclaration; we don't
     // want to fire off mutation events or document notifications anyway
     nsresult rv = mAttrsAndChildren.SetAndSwapAttr(nsGkAtoms::style, attrValue);
     NS_ENSURE_SUCCESS(rv, rv);
   }
   
   return NS_OK;
 }
 
+void
+nsStyledElement::NodeInfoChanged(nsIDocument* aOldDoc)
+{
+  nsStyledElementBase::NodeInfoChanged(aOldDoc);
+  if (OwnerDoc()->GetStyleBackendType() != aOldDoc->GetStyleBackendType()) {
+    ReparseStyleAttribute(false, /* aForceIfAlreadyParsed */ true);
+  }
+}
+
 nsICSSDeclaration*
 nsStyledElement::GetExistingStyle()
 {
   Element::nsDOMSlots* slots = GetExistingDOMSlots();
   if (!slots) {
     return nullptr;
   }
 
--- a/dom/base/nsStyledElement.h
+++ b/dom/base/nsStyledElement.h
@@ -71,15 +71,18 @@ protected:
                                 const nsAString& aValue, nsAttrValue& aResult) override;
 
   friend class mozilla::dom::Element;
 
   /**
    * 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.
+   * document. If aForceIfAlreadyParsed is set, this will always reparse even
+   * if the value has already been parsed.
    */
-  nsresult  ReparseStyleAttribute(bool aForceInDataDoc);
+  nsresult ReparseStyleAttribute(bool aForceInDataDoc, bool aForceIfAlreadyParsed);
+
+  virtual void NodeInfoChanged(nsIDocument* aOldDoc) override;
 };
 
 NS_DEFINE_STATIC_IID_ACCESSOR(nsStyledElement, NS_STYLED_ELEMENT_IID)
 #endif // __NS_STYLEDELEMENT_H_
--- a/dom/html/HTMLImageElement.cpp
+++ b/dom/html/HTMLImageElement.cpp
@@ -707,16 +707,17 @@ HTMLImageElement::IntrinsicState() const
 {
   return nsGenericHTMLElement::IntrinsicState() |
     nsImageLoadingContent::ImageState();
 }
 
 void
 HTMLImageElement::NodeInfoChanged(nsIDocument* aOldDoc)
 {
+  nsGenericHTMLElement::NodeInfoChanged(aOldDoc);
   // Resetting the last selected source if adoption steps are run.
   mLastSelectedSource = nullptr;
 }
 
 // static
 already_AddRefed<HTMLImageElement>
 HTMLImageElement::Image(const GlobalObject& aGlobal,
                         const Optional<uint32_t>& aWidth,
--- a/dom/html/nsGenericHTMLElement.cpp
+++ b/dom/html/nsGenericHTMLElement.cpp
@@ -2836,16 +2836,17 @@ nsGenericHTMLFormElementWithState::Resto
   }
 
   return false;
 }
 
 void
 nsGenericHTMLFormElementWithState::NodeInfoChanged(nsIDocument* aOldDoc)
 {
+  nsGenericHTMLElement::NodeInfoChanged(aOldDoc);
   mStateKey.SetIsVoid(true);
 }
 
 nsSize
 nsGenericHTMLElement::GetWidthHeightForImage(RefPtr<imgRequestProxy>& aImageRequest)
 {
   nsSize size(0,0);
 
--- a/dom/svg/nsSVGElement.cpp
+++ b/dom/svg/nsSVGElement.cpp
@@ -904,16 +904,17 @@ bool
 nsSVGElement::IsNodeOfType(uint32_t aFlags) const
 {
   return !(aFlags & ~eCONTENT);
 }
 
 void
 nsSVGElement::NodeInfoChanged(nsIDocument* aOldDoc)
 {
+  nsSVGElementBase::NodeInfoChanged(aOldDoc);
   aOldDoc->UnscheduleSVGForPresAttrEvaluation(this);
   mContentDeclarationBlock = nullptr;
   OwnerDoc()->ScheduleSVGForPresAttrEvaluation(this);
 }
 
 NS_IMETHODIMP
 nsSVGElement::WalkContentStyleRules(nsRuleWalker* aRuleWalker)
 {
--- a/dom/svg/nsSVGElement.h
+++ b/dom/svg/nsSVGElement.h
@@ -113,17 +113,17 @@ public:
                                               int32_t aModType) const override;
 
   virtual bool IsNodeOfType(uint32_t aFlags) const override;
 
   /**
    * We override the default to unschedule computation of Servo declaration blocks
    * when adopted across documents.
    */
-  virtual void NodeInfoChanged(nsIDocument* aOldDoc) override;
+  virtual void NodeInfoChanged(nsIDocument* aOldDoc) final;
 
   NS_IMETHOD WalkContentStyleRules(nsRuleWalker* aRuleWalker) override;
   void WalkAnimatedContentStyleRules(nsRuleWalker* aRuleWalker);
 
   NS_IMETHOD_(bool) IsAttributeMapped(const nsIAtom* aAttribute) const override;
 
   static const MappedAttributeEntry sFillStrokeMap[];
   static const MappedAttributeEntry sGraphicsMap[];
new file mode 100644
--- /dev/null
+++ b/layout/reftests/bugs/1330051-ref.svg
@@ -0,0 +1,17 @@
+<svg xmlns="http://www.w3.org/2000/svg" width="500" height="500" class="reftest-wait">
+  <foreignObject width="500" height="500">
+    <iframe xmlns="http://www.w3.org/1999/xhtml"
+            srcdoc="A test">
+    </iframe>
+  </foreignObject>
+  <script>
+    frames[0].onload = function() {
+      var doc = frames[0].document;
+      let el = doc.createElement('div');
+      el.innerHTML = "This should remain green";
+      el.style.color = "green";
+      doc.body.appendChild(el);
+      document.documentElement.removeAttribute("class");
+    }
+  </script>
+</svg>
\ No newline at end of file
new file mode 100644
--- /dev/null
+++ b/layout/reftests/bugs/1330051.svg
@@ -0,0 +1,17 @@
+<svg xmlns="http://www.w3.org/2000/svg" width="500" height="500" class="reftest-wait">
+  <foreignObject width="500" height="500">
+    <iframe xmlns="http://www.w3.org/1999/xhtml"
+            srcdoc="A test">
+    </iframe>
+    <div xmlns="http://www.w3.org/1999/xhtml"
+         style="color: green" id="myDiv">This should remain green</div>
+  </foreignObject>
+  <script>
+    let el = document.getElementById('myDiv');
+    frames[0].onload = function() {
+      var doc = frames[0].document;
+      doc.body.appendChild(el);
+      document.documentElement.removeAttribute("class");
+    }
+  </script>
+</svg>
\ No newline at end of file
--- a/layout/reftests/bugs/reftest-stylo.list
+++ b/layout/reftests/bugs/reftest-stylo.list
@@ -1983,10 +1983,11 @@ fails == 1316719-1a.html 1316719-1a.html
 fails == 1316719-1b.html 1316719-1b.html
 fails == 1316719-1c.html 1316719-1c.html
 
 HTTP == 652991-1a.html 652991-1a.html
 HTTP == 652991-1b.html 652991-1b.html
 HTTP == 652991-2.html 652991-2.html
 HTTP == 652991-3.html 652991-3.html
 fails HTTP == 652991-4.html 652991-4.html
+== 1330051.svg 1330051.svg
 
 
--- a/layout/reftests/bugs/reftest.list
+++ b/layout/reftests/bugs/reftest.list
@@ -1984,8 +1984,9 @@ fuzzy-if(Android,27,874) fuzzy-if(gtkWid
 fuzzy(2,320000) == 1315113-1.html 1315113-1-ref.html
 fuzzy(2,20000) == 1315113-2.html 1315113-2-ref.html
 == 1315632-1.html 1315632-1-ref.html
 fuzzy(2,40000) == 1316719-1a.html 1316719-1-ref.html
 fuzzy(2,40000) == 1316719-1b.html 1316719-1-ref.html
 fuzzy(2,40000) == 1316719-1c.html 1316719-1-ref.html
 skip-if(Android) != 1318769-1.html 1318769-1-ref.html
 == 1322512-1.html 1322512-1-ref.html
+== 1330051.svg 1330051-ref.svg