Bug 1399911 - preserve sourceURL comment directive on style sheets; r?heycam, bz draft
authorTom Tromey <tom@tromey.com>
Thu, 14 Sep 2017 14:59:32 -0600
changeset 667625 9cf658e9e0fd9d88cfa012a187706efebc4726f5
parent 665717 d6a3a83d9ce3c16773a5c818b5f754ff7728f665
child 732453 65e3363beb53ea04d9d05d8261cd5a0b747305f6
push id80785
push userbmo:ttromey@mozilla.com
push dateWed, 20 Sep 2017 12:42:11 +0000
reviewersheycam, bz
bugs1399911
milestone57.0a1
Bug 1399911 - preserve sourceURL comment directive on style sheets; r?heycam, bz In addition to the sourceMappingURL comment, there is a second special comment, "sourceURL", that can be used to set the "display name" of a style sheet for developer tools. This name is also used as the base URL for the source-map URL resolution algorithm. sourceURL is described here: https://blog.getfirebug.com/2009/08/11/give-your-eval-a-name-with-sourceurl/ This patch changes Firefox to record this URL, if specified, and to expose it (chrome-only) vai StyleSheet.webidl. MozReview-Commit-ID: 7NwXsOf7nbY
dom/webidl/StyleSheet.webidl
layout/style/ServoBindingList.h
layout/style/ServoStyleSheet.cpp
layout/style/StyleSheet.cpp
layout/style/StyleSheet.h
layout/style/StyleSheetInfo.h
layout/style/nsCSSParser.cpp
layout/style/nsCSSScanner.cpp
layout/style/nsCSSScanner.h
layout/style/test/browser.ini
layout/style/test/browser_sourceurl_comment.js
--- a/dom/webidl/StyleSheet.webidl
+++ b/dom/webidl/StyleSheet.webidl
@@ -34,9 +34,15 @@ interface StyleSheet {
   //
   // If the style sheet has the special "# sourceMappingURL=" comment,
   // then this is the URL specified there.
   //
   // If the source map URL is not found by either of these methods,
   // then this is an empty string.
   [ChromeOnly, Pure]
   readonly attribute DOMString sourceMapURL;
+  // The source URL for this style sheet.  If the style sheet has the
+  // special "# sourceURL=" comment, then this is the URL specified
+  // there.  If no such comment is found, then this is the empty
+  // string.
+  [ChromeOnly, Pure]
+  readonly attribute DOMString sourceURL;
 };
--- a/layout/style/ServoBindingList.h
+++ b/layout/style/ServoBindingList.h
@@ -58,16 +58,18 @@ SERVO_BINDING_FUNC(Servo_StyleSheet_GetR
 SERVO_BINDING_FUNC(Servo_StyleSheet_Clone, RawServoStyleSheetContentsStrong,
                    RawServoStyleSheetContentsBorrowed sheet,
                    const mozilla::ServoStyleSheet* reference_sheet);
 SERVO_BINDING_FUNC(Servo_StyleSheet_SizeOfIncludingThis, size_t,
                    mozilla::MallocSizeOf malloc_size_of,
                    RawServoStyleSheetContentsBorrowed sheet)
 SERVO_BINDING_FUNC(Servo_StyleSheet_GetSourceMapURL, void,
                    RawServoStyleSheetContentsBorrowed sheet, nsAString* result)
+SERVO_BINDING_FUNC(Servo_StyleSheet_GetSourceURL, void,
+                   RawServoStyleSheetContentsBorrowed sheet, nsAString* result)
 // We'd like to return `OriginFlags` here, but bindgen bitfield enums don't
 // work as return values with the Linux 32-bit ABI at the moment because
 // they wrap the value in a struct.
 SERVO_BINDING_FUNC(Servo_StyleSheet_GetOrigin, uint8_t,
                    RawServoStyleSheetContentsBorrowed sheet)
 SERVO_BINDING_FUNC(Servo_StyleSet_Init, RawServoStyleSet*, RawGeckoPresContextOwned pres_context)
 SERVO_BINDING_FUNC(Servo_StyleSet_RebuildCachedData, void,
                    RawServoStyleSetBorrowed set)
--- a/layout/style/ServoStyleSheet.cpp
+++ b/layout/style/ServoStyleSheet.cpp
@@ -217,16 +217,20 @@ ServoStyleSheet::ParseSheet(css::Loader*
                                                       aCompatMode,
                                                       aReusableSheets)
                          .Consume();
 
   nsString sourceMapURL;
   Servo_StyleSheet_GetSourceMapURL(Inner()->mContents, &sourceMapURL);
   SetSourceMapURLFromComment(sourceMapURL);
 
+  nsString sourceURL;
+  Servo_StyleSheet_GetSourceURL(Inner()->mContents, &sourceURL);
+  SetSourceURL(sourceURL);
+
   Inner()->mURLData = extraData.forget();
   return NS_OK;
 }
 
 nsresult
 ServoStyleSheet::ReparseSheet(const nsAString& aInput)
 {
   if (!mInner->mComplete) {
--- a/layout/style/StyleSheet.cpp
+++ b/layout/style/StyleSheet.cpp
@@ -256,16 +256,17 @@ StyleSheetInfo::StyleSheetInfo(StyleShee
   , mCORSMode(aCopy.mCORSMode)
   , mReferrerPolicy(aCopy.mReferrerPolicy)
   , mIntegrity(aCopy.mIntegrity)
   , mComplete(aCopy.mComplete)
   , mFirstChild()  // We don't rebuild the child because we're making a copy
                    // without children.
   , mSourceMapURL(aCopy.mSourceMapURL)
   , mSourceMapURLFromComment(aCopy.mSourceMapURLFromComment)
+  , mSourceURL(aCopy.mSourceURL)
 #ifdef DEBUG
   , mPrincipalSet(aCopy.mPrincipalSet)
 #endif
 {
   AddSheet(aPrimarySheet);
 }
 
 StyleSheetInfo::~StyleSheetInfo()
@@ -525,16 +526,28 @@ StyleSheet::SetSourceMapURL(const nsAStr
 }
 
 void
 StyleSheet::SetSourceMapURLFromComment(const nsAString& aSourceMapURLFromComment)
 {
   mInner->mSourceMapURLFromComment = aSourceMapURLFromComment;
 }
 
+void
+StyleSheet::GetSourceURL(nsAString& aSourceURL)
+{
+  aSourceURL = mInner->mSourceURL;
+}
+
+void
+StyleSheet::SetSourceURL(const nsAString& aSourceURL)
+{
+  mInner->mSourceURL = aSourceURL;
+}
+
 css::Rule*
 StyleSheet::GetDOMOwnerRule() const
 {
   return mOwnerRule;
 }
 
 uint32_t
 StyleSheet::InsertRule(const nsAString& aRule, uint32_t aIndex,
--- a/layout/style/StyleSheet.h
+++ b/layout/style/StyleSheet.h
@@ -211,16 +211,18 @@ public:
   inline StyleSheet* GetParentStyleSheet() const;
   // The XPCOM GetTitle is fine for WebIDL.
   dom::MediaList* Media();
   bool Disabled() const { return mDisabled; }
   // The XPCOM SetDisabled is fine for WebIDL.
   void GetSourceMapURL(nsAString& aTitle);
   void SetSourceMapURL(const nsAString& aSourceMapURL);
   void SetSourceMapURLFromComment(const nsAString& aSourceMapURLFromComment);
+  void GetSourceURL(nsAString& aSourceURL);
+  void SetSourceURL(const nsAString& aSourceURL);
 
   // WebIDL CSSStyleSheet API
   // Can't be inline because we can't include ImportRule here.  And can't be
   // called GetOwnerRule because that would be ambiguous with the ImportRule
   // version.
   css::Rule* GetDOMOwnerRule() const;
   dom::CSSRuleList* GetCssRules(nsIPrincipal& aSubjectPrincipal,
                                 ErrorResult& aRv);
--- a/layout/style/StyleSheetInfo.h
+++ b/layout/style/StyleSheetInfo.h
@@ -65,16 +65,19 @@ struct StyleSheetInfo
   // the value.  If both are seen, SourceMap is preferred.  If neither
   // is seen, this will be an empty string.
   nsString mSourceMapURL;
   // This stores any source map URL that might have been seen in a
   // comment in the style sheet.  This is separate from mSourceMapURL
   // so that the value does not overwrite any value that might have
   // come from a response header.
   nsString mSourceMapURLFromComment;
+  // This stores any source URL that might have been seen in a comment
+  // in the style sheet.
+  nsString mSourceURL;
 
 #ifdef DEBUG
   bool                   mPrincipalSet;
 #endif
 };
 
 } // namespace mozilla
 
--- a/layout/style/nsCSSParser.cpp
+++ b/layout/style/nsCSSParser.cpp
@@ -1643,16 +1643,17 @@ CSSParserImpl::ParseSheet(const nsAStrin
     }
     UngetToken();
     if (ParseRuleSet(AppendRuleToSheet, this)) {
       mSection = eCSSSection_General;
     }
   }
 
   mSheet->SetSourceMapURLFromComment(scanner.GetSourceMapURL());
+  mSheet->SetSourceURL(scanner.GetSourceURL());
   ReleaseScanner();
 
   mParsingMode = css::eAuthorSheetFeatures;
   mIsChrome = false;
   mReusableSheets = nullptr;
 
   return NS_OK;
 }
--- a/layout/style/nsCSSScanner.cpp
+++ b/layout/style/nsCSSScanner.cpp
@@ -538,92 +538,99 @@ nsCSSScanner::SkipWhitespace()
       AdvanceLine();
     } else {
       Advance();
     }
   }
 }
 
 /**
+ * If the given text appears at the current offset in the buffer,
+ * advance over the text and return true.  Otherwise, return false.
+ * mLength is the number of characters in mDirective.
+ */
+bool
+nsCSSScanner::CheckCommentDirective(const nsAString& aDirective)
+{
+  nsDependentSubstring text(&mBuffer[mOffset], &mBuffer[mCount]);
+
+  if (StringBeginsWith(text, aDirective)) {
+    Advance(aDirective.Length());
+    return true;
+  }
+  return false;
+}
+
+/**
  * Skip over one CSS comment starting at the current read position.
  */
 void
 nsCSSScanner::SkipComment()
 {
-  static const char sourceMappingURLDirective[] = "# sourceMappingURL=";
+  // Note that these do not start with "#" or "@" -- that is handled
+  // separately, below.
+  static NS_NAMED_LITERAL_STRING(kSourceMappingURLDirective, " sourceMappingURL=");
+  static NS_NAMED_LITERAL_STRING(kSourceURLDirective, " sourceURL=");
 
   MOZ_ASSERT(Peek() == '/' && Peek(1) == '*', "should not have been called");
   Advance(2);
-  // Look in each comment for a source map directive; using a simple
-  // state machine.  The states are:
-  // * sourceMapIndex >= 0 means that we're still looking for the
-  //   directive and expect the next character to be at that index of
-  //   sourceMappingURLDirective.
-  //   As a special case, when sourceMapIndex == 0, '@' is also recognized.
-  // * sourceMapIndex < 0 means that we don't need to look for the
-  //   directive any more -- whether it was found or not.
-  // * copying == true means that the directive was found and we're
-  //   copying characters into mSourceMapURL.  This stops at the first
-  //   whitespace, or at the end of the comment.
-  int sourceMapIndex = 0;
-  bool copying = false;
+
+  // If we saw one of the directives, this will be non-NULL and will
+  // point to the string into which the URL will be written.
+  nsString* directive = nullptr;
+  if (Peek() == '#' || Peek() == '@') {
+    // Check for the comment directives.
+    Advance();
+    if (CheckCommentDirective(kSourceMappingURLDirective)) {
+      mSourceMapURL.Truncate();
+      directive = &mSourceMapURL;
+    } else if (CheckCommentDirective(kSourceURLDirective)) {
+      mSourceURL.Truncate();
+      directive = &mSourceURL;
+    }
+  }
+
   for (;;) {
     int32_t ch = Peek();
     if (ch < 0) {
       if (mReporter)
         mReporter->ReportUnexpectedEOF("PECommentEOF");
       SetEOFCharacters(eEOFCharacters_Asterisk | eEOFCharacters_Slash);
       return;
     }
-    if (sourceMapIndex >= 0) {
-      if ((sourceMapIndex == 0 && ch == '@') || ch == sourceMappingURLDirective[sourceMapIndex]) {
-        ++sourceMapIndex;
-        if (sourceMappingURLDirective[sourceMapIndex] == '\0') {
-          sourceMapIndex = -1;
-          mSourceMapURL.Truncate();
-          copying = true;
-          Advance();
-          // Make sure we don't copy out the '=' by falling through.
-          continue;
-        }
-      } else {
-        // Did not see the directive.
-        sourceMapIndex = -1;
-      }
-    }
 
     if (ch == '*') {
       Advance();
       ch = Peek();
       if (ch < 0) {
         // In this case, even if we saw a source map directive, leave
         // the "*" out of it.
         if (mReporter)
           mReporter->ReportUnexpectedEOF("PECommentEOF");
         SetEOFCharacters(eEOFCharacters_Slash);
         return;
       }
       if (ch == '/') {
         Advance();
         return;
       }
-      if (copying) {
-        mSourceMapURL.Append('*');
+      if (directive != nullptr) {
+        directive->Append('*');
       }
     } else if (IsVertSpace(ch)) {
       AdvanceLine();
       // Done with the directive, so stop copying.
-      copying = false;
+      directive = nullptr;
     } else if (IsWhitespace(ch)) {
       Advance();
       // Done with the directive, so stop copying.
-      copying = false;
+      directive = nullptr;
     } else {
-      if (copying) {
-        mSourceMapURL.Append(ch);
+      if (directive != nullptr) {
+        directive->Append(ch);
       }
       Advance();
     }
   }
 }
 
 /**
  * If there is a valid escape sequence starting at the current read
--- a/layout/style/nsCSSScanner.h
+++ b/layout/style/nsCSSScanner.h
@@ -230,16 +230,19 @@ class nsCSSScanner {
   { return mTokenOffset; }
 
   uint32_t GetTokenEndOffset() const
   { return mOffset; }
 
   const nsAString& GetSourceMapURL() const
   { return mSourceMapURL; }
 
+  const nsAString& GetSourceURL() const
+  { return mSourceURL; }
+
   // Get the text of the line containing the first character of
   // the most recently processed token.
   nsDependentSubstring GetCurrentLine() const;
 
   // Get the next token.  Return false on EOF.  aTokenResult is filled
   // in with the data for the token.  aSkip controls whether
   // whitespace and/or comment tokens are ever returned.
   bool Next(nsCSSToken& aTokenResult, nsCSSScannerExclude aSkip);
@@ -325,16 +328,17 @@ class nsCSSScanner {
 #endif
 
 protected:
   int32_t Peek(uint32_t n = 0);
   void Advance(uint32_t n = 1);
   void AdvanceLine();
 
   void SkipWhitespace();
+  bool CheckCommentDirective(const nsAString& aDirective);
   void SkipComment();
 
   bool GatherEscape(nsString& aOutput, bool aInString);
   bool GatherText(uint8_t aClass, nsString& aIdent);
 
   bool ScanIdent(nsCSSToken& aResult);
   bool ScanAtKeyword(nsCSSToken& aResult);
   bool ScanHash(nsCSSToken& aResult);
@@ -361,16 +365,17 @@ protected:
 
   mozilla::css::ErrorReporter *mReporter;
 
   bool mRecording;
   bool mSeenBadToken;
   bool mSeenVariableReference;
 
   nsString mSourceMapURL;
+  nsString mSourceURL;
 };
 
 // Token for the grid-template-areas micro-syntax
 // http://dev.w3.org/csswg/css-grid/#propdef-grid-template-areas
 struct MOZ_STACK_CLASS nsCSSGridTemplateAreaToken {
   nsAutoString mName;  // Empty for a null cell, non-empty for a named cell
   bool isTrash;  // True for a trash token, mName is ignored in this case.
 };
--- a/layout/style/test/browser.ini
+++ b/layout/style/test/browser.ini
@@ -9,8 +9,9 @@ support-files =
   mapped2.css^headers^
   sourcemap_css.html
 
 [browser_bug453896.js]
 [browser_newtab_share_rule_processors.js]
 skip-if = stylo # Gecko-specific test
 [browser_sourcemap.js]
 [browser_sourcemap_comment.js]
+[browser_sourceurl_comment.js]
new file mode 100644
--- /dev/null
+++ b/layout/style/test/browser_sourceurl_comment.js
@@ -0,0 +1,40 @@
+add_task(async function() {
+  // Test text and expected results.
+  let test_cases = [
+    ["/*# sourceURL=here*/", "here"],
+    ["/*# sourceURL=here  */", "here"],
+    ["/*@ sourceURL=here*/", "here"],
+    ["/*@ sourceURL=there*/ /*# sourceURL=here*/", "here"],
+    ["/*# sourceURL=here there  */", "here"],
+
+    ["/*# sourceURL=  here  */", ""],
+    ["/*# sourceURL=*/", ""],
+    ["/*# sourceUR=here  */", ""],
+    ["/*! sourceURL=here  */", ""],
+    ["/*# sourceURL = here  */", ""],
+    ["/*   # sourceURL=here   */", ""],
+  ];
+
+  let page = "<!DOCTYPE HTML>\n<html>\n<head>\n";
+  for (let i = 0; i < test_cases.length; ++i) {
+    page += `<style type="text/css"> #x${i} { color: red; }${test_cases[i][0]}</style>\n`;
+  }
+  page += "</head><body>some text</body></html>";
+
+  let uri = "data:text/html;base64," + btoa(page);
+  info(`URI is ${uri}`);
+
+  await BrowserTestUtils.withNewTab({
+    gBrowser,
+    url: uri
+  }, async function(browser) {
+    await ContentTask.spawn(browser, test_cases, function* (tests) {
+      for (let i = 0; i < content.document.styleSheets.length; ++i) {
+        let sheet = content.document.styleSheets[i];
+
+        info(`Checking sheet #${i}`);
+        is(sheet.sourceURL, tests[i][1], `correct source URL for sheet ${i}`);
+      }
+    });
+  });
+});