Bug 1370458: Disallow floats, negative numbers, and long values in hashless color quirk; r?xidorn draft
authorManish Goregaokar <manishearth@gmail.com>
Mon, 05 Jun 2017 23:47:30 -0700
changeset 589440 7bcaf4d09d794b9839e12ea9b3696ea5c2494051
parent 589150 cad53f061da634a16ea75887558301b77f65745d
child 589868 52cc4ec4100477292ac4c4e37bfb306ce894fc55
push id62374
push userbmo:manishearth@gmail.com
push dateTue, 06 Jun 2017 07:21:33 +0000
reviewersxidorn
bugs1370458
milestone55.0a1
Bug 1370458: Disallow floats, negative numbers, and long values in hashless color quirk; r?xidorn MozReview-Commit-ID: FAwr9rgj58c
layout/style/nsCSSParser.cpp
testing/web-platform/meta/quirks-mode/hashless-hex-color.html.ini
--- a/layout/style/nsCSSParser.cpp
+++ b/layout/style/nsCSSParser.cpp
@@ -6690,16 +6690,43 @@ GetEnumColorValue(nsCSSKeyword aKeyword,
     // doesn't map it to a color. (This might be a platform-specific
     // ColorID, which only makes sense on another platform.)
     return Nothing();
   }
   // Known color provided by LookAndFeel.
   return Some(value);
 }
 
+/// Returns the number of digits in a positive number
+/// assuming it has <= 6 digits
+static uint32_t
+CountNumbersForHashlessColor(uint32_t number) {
+  /// Just use a simple match instead of calculating a log
+  /// or dividing in a loop to be more efficient.
+  if (number < 10) {
+    return 1;
+  } else if (number < 100) {
+    return 2;
+  } else if (number < 1000) {
+    return 3;
+  } else if (number < 10000) {
+    return 4;
+  } else if (number < 100000) {
+    return 5;
+  } else if (number < 1000000) {
+    return 6;
+  } else {
+    // we don't care about numbers with more than 6 digits other
+    // than the fact that they have more than 6 digits, so just return something
+    // larger than 6 here. This is incorrect in the general case.
+    return 100;
+  }
+}
+
+
 CSSParseResult
 CSSParserImpl::ParseColor(nsCSSValue& aValue)
 {
   if (!GetToken(true)) {
     REPORT_UNEXPECTED_EOF(PEColorEOF);
     return CSSParseResult::NotFound;
   }
 
@@ -6799,46 +6826,52 @@ CSSParserImpl::ParseColor(nsCSSValue& aV
       break;
     }
     default:
       break;
   }
 
   // try 'xxyyzz' without '#' prefix for compatibility with IE and Nav4x (bug 23236 and 45804)
   if (mHashlessColorQuirk) {
+    // https://quirks.spec.whatwg.org/#the-hashless-hex-color-quirk
+    //
     // - If the string starts with 'a-f', the nsCSSScanner builds the
     //   token as a eCSSToken_Ident and we can parse the string as a
     //   'xxyyzz' RGB color.
-    // - If it only contains '0-9' digits, the token is a
+    // - If it only contains up to six '0-9' digits, the token is a
     //   eCSSToken_Number and it must be converted back to a 6
-    //   characters string to be parsed as a RGB color.
+    //   characters string to be parsed as a RGB color. The number cannot
+    //   be specified as more than six digits.
     // - If it starts with '0-9' and contains any 'a-f', the token is a
     //   eCSSToken_Dimension, the mNumber part must be converted back to
     //   a string and the mIdent part must be appended to that string so
-    //   that the resulting string has 6 characters.
+    //   that the resulting string has 6 characters. The combined
+    //   dimension cannot be longer than 6 characters.
     // Note: This is a hack for Nav compatibility.  Do not attempt to
     // simplify it by hacking into the ncCSSScanner.  This would be very
     // bad.
     nsAutoString str;
     char buffer[20];
     switch (tk->mType) {
       case eCSSToken_Ident:
         str.Assign(tk->mIdent);
         break;
 
       case eCSSToken_Number:
-        if (tk->mIntegerValid) {
+        if (tk->mIntegerValid && tk->mInteger < 1000000 && tk->mInteger >= 0) {
           SprintfLiteral(buffer, "%06d", tk->mInteger);
           str.AssignWithConversion(buffer);
         }
         break;
 
       case eCSSToken_Dimension:
-        if (tk->mIdent.Length() <= 6) {
-          SprintfLiteral(buffer, "%06.0f", tk->mNumber);
+        if (tk->mIntegerValid &&
+            tk->mIdent.Length() + CountNumbersForHashlessColor(tk->mInteger) <= 6 &&
+            tk->mInteger >= 0) {
+          SprintfLiteral(buffer, "%06d", tk->mInteger);
           nsAutoString temp;
           temp.AssignWithConversion(buffer);
           temp.Right(str, 6 - tk->mIdent.Length());
           str.Append(tk->mIdent);
         }
         break;
       default:
         // There is a whole bunch of cases that are
--- a/testing/web-platform/meta/quirks-mode/hashless-hex-color.html.ini
+++ b/testing/web-platform/meta/quirks-mode/hashless-hex-color.html.ini
@@ -1,190 +1,10 @@
 [hashless-hex-color.html]
   type: testharness
-  [1abcdef (quirks)]
-    expected: FAIL
-
-  [+123456a (quirks)]
-    expected: FAIL
-
-  [+1234567a (quirks)]
-    expected: FAIL
-
-  [-1a (quirks)]
-    expected: FAIL
-
-  [-12a (quirks)]
-    expected: FAIL
-
-  [-123a (quirks)]
-    expected: FAIL
-
-  [-1234a (quirks)]
-    expected: FAIL
-
-  [-12345a (quirks)]
-    expected: FAIL
-
-  [-123456a (quirks)]
-    expected: FAIL
-
-  [-1234567a (quirks)]
-    expected: FAIL
-
-  [-12345678a (quirks)]
-    expected: FAIL
-
-  [+123456A (quirks)]
-    expected: FAIL
-
-  [+1234567A (quirks)]
-    expected: FAIL
-
-  [-1A (quirks)]
-    expected: FAIL
-
-  [-12A (quirks)]
-    expected: FAIL
-
-  [-123A (quirks)]
-    expected: FAIL
-
-  [-1234A (quirks)]
-    expected: FAIL
-
-  [-12345A (quirks)]
-    expected: FAIL
-
-  [-123456A (quirks)]
-    expected: FAIL
-
-  [-1234567A (quirks)]
-    expected: FAIL
-
-  [-12345678A (quirks)]
-    expected: FAIL
-
-  [1.1a (quirks)]
-    expected: FAIL
-
-  [1.11a (quirks)]
-    expected: FAIL
-
-  [1.111a (quirks)]
-    expected: FAIL
-
-  [1.1111a (quirks)]
-    expected: FAIL
-
-  [1.11111a (quirks)]
-    expected: FAIL
-
-  [1.111111a (quirks)]
-    expected: FAIL
-
-  [+1.1a (quirks)]
-    expected: FAIL
-
-  [+1.11a (quirks)]
-    expected: FAIL
-
-  [+1.111a (quirks)]
-    expected: FAIL
-
-  [+1.1111a (quirks)]
-    expected: FAIL
-
-  [+1.11111a (quirks)]
-    expected: FAIL
-
-  [+1.111111a (quirks)]
-    expected: FAIL
-
-  [-1.1a (quirks)]
-    expected: FAIL
-
-  [-1.11a (quirks)]
-    expected: FAIL
-
-  [-1.111a (quirks)]
-    expected: FAIL
-
-  [-1.1111a (quirks)]
-    expected: FAIL
-
-  [-1.11111a (quirks)]
-    expected: FAIL
-
-  [-1.111111a (quirks)]
-    expected: FAIL
-
-  [1.0a (quirks)]
-    expected: FAIL
-
-  [11.0a (quirks)]
-    expected: FAIL
-
-  [111.0a (quirks)]
-    expected: FAIL
-
-  [1111.0a (quirks)]
-    expected: FAIL
-
-  [11111.0a (quirks)]
-    expected: FAIL
-
-  [111111.0a (quirks)]
-    expected: FAIL
-
-  [1111111.0a (quirks)]
-    expected: FAIL
-
-  [+1.0a (quirks)]
-    expected: FAIL
-
-  [+11.0a (quirks)]
-    expected: FAIL
-
-  [+111.0a (quirks)]
-    expected: FAIL
-
-  [+1111.0a (quirks)]
-    expected: FAIL
-
-  [+11111.0a (quirks)]
-    expected: FAIL
-
-  [+111111.0a (quirks)]
-    expected: FAIL
-
-  [+1111111.0a (quirks)]
-    expected: FAIL
-
-  [-1.0a (quirks)]
-    expected: FAIL
-
-  [-11.0a (quirks)]
-    expected: FAIL
-
-  [-111.0a (quirks)]
-    expected: FAIL
-
-  [-1111.0a (quirks)]
-    expected: FAIL
-
-  [-11111.0a (quirks)]
-    expected: FAIL
-
-  [-111111.0a (quirks)]
-    expected: FAIL
-
-  [-1111111.0a (quirks)]
-    expected: FAIL
-
   [rgb(calc(100 + 155), 255, 255) (quirks)]
     expected: FAIL
 
   [rgb(calc(100 + 155), 255, 255) (almost standards)]
     expected: FAIL
 
   [rgb(calc(100 + 155), 255, 255) (standards)]
     expected: FAIL
@@ -234,27 +54,8 @@
   [rgb(calc(/**/100/**/ + /**/155/**/), 255, 255) (almost standards)]
     expected: FAIL
 
   [rgb(calc(/**/100/**/ + /**/155/**/), 255, 255) (standards)]
     expected: FAIL
 
   [rgb(calc(/**/100/**/ + /**/155/**/), 255, 255) (SVG)]
     expected: FAIL
-
-  [1e1a (quirks)]
-    expected: FAIL
-
-  [11e1a (quirks)]
-    expected: FAIL
-
-  [111e1a (quirks)]
-    expected: FAIL
-
-  [1111e1a (quirks)]
-    expected: FAIL
-
-  [11111e1a (quirks)]
-    expected: FAIL
-
-  [111111e1a (quirks)]
-    expected: FAIL
-