Bug 1459844: Share more code and fix some inconsistencies between html / svg style elements. r?heycam draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Tue, 08 May 2018 06:51:34 +0200
changeset 792355 e100ecf165746c332d4f9b224ee627b1b5344221
parent 792354 fb3186662dda96aeffeda3e9fb1138b0725fa634
push id109083
push userbmo:emilio@crisal.io
push dateTue, 08 May 2018 08:13:42 +0000
reviewersheycam
bugs1459844
milestone61.0a1
Bug 1459844: Share more code and fix some inconsistencies between html / svg style elements. r?heycam MozReview-Commit-ID: IkTrIfJI1iK
dom/base/nsStyleLinkElement.cpp
dom/base/nsStyleLinkElement.h
dom/html/HTMLLinkElement.cpp
dom/html/HTMLStyleElement.cpp
dom/svg/SVGStyleElement.cpp
--- a/dom/base/nsStyleLinkElement.cpp
+++ b/dom/base/nsStyleLinkElement.cpp
@@ -81,16 +81,44 @@ nsStyleLinkElement::nsStyleLinkElement()
 }
 
 nsStyleLinkElement::~nsStyleLinkElement()
 {
   nsStyleLinkElement::SetStyleSheet(nullptr);
 }
 
 void
+nsStyleLinkElement::GetTitleAndMediaForElement(const Element& aSelf,
+                                               nsString& aTitle,
+                                               nsString& aMedia) const
+{
+  aSelf.GetAttr(kNameSpaceID_None, nsGkAtoms::title, aTitle);
+  aTitle.CompressWhitespace();
+
+  aSelf.GetAttr(kNameSpaceID_None, nsGkAtoms::media, aMedia);
+  // The HTML5 spec is formulated in terms of the CSSOM spec, which specifies
+  // that media queries should be ASCII lowercased during serialization.
+  //
+  // FIXME(emilio): How does it matter? This is going to be parsed anyway, CSS
+  // should take care of serializing it properly.
+  nsContentUtils::ASCIIToLower(aMedia);
+}
+
+bool
+nsStyleLinkElement::IsCssMimeTypeAttribute(const Element& aSelf) const
+{
+  nsAutoString type;
+  nsAutoString mimeType;
+  nsAutoString notUsed;
+  aSelf.GetAttr(kNameSpaceID_None, nsGkAtoms::type, type);
+  nsContentUtils::SplitMimeType(type, mimeType, notUsed);
+  return mimeType.IsEmpty() || mimeType.LowerCaseEqualsLiteral("text/css");
+}
+
+void
 nsStyleLinkElement::Unlink()
 {
   nsStyleLinkElement::SetStyleSheet(nullptr);
 }
 
 void
 nsStyleLinkElement::Traverse(nsCycleCollectionTraversalCallback &cb)
 {
--- a/dom/base/nsStyleLinkElement.h
+++ b/dom/base/nsStyleLinkElement.h
@@ -88,16 +88,29 @@ protected:
    *
    * TODO(emilio): Should probably pass a single DocumentOrShadowRoot.
    */
   mozilla::Result<Update, nsresult> UpdateStyleSheetInternal(
       nsIDocument* aOldDocument,
       mozilla::dom::ShadowRoot* aOldShadowRoot,
       ForceUpdate = ForceUpdate::No);
 
+  // Gets a suitable title and media for SheetInfo out of an element, which
+  // needs to be `this`.
+  //
+  // NOTE(emilio): Needs nsString instead of nsAString because of
+  // CompressWhitespace.
+  void GetTitleAndMediaForElement(const mozilla::dom::Element& aSelf,
+                                  nsString& aTitle,
+                                  nsString& aMedia) const;
+
+  // Returns whether the type attribute specifies the text/css mime type.
+  bool IsCssMimeTypeAttribute(const mozilla::dom::Element& aSelf) const;
+
+  // TODO(emilio): Should probably be const.
   virtual mozilla::Maybe<SheetInfo> GetStyleSheetInfo() = 0;
 
   // CC methods
   void Unlink();
   void Traverse(nsCycleCollectionTraversalCallback &cb);
 
 private:
   /**
--- a/dom/html/HTMLLinkElement.cpp
+++ b/dom/html/HTMLLinkElement.cpp
@@ -425,50 +425,36 @@ HTMLLinkElement::GetStyleSheetInfo()
 {
   nsAutoString rel;
   GetAttr(kNameSpaceID_None, nsGkAtoms::rel, rel);
   uint32_t linkTypes = nsStyleLinkElement::ParseLinkTypes(rel);
   if (!(linkTypes & nsStyleLinkElement::eSTYLESHEET)) {
     return Nothing();
   }
 
+  if (!IsCssMimeTypeAttribute(*this)) {
+    return Nothing();
+  }
+
   nsAutoString title;
-  GetAttr(kNameSpaceID_None, nsGkAtoms::title, title);
-  title.CompressWhitespace();
+  nsAutoString media;
+  GetTitleAndMediaForElement(*this, title, media);
 
   bool alternate = linkTypes & nsStyleLinkElement::eALTERNATE;
   if (alternate && title.IsEmpty()) {
     // alternates must have title.
     return Nothing();
   }
 
-  nsAutoString type;
-  nsAutoString mimeType;
-  nsAutoString notUsed;
-  GetAttr(kNameSpaceID_None, nsGkAtoms::type, type);
-  nsContentUtils::SplitMimeType(type, mimeType, notUsed);
-  if (!mimeType.IsEmpty() && !mimeType.LowerCaseEqualsLiteral("text/css")) {
-    return Nothing();
-  }
-
   nsAutoString href;
   GetAttr(kNameSpaceID_None, nsGkAtoms::href, href);
   if (href.IsEmpty()) {
     return Nothing();
   }
 
-  nsAutoString media;
-  GetAttr(kNameSpaceID_None, nsGkAtoms::media, media);
-  // The HTML5 spec is formulated in terms of the CSSOM spec, which specifies
-  // that media queries should be ASCII lowercased during serialization.
-  //
-  // FIXME(emilio): How does it matter? This is going to be parsed anyway, CSS
-  // should take care of serializing it properly.
-  nsContentUtils::ASCIIToLower(media);
-
   nsCOMPtr<nsIURI> uri = Link::GetURI();
   nsCOMPtr<nsIPrincipal> prin = mTriggeringPrincipal;
   return Some(SheetInfo {
     *OwnerDoc(),
     this,
     uri.forget(),
     prin.forget(),
     GetReferrerPolicyAsEnum(),
--- a/dom/html/HTMLStyleElement.cpp
+++ b/dom/html/HTMLStyleElement.cpp
@@ -192,37 +192,24 @@ HTMLStyleElement::SetTextContentInternal
   mTriggeringPrincipal = aScriptedPrincipal;
 
   Unused << UpdateStyleSheetInternal(nullptr, nullptr);
 }
 
 Maybe<nsStyleLinkElement::SheetInfo>
 HTMLStyleElement::GetStyleSheetInfo()
 {
-  nsAutoString title;
-  GetAttr(kNameSpaceID_None, nsGkAtoms::title, title);
-  title.CompressWhitespace();
-
-  nsAutoString media;
-  GetAttr(kNameSpaceID_None, nsGkAtoms::media, media);
-  // The HTML5 spec is formulated in terms of the CSSOM spec, which specifies
-  // that media queries should be ASCII lowercased during serialization.
-  //
-  // FIXME(emilio): Doesn't matter I'd think, style should take care of that.
-  nsContentUtils::ASCIIToLower(media);
-
-  nsAutoString type;
-  nsAutoString mimeType;
-  nsAutoString notUsed;
-  GetAttr(kNameSpaceID_None, nsGkAtoms::type, type);
-  nsContentUtils::SplitMimeType(type, mimeType, notUsed);
-  if (!mimeType.IsEmpty() && !mimeType.LowerCaseEqualsLiteral("text/css")) {
+  if (!IsCssMimeTypeAttribute(*this)) {
     return Nothing();
   }
 
+  nsAutoString title;
+  nsAutoString media;
+  GetTitleAndMediaForElement(*this, title, media);
+
   nsCOMPtr<nsIPrincipal> prin = mTriggeringPrincipal;
   return Some(SheetInfo {
     *OwnerDoc(),
     this,
     nullptr,
     prin.forget(),
     net::ReferrerPolicy::RP_Unset,
     CORS_NONE,
--- a/dom/svg/SVGStyleElement.cpp
+++ b/dom/svg/SVGStyleElement.cpp
@@ -214,39 +214,34 @@ SVGStyleElement::SetTitle(const nsAStrin
 }
 
 //----------------------------------------------------------------------
 // nsStyleLinkElement methods
 
 Maybe<nsStyleLinkElement::SheetInfo>
 SVGStyleElement::GetStyleSheetInfo()
 {
-  nsAutoString title;
-  GetAttr(kNameSpaceID_None, nsGkAtoms::title, title);
-  title.CompressWhitespace();
-
-  nsAutoString media;
-  GetAttr(kNameSpaceID_None, nsGkAtoms::media, media);
-  // The SVG spec is formulated in terms of the CSS2 spec,
-  // which specifies that media queries are case insensitive.
-  nsContentUtils::ASCIIToLower(media);
-
-  // FIXME(emilio): Why doesn't this do the same as HTMLStyleElement?
-  nsAutoString type;
-  GetAttr(kNameSpaceID_None, nsGkAtoms::type, type);
-  if (!type.IsEmpty() && !type.LowerCaseEqualsLiteral("text/css")) {
+  if (!IsCssMimeTypeAttribute(*this)) {
     return Nothing();
   }
 
+  nsAutoString title;
+  nsAutoString media;
+  GetTitleAndMediaForElement(*this, title, media);
+
   return Some(SheetInfo {
     *OwnerDoc(),
     this,
     nullptr,
+    // FIXME(bug 1459822): Why doesn't this need a principal, but
+    // HTMLStyleElement does?
     nullptr,
     net::ReferrerPolicy::RP_Unset,
+    // FIXME(bug 1459822): Why does this need a crossorigin attribute, but
+    // HTMLStyleElement doesn't?
     AttrValueToCORSMode(GetParsedAttr(nsGkAtoms::crossorigin)),
     title,
     media,
     HasAlternateRel::No,
     IsInline::Yes,
   });
 }