Bug 1383075 part 1 - Handle URL token in a closer way to the spec. r=dbaron
authorXidorn Quan <me@upsuper.org>
Fri, 28 Jul 2017 13:20:37 +1000
changeset 420813 1a6ea79cb3d707853597213904a979c40bd8a342
parent 420812 92b4376175cec6e90f41dfa1efdc95af9cd4846a
child 420814 5ab2855930cd07c9f671166c9f6c64c7a6b00bb3
push id7566
push usermtabara@mozilla.com
push dateWed, 02 Aug 2017 08:25:16 +0000
treeherdermozilla-beta@86913f512c3c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdbaron
bugs1383075
milestone56.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 1383075 part 1 - Handle URL token in a closer way to the spec. r=dbaron MozReview-Commit-ID: B31txMs8fVw
layout/reftests/css-parsing/invalid-url-handling.xhtml
layout/reftests/css-parsing/reftest.list
layout/style/nsCSSScanner.cpp
testing/web-platform/meta/css/CSS2/syntax/uri-013.xht.ini
--- a/layout/reftests/css-parsing/invalid-url-handling.xhtml
+++ b/layout/reftests/css-parsing/invalid-url-handling.xhtml
@@ -17,27 +17,26 @@
   </style>
   <style type="text/css">
   /* not a URI token, but handled according to rules for parsing errors */
   #foo { background: url(foo"bar
   ) }
   #two { background-color: green; }
   </style>
   <style type="text/css">
-  /* not a URI token; the unterminated string ends at end of line, so
-     the brace never matches */
+  /* not a URI token; bad-url token is consumed until the first closing ) */
+  #foo { background: url(foo"bar) }
   #three { background-color: green; }
-  #foo { background: url(foo"bar) }
-  #three { background-color: red; }
   </style>
   <style type="text/css">
-  /* not a URI token; the unterminated string ends at end of line */
+  /* not a URI token; bad-url token is consumed until the first closing ) */
+  #four { background-color: green; }
   #foo { background: url(foo"bar) }
   ) }
-  #four { background-color: green; }
+  #four { background-color: red; }
   </style>
   <style type="text/css">
   /* not a URI token; the unterminated string ends at end of line, so
      the brace never matches */
   #five { background-color: green; }
   #foo { background: url("bar) }
   #five { background-color: red; }
   </style>
@@ -63,28 +62,29 @@
   /* perfectly good URI token (image is a 404, though) */
   #ten { background: url({) green; }
   </style>
   <style type="text/css">
   /* perfectly good URI token (image is a 404, though) */
   #eleven { background: url([) green; }
   </style>
   <style type="text/css">
-  /* not a URI token; brace matching should work only after invalid URI token */
-  #twelve { background: url(}{""{)}); background-color: green; }
+  /* not a URI token; bad-url token is consumed until the first closing )
+     so the brace immediately after it closes the declaration block */
+  #twelve { background-color: green; }
+  #twelve { background: url(}{""{)}); background-color: red; }
   </style>
   <style type="text/css">
   /* invalid URI token absorbs the [ */
   #thirteen { background: url([""); background-color: green; }
   </style>
   <style type="text/css">
-  /* not a URI token; the opening ( is never matched */
+  /* not a URI token; bad-url token is consumed until the first closing ) */
+  #foo { background: url(() }
   #fourteen { background-color: green; }
-  #foo { background: url(() }
-  #fourteen { background-color: red; }
   </style>
   <!-- The next three tests test that invalid URI tokens absorb [ and { -->
   <style type="text/css">
   #foo { background: url(a()); }
   #fifteen { background-color: green }
   </style>
   <style type="text/css">
   #foo { background: url([()); }
--- a/layout/reftests/css-parsing/reftest.list
+++ b/layout/reftests/css-parsing/reftest.list
@@ -1,9 +1,9 @@
 == at-rule-013.html at-rule-013-ref.html
-fails-if(styloVsGecko||stylo) == invalid-url-handling.xhtml invalid-url-handling-ref.xhtml # bug 1383075
+== invalid-url-handling.xhtml invalid-url-handling-ref.xhtml
 == pseudo-elements-1.html pseudo-elements-1-ref.html
 == invalid-attr-1.html invalid-attr-1-ref.html
 == at-rule-error-handling-import-1.html at-rule-error-handling-ref.html
 == at-rule-error-handling-media-1.html at-rule-error-handling-ref.html
 == invalid-font-face-descriptor-1.html invalid-font-face-descriptor-1-ref.html
 == two-dash-identifiers.html two-dash-identifiers-ref.html
 == supports-moz-bool-pref.html supports-moz-bool-pref-ref.html
--- a/layout/style/nsCSSScanner.cpp
+++ b/layout/style/nsCSSScanner.cpp
@@ -1160,26 +1160,27 @@ nsCSSScanner::AppendImpliedEOFCharacters
 void
 nsCSSScanner::NextURL(nsCSSToken& aToken)
 {
   SkipWhitespace();
 
   // aToken.mIdent may be "url" at this point; clear that out
   aToken.mIdent.Truncate();
 
+  bool hasString = false;
   int32_t ch = Peek();
   // Do we have a string?
   if (ch == '"' || ch == '\'') {
     ScanString(aToken);
     if (MOZ_UNLIKELY(aToken.mType == eCSSToken_Bad_String)) {
       aToken.mType = eCSSToken_Bad_URL;
       return;
     }
     MOZ_ASSERT(aToken.mType == eCSSToken_String, "unexpected token type");
-
+    hasString = true;
   } else {
     // Otherwise, this is the start of a non-quoted url (which may be empty).
     aToken.mSymbol = char16_t(0);
     GatherText(IS_URL_CHAR, aToken.mIdent);
   }
 
   // Consume trailing whitespace and then look for a close parenthesis.
   SkipWhitespace();
@@ -1189,16 +1190,35 @@ nsCSSScanner::NextURL(nsCSSToken& aToken
     Advance();
     aToken.mType = eCSSToken_URL;
     if (ch < 0) {
       AddEOFCharacters(eEOFCharacters_CloseParen);
     }
   } else {
     mSeenBadToken = true;
     aToken.mType = eCSSToken_Bad_URL;
+    if (!hasString) {
+      // Consume until before the next right parenthesis, which follows
+      // how <bad-url-token> is consumed in CSS Syntax 3 spec.
+      // Note that, we only do this when "url(" is not followed by a
+      // string, because in the spec, "url(" followed by a string is
+      // handled as a url function rather than a <url-token>, so the
+      // rest of content before ")" should be consumed in balance,
+      // which will be done by the parser.
+      // The closing ")" is not consumed here. It is left to the parser
+      // so that the parser can handle both cases.
+      do {
+        if (IsVertSpace(ch)) {
+          AdvanceLine();
+        } else {
+          Advance();
+        }
+        ch = Peek();
+      } while (ch >= 0 && ch != ')');
+    }
   }
 }
 
 /**
  * Primary scanner entry point.  Consume one token and fill in
  * |aToken| accordingly.  Will skip over any number of comments first,
  * and will also skip over rather than return whitespace and comment
  * tokens, depending on the value of |aSkip|.
--- a/testing/web-platform/meta/css/CSS2/syntax/uri-013.xht.ini
+++ b/testing/web-platform/meta/css/CSS2/syntax/uri-013.xht.ini
@@ -1,4 +1,3 @@
 [uri-013.xht]
   type: reftest
-  expected:
-    if stylo: FAIL
+  expected: FAIL