Bug 1399911 - preserve sourceURL comment directive on style sheets; r=bz,heycam
authorTom Tromey <tom@tromey.com>
Thu, 14 Sep 2017 14:59:32 -0600
changeset 431596 2d5c378f56108610c674b8ba8147f969c166ef48
parent 431595 8e98f894a8a322859c307c84778eada9444a5dfd
child 431597 89b1fac5342d401365727928dedc87254a7ec748
push id7785
push userryanvm@gmail.com
push dateThu, 21 Sep 2017 13:39:55 +0000
treeherdermozilla-beta@06d4034a8a03 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz, heycam
bugs1399911
milestone57.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1399911 - preserve sourceURL comment directive on style sheets; r=bz,heycam 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
@@ -63,16 +63,18 @@ SERVO_BINDING_FUNC(Servo_StyleSheet_Clon
                    RawServoStyleSheetContentsBorrowed sheet,
                    const mozilla::ServoStyleSheet* reference_sheet);
 SERVO_BINDING_FUNC(Servo_StyleSheet_SizeOfIncludingThis, size_t,
                    mozilla::MallocSizeOf malloc_size_of,
                    mozilla::MallocSizeOf malloc_enclosing_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
@@ -220,16 +220,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}`);
+      }
+    });
+  });
+});