Bug 1370458: Disallow floats, negative numbers, and long values in hashless color quirk; r=xidorn
authorManish Goregaokar <manishearth@gmail.com>
Mon, 05 Jun 2017 23:47:30 -0700
changeset 410637 f061b418bf20459b682f5be27972c1c1ad8dd575
parent 410636 592139f3c7bab20eeced33e4d32f1bd6208d8f09
child 410638 bee4404d2f888596ded102cf7599d57cfdbf421a
push id7391
push usermtabara@mozilla.com
push dateMon, 12 Jun 2017 13:08:53 +0000
treeherdermozilla-beta@2191d7f87e2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersxidorn
bugs1370458
milestone55.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 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
-