LibWeb: Store CSS color name in CSSRGB

When serializing an sRGB color value that originated from a named color,
it should return the color name converted to ASCII lowercase. This
requires storing the color name (if it has one).

This change also requires explicitly removing the color names when
computing style, because computed color values do not retain their name.
It also requires removing a caching optimization in create_from_color(),
because adding the name means that the cached value might be wrong.

This fixes some WPT subtests, and also required updating some of our own
tests.

Libraries/LibWeb/CSS/Parser/Parser.cpp

@@ -3413,7 +3413,7 @@ RefPtr<CSSStyleValue> Parser::parse_color_value(TokenStream<ComponentValue>& tok
         auto color = Color::from_string(ident);
         if (color.has_value()) {
             transaction.commit();
-            return CSSColorValue::create_from_color(color.release_value());
+            return CSSColorValue::create_from_color(color.release_value(), ident);
         }
         // Otherwise, fall through to the hashless-hex-color case
     }

Libraries/LibWeb/CSS/StyleComputer.cpp

@@ -2425,6 +2425,17 @@ Optional<StyleProperties> StyleComputer::compute_style_impl(DOM::Element& elemen
         start_needed_transitions(*previous_style, style, element, pseudo_element);
     }
 
+    // Remove color names from CSS color values. This is needed because computed values cannot be named colors.
+    for (auto i = to_underlying(CSS::first_property_id); i <= to_underlying(CSS::last_property_id); ++i) {
+        auto property_id = (CSS::PropertyID)i;
+        auto* property = style.maybe_null_property(property_id);
+        if (property && property->is_color()) {
+            auto& color_value = property->as_color();
+            if (color_value.color_type() == CSSColorValue::ColorType::RGB)
+                style.set_property(property_id, CSSColorValue::create_from_color(color_value.to_color({})));
+        }
+    }
+
     return style;
 }
 

Libraries/LibWeb/CSS/StyleValues/CSSColorValue.cpp

@@ -17,32 +17,14 @@
 
 namespace Web::CSS {
 
-ValueComparingNonnullRefPtr<CSSColorValue> CSSColorValue::create_from_color(Color color)
+ValueComparingNonnullRefPtr<CSSColorValue> CSSColorValue::create_from_color(Color color, Optional<FlyString> name)
 {
-    auto make_rgb_color = [](Color const& color) {
-        return CSSRGB::create(
-            NumberStyleValue::create(color.red()),
-            NumberStyleValue::create(color.green()),
-            NumberStyleValue::create(color.blue()),
-            NumberStyleValue::create(color.alpha() / 255.0));
-    };
-
-    if (color.value() == 0) {
-        static auto transparent = make_rgb_color(color);
-        return transparent;
-    }
-
-    if (color == Color::from_rgb(0x000000)) {
-        static auto black = make_rgb_color(color);
-        return black;
-    }
-
-    if (color == Color::from_rgb(0xffffff)) {
-        static auto white = make_rgb_color(color);
-        return white;
-    }
-
-    return make_rgb_color(color);
+    return CSSRGB::create(
+        NumberStyleValue::create(color.red()),
+        NumberStyleValue::create(color.green()),
+        NumberStyleValue::create(color.blue()),
+        NumberStyleValue::create(color.alpha() / 255.0),
+        name);
 }
 
 Optional<double> CSSColorValue::resolve_hue(CSSStyleValue const& style_value)

Libraries/LibWeb/CSS/StyleValues/CSSColorValue.h

@@ -9,6 +9,7 @@
 
 #pragma once
 
+#include <AK/FlyString.h>
 #include <LibGfx/Color.h>
 #include <LibWeb/CSS/CSSStyleValue.h>
 
@@ -17,7 +18,7 @@ namespace Web::CSS {
 // https://drafts.css-houdini.org/css-typed-om-1/#csscolorvalue
 class CSSColorValue : public CSSStyleValue {
 public:
-    static ValueComparingNonnullRefPtr<CSSColorValue> create_from_color(Color color);
+    static ValueComparingNonnullRefPtr<CSSColorValue> create_from_color(Color color, Optional<FlyString> name = {});
     virtual ~CSSColorValue() override = default;
 
     virtual bool has_color() const override { return true; }

Libraries/LibWeb/CSS/StyleValues/CSSRGB.cpp

@@ -71,6 +71,8 @@ bool CSSRGB::equals(CSSStyleValue const& other) const
 String CSSRGB::to_string() const
 {
     // FIXME: Do this properly, taking unresolved calculated values into account.
+    if (m_properties.name.has_value())
+        return m_properties.name.value().to_string().to_ascii_lowercase();
     return serialize_a_srgb_value(to_color({}));
 }
 

Libraries/LibWeb/CSS/StyleValues/CSSRGB.h

@@ -14,13 +14,13 @@ namespace Web::CSS {
 // https://drafts.css-houdini.org/css-typed-om-1/#cssrgb
 class CSSRGB final : public CSSColorValue {
 public:
-    static ValueComparingNonnullRefPtr<CSSRGB> create(ValueComparingNonnullRefPtr<CSSStyleValue> r, ValueComparingNonnullRefPtr<CSSStyleValue> g, ValueComparingNonnullRefPtr<CSSStyleValue> b, ValueComparingRefPtr<CSSStyleValue> alpha = {})
+    static ValueComparingNonnullRefPtr<CSSRGB> create(ValueComparingNonnullRefPtr<CSSStyleValue> r, ValueComparingNonnullRefPtr<CSSStyleValue> g, ValueComparingNonnullRefPtr<CSSStyleValue> b, ValueComparingRefPtr<CSSStyleValue> alpha = {}, Optional<FlyString> name = {})
     {
         // alpha defaults to 1
         if (!alpha)
-            return adopt_ref(*new (nothrow) CSSRGB(move(r), move(g), move(b), NumberStyleValue::create(1)));
+            return adopt_ref(*new (nothrow) CSSRGB(move(r), move(g), move(b), NumberStyleValue::create(1), name));
 
-        return adopt_ref(*new (nothrow) CSSRGB(move(r), move(g), move(b), alpha.release_nonnull()));
+        return adopt_ref(*new (nothrow) CSSRGB(move(r), move(g), move(b), alpha.release_nonnull(), name));
     }
     virtual ~CSSRGB() override = default;
 
@@ -36,9 +36,9 @@ class CSSRGB final : public CSSColorValue {
     virtual bool equals(CSSStyleValue const& other) const override;
 
 private:
-    CSSRGB(ValueComparingNonnullRefPtr<CSSStyleValue> r, ValueComparingNonnullRefPtr<CSSStyleValue> g, ValueComparingNonnullRefPtr<CSSStyleValue> b, ValueComparingNonnullRefPtr<CSSStyleValue> alpha)
+    CSSRGB(ValueComparingNonnullRefPtr<CSSStyleValue> r, ValueComparingNonnullRefPtr<CSSStyleValue> g, ValueComparingNonnullRefPtr<CSSStyleValue> b, ValueComparingNonnullRefPtr<CSSStyleValue> alpha, Optional<FlyString> name = {})
         : CSSColorValue(ColorType::RGB)
-        , m_properties { .r = move(r), .g = move(g), .b = move(b), .alpha = move(alpha) }
+        , m_properties { .r = move(r), .g = move(g), .b = move(b), .alpha = move(alpha), .name = name }
     {
     }
 
@@ -47,6 +47,7 @@ class CSSRGB final : public CSSColorValue {
         ValueComparingNonnullRefPtr<CSSStyleValue> g;
         ValueComparingNonnullRefPtr<CSSStyleValue> b;
         ValueComparingNonnullRefPtr<CSSStyleValue> alpha;
+        Optional<FlyString> name;
         bool operator==(Properties const&) const = default;
     } m_properties;
 };

Tests/LibWeb/Text/expected/HTML/background-shorthand.txt

@@ -1,4 +1,4 @@
-style.cssText = background-color: rgb(255, 255, 0); background-image: none; background-position-x: left 0%; background-position-y: top 0%; background-size: auto auto; background-repeat: repeat repeat; background-attachment: scroll; background-origin: padding-box; background-clip: border-box;
+style.cssText = background-color: yellow; background-image: none; background-position-x: left 0%; background-position-y: top 0%; background-size: auto auto; background-repeat: repeat repeat; background-attachment: scroll; background-origin: padding-box; background-clip: border-box;
 style.length = 9
 style[] =
 1. background-color

Tests/LibWeb/Text/expected/css/PropertyOwningCSSStyleDeclaration-serialized-custom-properties.txt

@@ -1 +1 @@
-test { --color: red; color: rgb(255, 0, 0); }
+test { --color: red; color: red; }

Tests/LibWeb/Text/expected/css/keyframes-css-rules.txt

@@ -1,7 +1,7 @@
-fooRule: [object CSSKeyframesRule] ~ @keyframes "foo" { 0% { color: rgb(0, 0, 0); } 100% { color: rgb(255, 255, 255); } }
+fooRule: [object CSSKeyframesRule] ~ @keyframes "foo" { 0% { color: black; } 100% { color: white; } }
 fooRule.cssRules: [object CSSRuleList]
-fooRule.cssRules[0]: [object CSSKeyframeRule] ~ 0% { color: rgb(0, 0, 0); }
+fooRule.cssRules[0]: [object CSSKeyframeRule] ~ 0% { color: black; }
 fooRule.cssRules[0].parentRule: [object CSSKeyframesRule]
-fooRule.cssRules[0].parentRule: [object CSSKeyframesRule] ~ @keyframes "foo" { 0% { color: rgb(0, 0, 0); } 100% { color: rgb(255, 255, 255); } }
-fooRule.cssRules[0].style.parentRule: [object CSSKeyframeRule] ~ 0% { color: rgb(0, 0, 0); }
+fooRule.cssRules[0].parentRule: [object CSSKeyframesRule] ~ @keyframes "foo" { 0% { color: black; } 100% { color: white; } }
+fooRule.cssRules[0].style.parentRule: [object CSSKeyframeRule] ~ 0% { color: black; }
 fooRule.cssRules[0].style.parentRule === fooRule.cssRules[0]: true

Tests/LibWeb/Text/expected/css/set-style-declaration-shorthand.txt

@@ -28,7 +28,7 @@ Setting flex: ''; becomes...
 Setting border: '1px solid red'; becomes...
   border-width: '1px 1px 1px 1px'
   border-style: 'solid solid solid solid'
-  border-color: 'rgb(255, 0, 0) rgb(255, 0, 0) rgb(255, 0, 0) rgb(255, 0, 0)'
+  border-color: 'red red red red'
   border: ''
   e.style.length: 12
   > [0] border-top-width

Tests/LibWeb/Text/expected/css/style-declaration-parent-rule.txt

@@ -1,4 +1,4 @@
-spanRule: [object CSSStyleRule] ~ span { color: rgb(128, 0, 128); }
-spanRule.style: [object CSSStyleDeclaration] ~ span { color: rgb(128, 0, 128); }
-spanRule.style.parentRule: [object CSSStyleRule] ~ span { color: rgb(128, 0, 128); }
+spanRule: [object CSSStyleRule] ~ span { color: purple; }
+spanRule.style: [object CSSStyleDeclaration] ~ span { color: purple; }
+spanRule.style.parentRule: [object CSSStyleRule] ~ span { color: purple; }
 spanRule.style.parentRule === spanRule: true

Tests/LibWeb/Text/expected/wpt-import/css/css-nesting/cssom.txt

@@ -6,19 +6,19 @@ Rerun
 
 Found 13 tests
 
-1 Pass
-12 Fail
+11 Pass
+2 Fail
 Details
 Result	Test Name	MessageFail	CSSStyleRule is a CSSGroupingRule	
-Fail	Simple CSSOM manipulation of subrules	
-Fail	Simple CSSOM manipulation of subrules 1	
-Fail	Simple CSSOM manipulation of subrules 2	
-Fail	Simple CSSOM manipulation of subrules 3	
-Fail	Simple CSSOM manipulation of subrules 4	
-Fail	Simple CSSOM manipulation of subrules 5	
-Fail	Simple CSSOM manipulation of subrules 6	
-Fail	Simple CSSOM manipulation of subrules 7	
+Pass	Simple CSSOM manipulation of subrules	
+Pass	Simple CSSOM manipulation of subrules 1	
+Pass	Simple CSSOM manipulation of subrules 2	
+Pass	Simple CSSOM manipulation of subrules 3	
+Pass	Simple CSSOM manipulation of subrules 4	
+Pass	Simple CSSOM manipulation of subrules 5	
+Pass	Simple CSSOM manipulation of subrules 6	
+Pass	Simple CSSOM manipulation of subrules 7	
 Fail	Simple CSSOM manipulation of subrules 8	
-Fail	Simple CSSOM manipulation of subrules 9	
-Fail	Simple CSSOM manipulation of subrules 10	
+Pass	Simple CSSOM manipulation of subrules 9	
+Pass	Simple CSSOM manipulation of subrules 10	
 Pass	Mutating the selectorText of outer rule invalidates inner rules	

Tests/LibWeb/Text/expected/wpt-import/css/css-nesting/invalid-inner-rules.txt

@@ -6,7 +6,8 @@ Rerun
 
 Found 2 tests
 
-2 Fail
+1 Pass
+1 Fail
 Details
-Result	Test Name	MessageFail	Simple CSSOM manipulation of subrules	
+Result	Test Name	MessagePass	Simple CSSOM manipulation of subrules	
 Fail	Simple CSSOM manipulation of subrules 1	

Tests/LibWeb/Text/expected/wpt-import/css/css-nesting/serialize-group-rules-with-decls.txt

@@ -6,20 +6,19 @@ Rerun
 
 Found 15 tests
 
-4 Pass
-11 Fail
+15 Pass
 Details
-Result	Test Name	MessageFail	Declarations are serialized on one line, rules on two.	
-Fail	Mixed declarations/rules are on two lines.	
-Fail	Implicit rule is serialized	
-Fail	Implicit rule not removed	
-Fail	Implicit + empty hover rule	
-Fail	Implicit like rule not in first position	
-Fail	Two implicit-like rules	
-Fail	Implicit like rule after decls	
-Fail	Implicit like rule after decls, missing closing braces	
-Fail	Implicit like rule with other selectors	
-Fail	Implicit-like rule in style rule	
+Result	Test Name	MessagePass	Declarations are serialized on one line, rules on two.	
+Pass	Mixed declarations/rules are on two lines.	
+Pass	Implicit rule is serialized	
+Pass	Implicit rule not removed	
+Pass	Implicit + empty hover rule	
+Pass	Implicit like rule not in first position	
+Pass	Two implicit-like rules	
+Pass	Implicit like rule after decls	
+Pass	Implicit like rule after decls, missing closing braces	
+Pass	Implicit like rule with other selectors	
+Pass	Implicit-like rule in style rule	
 Pass	Empty conditional rule	
 Pass	Empty style rule	
 Pass	Empty conditional inside style rule	

Tests/LibWeb/Text/expected/wpt-import/domparsing/style_attribute_html.txt

@@ -0,0 +1,15 @@
+Summary
+
+Harness status: OK
+
+Rerun
+
+Found 4 tests
+
+2 Pass
+2 Fail
+Details
+Result	Test Name	MessagePass	Parsing of initial style attribute	
+Fail	Parsing of invalid style attribute	
+Fail	Parsing of style attribute	
+Pass	Update style.backgroundColor	

Tests/LibWeb/Text/input/wpt-import/domparsing/style_attribute_html.html

@@ -0,0 +1,52 @@
+<!doctype html>
+<meta charset=utf-8>
+<title>Style attribute in HTML</title>
+<script src=../resources/testharness.js></script>
+<script src=../resources/testharnessreport.js></script>
+<script>
+
+var div;
+setup(function() {
+    var input = '<div style="color: red">Foo</div>';
+    var doc = (new DOMParser()).parseFromString(input, 'text/html');
+    div = doc.querySelector('div');
+});
+
+test(function() {
+    var style = div.style;
+    assert_equals(style.cssText, 'color: red;');
+    assert_equals(style.color, 'red');
+    assert_equals(div.getAttribute("style"), 'color: red',
+                  'Value of style attribute should match the string value that was set');
+}, 'Parsing of initial style attribute');
+
+test(function() {
+    var style = div.style;
+    div.setAttribute('style', 'color:: invalid');
+    assert_equals(style.cssText, '');
+    assert_equals(style.color, '');
+    assert_equals(div.getAttribute('style'), 'color:: invalid',
+                  'Value of style attribute should match the string value that was set');
+}, 'Parsing of invalid style attribute');
+
+test(function() {
+    var style = div.style;
+    div.setAttribute('style', 'color: green');
+    assert_equals(style.cssText, 'color: green;');
+    assert_equals(style.color, 'green');
+    assert_equals(div.getAttribute('style'), 'color: green',
+                  'Value of style attribute should match the string value that was set');
+}, 'Parsing of style attribute');
+
+test(function() {
+    var style = div.style;
+    style.backgroundColor = 'blue';
+    assert_equals(style.cssText, 'color: green; background-color: blue;',
+                  'Should not drop the existing style');
+    assert_equals(style.color, 'green',
+                  'Should not drop the existing style');
+    assert_equals(div.getAttribute('style'), 'color: green; background-color: blue;',
+                  'Should update style attribute');
+}, 'Update style.backgroundColor');
+
+</script>