Changeset 200501 in webkit

Timestamp:

May 5, 2016 6:26:26 PM (8 years ago)

Author:

dino@apple.com

Message:

4 and 8 digit color attributes should use the crazypants algorithm, not the CSS one.
​https://bugs.webkit.org/show_bug.cgi?id=157402
<rdar://problem/26131142>

Reviewed by Simon Fraser.

Source/WebCore:

Noel and Tab from Google alerted me to the fact I broke some legacy
HTML color attributes when I added support for 4 and 8 digit hex colors
in CSS. The fix is to favor the "crazy" parsing unless we know it is a 3 or 6
digit hex value (in which case we try the CSS algorithm first).

Covered by reverting an existing test.

• html/HTMLElement.cpp:

(WebCore::HTMLElement::addHTMLColorToStyle):

LayoutTests:

Restore the version that was there before I added support for
4 and 8 digit hex colors in CSS.

• fast/dom/attribute-legacy-colors-expected.txt:
• fast/dom/script-tests/attribute-legacy-colors.js:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/dom/attribute-legacy-colors-expected.txt (modified) (2 diffs)
• LayoutTests/fast/dom/script-tests/attribute-legacy-colors.js (modified) (2 diffs)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/html/HTMLElement.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2016-05-05  Dean Jackson  <dino@apple.com>
+
+        4 and 8 digit color attributes should use the crazypants algorithm, not the CSS one.
+        https://bugs.webkit.org/show_bug.cgi?id=157402
+        <rdar://problem/26131142>
+
+        Reviewed by Simon Fraser.
+
+        Restore the version that was there before I added support for
+        4 and 8 digit hex colors in CSS.
+
+        * fast/dom/attribute-legacy-colors-expected.txt:
+        * fast/dom/script-tests/attribute-legacy-colors.js:
+
 2016-05-05  Joseph Pecoraro  <pecoraro@apple.com>
 

trunk/LayoutTests/fast/dom/attribute-legacy-colors-expected.txt

@@ -17 +17 @@
 PASS document.body.bgColor='f00';getComputedStyle(document.body).backgroundColor; is 'rgb(15, 0, 0)'
 PASS document.body.bgColor='ff0000';getComputedStyle(document.body).backgroundColor; is 'rgb(255, 0, 0)'
+PASS document.body.bgColor='#00000000';getComputedStyle(document.body).backgroundColor; is 'rgb(0, 0, 0)'
 PASS document.body.bgColor='foo';getComputedStyle(document.body).backgroundColor; is 'rgb(15, 0, 0)'
 PASS document.body.bgColor='cheese';getComputedStyle(document.body).backgroundColor; is 'rgb(192, 238, 14)'
@@ -35 +36 @@
 PASS document.body.bgColor='#55';getComputedStyle(document.body).backgroundColor; is 'rgb(5, 5, 0)'
 PASS document.body.bgColor='#555';getComputedStyle(document.body).backgroundColor; is 'rgb(85, 85, 85)'
+PASS document.body.bgColor='#5555';getComputedStyle(document.body).backgroundColor; is 'rgb(85, 85, 0)'
 PASS document.body.bgColor='#55555';getComputedStyle(document.body).backgroundColor; is 'rgb(85, 85, 80)'
 PASS document.body.bgColor='#555555';getComputedStyle(document.body).backgroundColor; is 'rgb(85, 85, 85)'
 PASS document.body.bgColor='#5555555';getComputedStyle(document.body).backgroundColor; is 'rgb(85, 85, 80)'
+PASS document.body.bgColor='#55555555';getComputedStyle(document.body).backgroundColor; is 'rgb(85, 85, 85)'
 PASS document.body.bgColor='5';getComputedStyle(document.body).backgroundColor; is 'rgb(5, 0, 0)'
 PASS document.body.bgColor='55';getComputedStyle(document.body).backgroundColor; is 'rgb(5, 5, 0)'

trunk/LayoutTests/fast/dom/script-tests/attribute-legacy-colors.js

@@ -16 +16 @@
         {'test':'f00', 'expected':[15, 0, 0]},
         {'test':'ff0000', 'expected':[255, 0, 0]},
+        {'test':'#00000000', 'expected':[0, 0, 0]},
         {'test':'foo', 'expected':[15, 0, 0]},
         {'test':'cheese', 'expected':[192, 238, 14]},
@@ -34 +35 @@
         {'test':'#55', 'expected':[5, 5, 0]},
         {'test':'#555', 'expected':[85, 85, 85]},
+        {'test':'#5555', 'expected':[85, 85, 0]},
         {'test':'#55555', 'expected':[85, 85, 80]},
         {'test':'#555555', 'expected':[85, 85, 85]},
         {'test':'#5555555', 'expected':[85, 85, 80]},
+        {'test':'#55555555', 'expected':[85, 85, 85]},
         {'test':'5', 'expected':[5, 0, 0]},
         {'test':'55', 'expected':[5, 5, 0]},

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2016-05-05  Dean Jackson  <dino@apple.com>
+
+        4 and 8 digit color attributes should use the crazypants algorithm, not the CSS one.
+        https://bugs.webkit.org/show_bug.cgi?id=157402
+        <rdar://problem/26131142>
+
+        Reviewed by Simon Fraser.
+
+        Noel and Tab from Google alerted me to the fact I broke some legacy
+        HTML color attributes when I added support for 4 and 8 digit hex colors
+        in CSS. The fix is to favor the "crazy" parsing unless we know it is a 3 or 6
+        digit hex value (in which case we try the CSS algorithm first).
+
+        Covered by reverting an existing test.
+
+        * html/HTMLElement.cpp:
+        (WebCore::HTMLElement::addHTMLColorToStyle):
+
 2016-05-05  Brady Eidson  <beidson@apple.com>
 

trunk/Source/WebCore/html/HTMLElement.cpp

@@ -1140 +1140 @@
 
     // If the string is a named CSS color or a 3/6-digit hex color, use that.
-    Color parsedColor(colorString);
-    if (!parsedColor.isValid())
-        parsedColor.setRGB(parseColorStringWithCrazyLegacyRules(colorString));
-
-    style.setProperty(propertyID, CSSValuePool::singleton().createColorValue(parsedColor.rgb()));
+    // We can't use the default Color constructor because it accepts
+    // 4/8-digit hex, which conflict with some legacy HTML content using attributes.
+
+    Color color;
+
+    if ((colorString.length() == 4 || colorString.length() == 7) && colorString[0] == '#')
+        color = Color(colorString);
+    if (!color.isValid())
+        color.setNamedColor(colorString);
+    if (!color.isValid())
+        color.setRGB(parseColorStringWithCrazyLegacyRules(colorString));
+
+    style.setProperty(propertyID, CSSValuePool::singleton().createColorValue(color.rgb()));
 }