Bug 1203973 - Move <style> and <link> attribute change handling to AfterSetAttr so that it doesn't trigger for no-op attribute changes. r=smaug

dom/html/HTMLLinkElement.cpp

@@ -326,96 +326,77 @@ HTMLLinkElement::UpdatePreconnect()
 }
 
 nsresult
-HTMLLinkElement::SetAttr(int32_t aNameSpaceID, nsIAtom* aName,
+HTMLLinkElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName,
-                         nsIAtom* aPrefix, const nsAString& aValue,
+                              const nsAttrValue* aValue, bool aNotify)
-                         bool aNotify)
 {
-  nsresult rv = nsGenericHTMLElement::SetAttr(aNameSpaceID, aName, aPrefix,
+  // It's safe to call ResetLinkState here because our new attr value has
-                                              aValue, aNotify);
+  // already been set or unset.  ResetLinkState needs the updated attribute
-
+  // value because notifying the document that content states have changed will
-  // The ordering of the parent class's SetAttr call and Link::ResetLinkState
+  // call IntrinsicState, which will try to get updated information about the
-  // is important here!  The attribute is not set until SetAttr returns, and
+  // visitedness from Link.
-  // we will need the updated attribute value because notifying the document
-  // that content states have changed will call IntrinsicState, which will try
-  // to get updated information about the visitedness from Link.
   if (aName == nsGkAtoms::href && kNameSpaceID_None == aNameSpaceID) {
-    Link::ResetLinkState(!!aNotify, true);
+    bool hasHref = aValue;
+    Link::ResetLinkState(!!aNotify, hasHref);
     if (IsInUncomposedDoc()) {
       CreateAndDispatchEvent(OwnerDoc(), NS_LITERAL_STRING("DOMLinkChanged"));
     }
   }
 
-  if (NS_SUCCEEDED(rv) && aNameSpaceID == kNameSpaceID_None &&
+  if (aValue) {
-      (aName == nsGkAtoms::href ||
+    if (aNameSpaceID == kNameSpaceID_None &&
-       aName == nsGkAtoms::rel ||
+        (aName == nsGkAtoms::href ||
-       aName == nsGkAtoms::title ||
+         aName == nsGkAtoms::rel ||
-       aName == nsGkAtoms::media ||
+         aName == nsGkAtoms::title ||
-       aName == nsGkAtoms::type)) {
+         aName == nsGkAtoms::media ||
-    bool dropSheet = false;
+         aName == nsGkAtoms::type)) {
-    if (aName == nsGkAtoms::rel) {
+      bool dropSheet = false;
-      uint32_t linkTypes = nsStyleLinkElement::ParseLinkTypes(aValue,
+      if (aName == nsGkAtoms::rel) {
-                                                              NodePrincipal());
+        nsAutoString value;
-      if (GetSheet()) {
+        aValue->ToString(value);
-        dropSheet = !(linkTypes & nsStyleLinkElement::eSTYLESHEET);
+        uint32_t linkTypes = nsStyleLinkElement::ParseLinkTypes(value,
-      } else if (linkTypes & eHTMLIMPORT) {
+                                                                NodePrincipal());
+        if (GetSheet()) {
+          dropSheet = !(linkTypes & nsStyleLinkElement::eSTYLESHEET);
+        } else if (linkTypes & eHTMLIMPORT) {
+          UpdateImport();
+        } else if ((linkTypes & ePRECONNECT) && IsInComposedDoc()) {
+          UpdatePreconnect();
+        }
+      }
+
+      if (aName == nsGkAtoms::href) {
+        UpdateImport();
+        if (IsInComposedDoc()) {
+          UpdatePreconnect();
+        }
+      }
+
+      UpdateStyleSheetInternal(nullptr, nullptr,
+                               dropSheet ||
+                               (aName == nsGkAtoms::title ||
+                                aName == nsGkAtoms::media ||
+                                aName == nsGkAtoms::type));
+    }
+  } else {
+    // Since removing href or rel makes us no longer link to a
+    // stylesheet, force updates for those too.
+    if (aNameSpaceID == kNameSpaceID_None) {
+      if (aName == nsGkAtoms::href ||
+          aName == nsGkAtoms::rel ||
+          aName == nsGkAtoms::title ||
+          aName == nsGkAtoms::media ||
+          aName == nsGkAtoms::type) {
+        UpdateStyleSheetInternal(nullptr, nullptr, true);
+      }
+      if (aName == nsGkAtoms::href ||
+          aName == nsGkAtoms::rel) {
         UpdateImport();
-      } else if ((linkTypes & ePRECONNECT) && IsInComposedDoc()) {
-        UpdatePreconnect();
       }
     }
-
-    if (aName == nsGkAtoms::href) {
-      UpdateImport();
-      if (IsInComposedDoc()) {
-        UpdatePreconnect();
-      }
-    }
-    
-    UpdateStyleSheetInternal(nullptr, nullptr,
-                             dropSheet ||
-                             (aName == nsGkAtoms::title ||
-                              aName == nsGkAtoms::media ||
-                              aName == nsGkAtoms::type));
   }
 
-  return rv;
+  return nsGenericHTMLElement::AfterSetAttr(aNameSpaceID, aName, aValue,
-}
+                                            aNotify);
-
-nsresult
-HTMLLinkElement::UnsetAttr(int32_t aNameSpaceID, nsIAtom* aAttribute,
-                           bool aNotify)
-{
-  nsresult rv = nsGenericHTMLElement::UnsetAttr(aNameSpaceID, aAttribute,
-                                                aNotify);
-  // Since removing href or rel makes us no longer link to a
-  // stylesheet, force updates for those too.
-  if (NS_SUCCEEDED(rv) && aNameSpaceID == kNameSpaceID_None) {
-    if (aAttribute == nsGkAtoms::href ||
-        aAttribute == nsGkAtoms::rel ||
-        aAttribute == nsGkAtoms::title ||
-        aAttribute == nsGkAtoms::media ||
-        aAttribute == nsGkAtoms::type) {
-      UpdateStyleSheetInternal(nullptr, nullptr, true);
-    }
-    if (aAttribute == nsGkAtoms::href ||
-        aAttribute == nsGkAtoms::rel) {
-      UpdateImport();
-    }
-  }
-
-  // The ordering of the parent class's UnsetAttr call and Link::ResetLinkState
-  // is important here!  The attribute is not unset until UnsetAttr returns, and
-  // we will need the updated attribute value because notifying the document
-  // that content states have changed will call IntrinsicState, which will try
-  // to get updated information about the visitedness from Link.
-  if (aAttribute == nsGkAtoms::href && kNameSpaceID_None == aNameSpaceID) {
-    Link::ResetLinkState(!!aNotify, false);
-    if (IsInUncomposedDoc()) {
-      CreateAndDispatchEvent(OwnerDoc(), NS_LITERAL_STRING("DOMLinkChanged"));
-    }
-  }
-
-  return rv;
 }
 
 nsresult

dom/html/HTMLLinkElement.h

@@ -61,16 +61,9 @@ public:
                               bool aCompileEventHandlers) override;
   virtual void UnbindFromTree(bool aDeep = true,
                               bool aNullParent = true) override;
-  nsresult SetAttr(int32_t aNameSpaceID, nsIAtom* aName,
+  virtual nsresult AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName,
-                   const nsAString& aValue, bool aNotify)
+                                const nsAttrValue* aValue,
-  {
+                                bool aNotify) override;
-    return SetAttr(aNameSpaceID, aName, nullptr, aValue, aNotify);
-  }
-  virtual nsresult SetAttr(int32_t aNameSpaceID, nsIAtom* aName,
-                           nsIAtom* aPrefix, const nsAString& aValue,
-                           bool aNotify) override;
-  virtual nsresult UnsetAttr(int32_t aNameSpaceID, nsIAtom* aAttribute,
-                             bool aNotify) override;
   virtual bool IsLink(nsIURI** aURI) const override;
   virtual already_AddRefed<nsIURI> GetHrefURI() const override;
 

dom/html/HTMLStyleElement.cpp

@@ -172,42 +172,22 @@ HTMLStyleElement::UnbindFromTree(bool aDeep, bool aNullParent)
 }
 
 nsresult
-HTMLStyleElement::SetAttr(int32_t aNameSpaceID, nsIAtom* aName,
+HTMLStyleElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName,
-                          nsIAtom* aPrefix, const nsAString& aValue,
+                               const nsAttrValue* aValue, bool aNotify)
-                          bool aNotify)
 {
-  nsresult rv = nsGenericHTMLElement::SetAttr(aNameSpaceID, aName, aPrefix,
+  if (aNameSpaceID == kNameSpaceID_None) {
-                                              aValue, aNotify);
-  if (NS_SUCCEEDED(rv) && aNameSpaceID == kNameSpaceID_None) {
     if (aName == nsGkAtoms::title ||
         aName == nsGkAtoms::media ||
         aName == nsGkAtoms::type) {
       UpdateStyleSheetInternal(nullptr, nullptr, true);
     } else if (aName == nsGkAtoms::scoped) {
-      UpdateStyleSheetScopedness(true);
+      bool isScoped = aValue;
+      UpdateStyleSheetScopedness(isScoped);
     }
   }
 
-  return rv;
+  return nsGenericHTMLElement::AfterSetAttr(aNameSpaceID, aName, aValue,
-}
+                                            aNotify);
-
-nsresult
-HTMLStyleElement::UnsetAttr(int32_t aNameSpaceID, nsIAtom* aAttribute,
-                            bool aNotify)
-{
-  nsresult rv = nsGenericHTMLElement::UnsetAttr(aNameSpaceID, aAttribute,
-                                                aNotify);
-  if (NS_SUCCEEDED(rv) && aNameSpaceID == kNameSpaceID_None) {
-    if (aAttribute == nsGkAtoms::title ||
-        aAttribute == nsGkAtoms::media ||
-        aAttribute == nsGkAtoms::type) {
-      UpdateStyleSheetInternal(nullptr, nullptr, true);
-    } else if (aAttribute == nsGkAtoms::scoped) {
-      UpdateStyleSheetScopedness(false);
-    }
-  }
-
-  return rv;
 }
 
 NS_IMETHODIMP

dom/html/HTMLStyleElement.h

@@ -46,16 +46,9 @@ public:
                               bool aCompileEventHandlers) override;
   virtual void UnbindFromTree(bool aDeep = true,
                               bool aNullParent = true) override;
-  nsresult SetAttr(int32_t aNameSpaceID, nsIAtom* aName,
+  virtual nsresult AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName,
-                   const nsAString& aValue, bool aNotify)
+                                const nsAttrValue* aValue,
-  {
+                                bool aNotify) override;
-    return SetAttr(aNameSpaceID, aName, nullptr, aValue, aNotify);
-  }
-  virtual nsresult SetAttr(int32_t aNameSpaceID, nsIAtom* aName,
-                           nsIAtom* aPrefix, const nsAString& aValue,
-                           bool aNotify) override;
-  virtual nsresult UnsetAttr(int32_t aNameSpaceID, nsIAtom* aAttribute,
-                             bool aNotify) override;
 
   virtual nsresult Clone(mozilla::dom::NodeInfo *aNodeInfo, nsINode **aResult) const override;