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. This fixes at least 2 WPT subtests.
LadybirdBrowser · Nov 25, 2024 · 1280920 · 1280920
1 parent 6f52064
commit 1280920
Show file tree

Hide file tree

Showing 8 changed files with 93 additions and 10 deletions.
diff --git a/Libraries/LibWeb/CSS/Parser/Parser.cpp b/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
     }

diff --git a/Libraries/LibWeb/CSS/StyleComputer.cpp b/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;
 }
 

diff --git a/Libraries/LibWeb/CSS/StyleValues/CSSColorValue.cpp b/Libraries/LibWeb/CSS/StyleValues/CSSColorValue.cpp
@@ -17,14 +17,15 @@
 
 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) {
+    auto make_rgb_color = [name](Color const& color) {
         return CSSRGB::create(
             NumberStyleValue::create(color.red()),
             NumberStyleValue::create(color.green()),
             NumberStyleValue::create(color.blue()),
-            NumberStyleValue::create(color.alpha() / 255.0));
+            NumberStyleValue::create(color.alpha() / 255.0),
+            name);
     };
 
     if (color.value() == 0) {

diff --git a/Libraries/LibWeb/CSS/StyleValues/CSSColorValue.h b/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; }

diff --git a/Libraries/LibWeb/CSS/StyleValues/CSSRGB.cpp b/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({}));
 }
 

diff --git a/Libraries/LibWeb/CSS/StyleValues/CSSRGB.h b/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;
 };

diff --git a/Tests/LibWeb/Text/expected/wpt-import/domparsing/style_attribute_html.txt b/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	
diff --git a/Tests/LibWeb/Text/input/wpt-import/domparsing/style_attribute_html.html b/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>