LibWeb: Don't serialize omitted box-shadow and text-shadow values

Author: https://github.com/tcl3
Commit: d89a67ffee
Pull-request: https://github.com/LadybirdBrowser/ladybird/pull/5087

Libraries/LibWeb/CSS/Parser/PropertyParsing.cpp

@@ -1921,26 +1921,16 @@ RefPtr<CSSStyleValue const> Parser::parse_single_shadow_value(TokenStream<Compon
         return nullptr;
     }
 
-    // If color is absent, default to `currentColor`
-    if (!color)
-        color = CSSKeywordValue::create(Keyword::Currentcolor);
-
     // x/y offsets are required
     if (!offset_x || !offset_y)
         return nullptr;
 
-    // Other lengths default to 0
-    if (!blur_radius)
-        blur_radius = LengthStyleValue::create(Length::make_px(0));
-    if (!spread_distance)
-        spread_distance = LengthStyleValue::create(Length::make_px(0));
-
     // Placement is outer by default
     if (!placement.has_value())
         placement = ShadowPlacement::Outer;
 
     transaction.commit();
-    return ShadowStyleValue::create(color.release_nonnull(), offset_x.release_nonnull(), offset_y.release_nonnull(), blur_radius.release_nonnull(), spread_distance.release_nonnull(), placement.release_value());
+    return ShadowStyleValue::create(color, offset_x.release_nonnull(), offset_y.release_nonnull(), blur_radius, spread_distance, placement.release_value());
 }
 
 RefPtr<CSSStyleValue const> Parser::parse_rotate_value(TokenStream<ComponentValue>& tokens)

Libraries/LibWeb/CSS/StyleValues/ShadowStyleValue.cpp

@@ -8,6 +8,8 @@
  */
 
 #include <LibWeb/CSS/Serialize.h>
+#include <LibWeb/CSS/StyleValues/CSSKeywordValue.h>
+#include <LibWeb/CSS/StyleValues/LengthStyleValue.h>
 #include <LibWeb/CSS/StyleValues/ShadowStyleValue.h>
 
 namespace Web::CSS {
@@ -15,19 +17,56 @@ namespace Web::CSS {
 String ShadowStyleValue::to_string(SerializationMode mode) const
 {
     StringBuilder builder;
-    builder.appendff("{} {} {} {} {}", m_properties.color->to_string(mode), m_properties.offset_x->to_string(mode), m_properties.offset_y->to_string(mode), m_properties.blur_radius->to_string(mode), m_properties.spread_distance->to_string(mode));
+    if (m_properties.color)
+        builder.append(m_properties.color->to_string(mode));
+
+    if (!builder.is_empty())
+        builder.append(' ');
+    builder.appendff("{} {}", m_properties.offset_x->to_string(mode), m_properties.offset_y->to_string(mode));
+
+    auto append_value = [&](ValueComparingRefPtr<CSSStyleValue const> const& value) {
+        if (!value)
+            return;
+        if (!builder.is_empty())
+            builder.append(' ');
+        builder.append(value->to_string(mode));
+    };
+    append_value(m_properties.blur_radius);
+    append_value(m_properties.spread_distance);
+
     if (m_properties.placement == ShadowPlacement::Inner)
         builder.append(" inset"sv);
     return MUST(builder.to_string());
 }
 
+ValueComparingNonnullRefPtr<CSSStyleValue const> ShadowStyleValue::color() const
+{
+    if (!m_properties.color)
+        return CSSKeywordValue::create(Keyword::Currentcolor);
+    return *m_properties.color;
+}
+
+ValueComparingNonnullRefPtr<CSSStyleValue const> ShadowStyleValue::blur_radius() const
+{
+    if (!m_properties.blur_radius)
+        return LengthStyleValue::create(Length::make_px(0));
+    return *m_properties.blur_radius;
+}
+
+ValueComparingNonnullRefPtr<CSSStyleValue const> ShadowStyleValue::spread_distance() const
+{
+    if (!m_properties.spread_distance)
+        return LengthStyleValue::create(Length::make_px(0));
+    return *m_properties.spread_distance;
+}
+
 ValueComparingNonnullRefPtr<CSSStyleValue const> ShadowStyleValue::absolutized(CSSPixelRect const& viewport_rect, Length::FontMetrics const& font_metrics, Length::FontMetrics const& root_font_metrics) const
 {
-    auto absolutized_offset_x = m_properties.offset_x->absolutized(viewport_rect, font_metrics, root_font_metrics);
+    auto absolutized_offset_x = offset_x()->absolutized(viewport_rect, font_metrics, root_font_metrics);
-    auto absolutized_offset_y = m_properties.offset_y->absolutized(viewport_rect, font_metrics, root_font_metrics);
+    auto absolutized_offset_y = offset_y()->absolutized(viewport_rect, font_metrics, root_font_metrics);
-    auto absolutized_blur_radius = m_properties.blur_radius->absolutized(viewport_rect, font_metrics, root_font_metrics);
+    auto absolutized_blur_radius = blur_radius()->absolutized(viewport_rect, font_metrics, root_font_metrics);
-    auto absolutized_spread_distance = m_properties.spread_distance->absolutized(viewport_rect, font_metrics, root_font_metrics);
+    auto absolutized_spread_distance = spread_distance()->absolutized(viewport_rect, font_metrics, root_font_metrics);
-    return ShadowStyleValue::create(m_properties.color, absolutized_offset_x, absolutized_offset_y, absolutized_blur_radius, absolutized_spread_distance, m_properties.placement);
+    return create(color(), absolutized_offset_x, absolutized_offset_y, absolutized_blur_radius, absolutized_spread_distance, placement());
 }
 
 }

Libraries/LibWeb/CSS/StyleValues/ShadowStyleValue.h

@@ -23,22 +23,22 @@ enum class ShadowPlacement {
 class ShadowStyleValue final : public StyleValueWithDefaultOperators<ShadowStyleValue> {
 public:
     static ValueComparingNonnullRefPtr<ShadowStyleValue const> create(
-        ValueComparingNonnullRefPtr<CSSStyleValue const> color,
+        ValueComparingRefPtr<CSSStyleValue const> color,
         ValueComparingNonnullRefPtr<CSSStyleValue const> offset_x,
         ValueComparingNonnullRefPtr<CSSStyleValue const> offset_y,
-        ValueComparingNonnullRefPtr<CSSStyleValue const> blur_radius,
+        ValueComparingRefPtr<CSSStyleValue const> blur_radius,
-        ValueComparingNonnullRefPtr<CSSStyleValue const> spread_distance,
+        ValueComparingRefPtr<CSSStyleValue const> spread_distance,
         ShadowPlacement placement)
     {
         return adopt_ref(*new (nothrow) ShadowStyleValue(move(color), move(offset_x), move(offset_y), move(blur_radius), move(spread_distance), placement));
     }
     virtual ~ShadowStyleValue() override = default;
 
-    ValueComparingNonnullRefPtr<CSSStyleValue const> const& color() const { return m_properties.color; }
+    ValueComparingNonnullRefPtr<CSSStyleValue const> color() const;
-    ValueComparingNonnullRefPtr<CSSStyleValue const> const& offset_x() const { return m_properties.offset_x; }
+    ValueComparingNonnullRefPtr<CSSStyleValue const> offset_x() const { return m_properties.offset_x; }
-    ValueComparingNonnullRefPtr<CSSStyleValue const> const& offset_y() const { return m_properties.offset_y; }
+    ValueComparingNonnullRefPtr<CSSStyleValue const> offset_y() const { return m_properties.offset_y; }
-    ValueComparingNonnullRefPtr<CSSStyleValue const> const& blur_radius() const { return m_properties.blur_radius; }
+    ValueComparingNonnullRefPtr<CSSStyleValue const> blur_radius() const;
-    ValueComparingNonnullRefPtr<CSSStyleValue const> const& spread_distance() const { return m_properties.spread_distance; }
+    ValueComparingNonnullRefPtr<CSSStyleValue const> spread_distance() const;
     ShadowPlacement placement() const { return m_properties.placement; }
 
     virtual String to_string(SerializationMode) const override;
@@ -47,11 +47,11 @@ public:
 
 private:
     ShadowStyleValue(
-        ValueComparingNonnullRefPtr<CSSStyleValue const> color,
+        ValueComparingRefPtr<CSSStyleValue const> color,
         ValueComparingNonnullRefPtr<CSSStyleValue const> offset_x,
         ValueComparingNonnullRefPtr<CSSStyleValue const> offset_y,
-        ValueComparingNonnullRefPtr<CSSStyleValue const> blur_radius,
+        ValueComparingRefPtr<CSSStyleValue const> blur_radius,
-        ValueComparingNonnullRefPtr<CSSStyleValue const> spread_distance,
+        ValueComparingRefPtr<CSSStyleValue const> spread_distance,
         ShadowPlacement placement)
         : StyleValueWithDefaultOperators(Type::Shadow)
         , m_properties {
@@ -68,11 +68,11 @@ private:
     virtual ValueComparingNonnullRefPtr<CSSStyleValue const> absolutized(CSSPixelRect const& viewport_rect, Length::FontMetrics const& font_metrics, Length::FontMetrics const& root_font_metrics) const override;
 
     struct Properties {
-        ValueComparingNonnullRefPtr<CSSStyleValue const> color;
+        ValueComparingRefPtr<CSSStyleValue const> color;
         ValueComparingNonnullRefPtr<CSSStyleValue const> offset_x;
         ValueComparingNonnullRefPtr<CSSStyleValue const> offset_y;
-        ValueComparingNonnullRefPtr<CSSStyleValue const> blur_radius;
+        ValueComparingRefPtr<CSSStyleValue const> blur_radius;
-        ValueComparingNonnullRefPtr<CSSStyleValue const> spread_distance;
+        ValueComparingRefPtr<CSSStyleValue const> spread_distance;
         ShadowPlacement placement;
         bool operator==(Properties const&) const = default;
     } m_properties;

Tests/LibWeb/Text/expected/wpt-import/css/css-backgrounds/parsing/box-shadow-valid.txt

@@ -0,0 +1,43 @@
+Harness status: OK
+
+Found 38 tests
+
+38 Pass
+Pass	e.style['box-shadow'] = "none" should set the property value
+Pass	e.style['box-shadow'] = "1px 2px" should set the property value
+Pass	e.style['box-shadow'] = "red 1px 2px 3px -4px inset" should set the property value
+Pass	e.style['box-shadow'] = "inset 1px 2px red" should set the property value
+Pass	e.style['box-shadow'] = "1px -2px inset, red -3px 4px" should set the property value
+Pass	e.style['box-shadow'] = "inset 1px -2px, -3px 4px red" should set the property value
+Pass	e.style['box-shadow'] = "4px 4px green" should set the property value
+Pass	e.style['box-shadow'] = "green -4px 4px" should set the property value
+Pass	e.style['box-shadow'] = "-4px 4px 0 green" should set the property value
+Pass	e.style['box-shadow'] = "green -4px 4px 0" should set the property value
+Pass	e.style['box-shadow'] = "-4px 4px 0 0 green" should set the property value
+Pass	e.style['box-shadow'] = "green -4px 4px 0 0" should set the property value
+Pass	e.style['box-shadow'] = "4px -4px inset" should set the property value
+Pass	e.style['box-shadow'] = "inset 4px -4px" should set the property value
+Pass	e.style['box-shadow'] = "4px -4px 0 inset" should set the property value
+Pass	e.style['box-shadow'] = "inset 4px -4px 0" should set the property value
+Pass	e.style['box-shadow'] = "4px -4px 0 0 inset" should set the property value
+Pass	e.style['box-shadow'] = "inset 4px -4px 0 0" should set the property value
+Pass	e.style['box-shadow'] = "4px -4px green inset" should set the property value
+Pass	e.style['box-shadow'] = "4px -4px inset green" should set the property value
+Pass	e.style['box-shadow'] = "inset green 4px -4px" should set the property value
+Pass	e.style['box-shadow'] = "green inset 4px -4px" should set the property value
+Pass	e.style['box-shadow'] = "green 4px -4px inset" should set the property value
+Pass	e.style['box-shadow'] = "inset 4px -4px green" should set the property value
+Pass	e.style['box-shadow'] = "inset green 4px -4px 0" should set the property value
+Pass	e.style['box-shadow'] = "green inset 4px -4px 0" should set the property value
+Pass	e.style['box-shadow'] = "4px -4px 0 green inset" should set the property value
+Pass	e.style['box-shadow'] = "4px -4px 0 inset green" should set the property value
+Pass	e.style['box-shadow'] = "green 4px -4px 0 inset" should set the property value
+Pass	e.style['box-shadow'] = "inset 4px -4px 0 green" should set the property value
+Pass	e.style['box-shadow'] = "inset green 4px -4px 0 0" should set the property value
+Pass	e.style['box-shadow'] = "green inset 4px -4px 0 0" should set the property value
+Pass	e.style['box-shadow'] = "4px -4px 0 0 green inset" should set the property value
+Pass	e.style['box-shadow'] = "4px -4px 0 0 inset green" should set the property value
+Pass	e.style['box-shadow'] = "green 4px -4px 0 0 inset" should set the property value
+Pass	e.style['box-shadow'] = "inset 4px -4px 0 0 green" should set the property value
+Pass	e.style['box-shadow'] = "1px 1px calc(-1px)" should set the property value
+Pass	e.style['box-shadow'] = "1px 1px calc(1em - 2px)" should set the property value

Tests/LibWeb/Text/expected/wpt-import/css/css-text-decor/text-shadow/parsing/text-shadow-valid.txt

@@ -0,0 +1,15 @@
+Harness status: OK
+
+Found 10 tests
+
+10 Pass
+Pass	e.style['text-shadow'] = "none" should set the property value
+Pass	e.style['text-shadow'] = "10px 20px" should set the property value
+Pass	e.style['text-shadow'] = "10px 20px 30px" should set the property value
+Pass	e.style['text-shadow'] = "calc(1em + 2px) calc(3em + 4px) calc(5em + 6px)" should set the property value
+Pass	e.style['text-shadow'] = "-10px 20px 30px" should set the property value
+Pass	e.style['text-shadow'] = "10px -20px 30px" should set the property value
+Pass	e.style['text-shadow'] = "rgb(255, 0, 0) 10px 20px" should set the property value
+Pass	e.style['text-shadow'] = "10px 20px 30px lime" should set the property value
+Pass	e.style['text-shadow'] = "10px 20px, 30px 40px" should set the property value
+Pass	e.style['text-shadow'] = "lime 10px 20px 30px, blue 40px 50px" should set the property value

Tests/LibWeb/Text/input/wpt-import/css/css-backgrounds/parsing/box-shadow-valid.html

@@ -0,0 +1,67 @@
+<!DOCTYPE html>
+<html>
+<head>
+<meta charset="utf-8">
+<title>CSS Backgrounds and Borders Module Level 3: parsing box-shadow with valid values</title>
+<link rel="author" title="Elika J. Etemad" href="http://fantasai.inkedblade.net/contact"/>
+<link rel="author" title="Eric Willigers" href="mailto:ericwilligers@chromium.org">
+<link rel="help" href="https://drafts.csswg.org/css-backgrounds/#box-shadow">
+<meta name="assert" content="box-shadow supports the full grammar 'none | <shadow>#'.">
+<meta name="assert" content="Box shadow color, inset, and length parameters can be mixed in any order." />
+<script src="../../../resources/testharness.js"></script>
+<script src="../../../resources/testharnessreport.js"></script>
+<script src="../../../css/support/parsing-testcommon.js"></script>
+</head>
+<body>
+<script>
+test_valid_value("box-shadow", "none");
+test_valid_value("box-shadow", "1px 2px");
+test_valid_value("box-shadow", "red 1px 2px 3px -4px inset"); // Edge serializes as "inset 1px 2px 3px -4px red"
+test_valid_value("box-shadow", "inset 1px 2px red", "red 1px 2px inset");
+test_valid_value("box-shadow", "1px -2px inset, red -3px 4px"); // Edge serializes as "inset 1px -2px, -3px 4px red"
+test_valid_value("box-shadow", "inset 1px -2px, -3px 4px red", "1px -2px inset, red -3px 4px");
+
+
+// color only
+test_valid_value("box-shadow", "4px 4px green", "green 4px 4px");
+test_valid_value("box-shadow", "green -4px 4px", "green -4px 4px");
+test_valid_value("box-shadow", "-4px 4px 0 green", "green -4px 4px 0px");
+test_valid_value("box-shadow", "green -4px 4px 0", "green -4px 4px 0px");
+test_valid_value("box-shadow", "-4px 4px 0 0 green", "green -4px 4px 0px 0px");
+test_valid_value("box-shadow", "green -4px 4px 0 0", "green -4px 4px 0px 0px");
+
+// inset only
+test_valid_value("box-shadow", "4px -4px inset", "4px -4px inset");
+test_valid_value("box-shadow", "inset 4px -4px", "4px -4px inset");
+test_valid_value("box-shadow", "4px -4px 0 inset", "4px -4px 0px inset");
+test_valid_value("box-shadow", "inset 4px -4px 0", "4px -4px 0px inset");
+test_valid_value("box-shadow", "4px -4px 0 0 inset", "4px -4px 0px 0px inset");
+test_valid_value("box-shadow", "inset 4px -4px 0 0", "4px -4px 0px 0px inset");
+
+// color and inset
+test_valid_value("box-shadow", "4px -4px green inset", "green 4px -4px inset");
+test_valid_value("box-shadow", "4px -4px inset green", "green 4px -4px inset");
+test_valid_value("box-shadow", "inset green 4px -4px", "green 4px -4px inset");
+test_valid_value("box-shadow", "green inset 4px -4px", "green 4px -4px inset");
+test_valid_value("box-shadow", "green 4px -4px inset", "green 4px -4px inset");
+test_valid_value("box-shadow", "inset 4px -4px green", "green 4px -4px inset");
+test_valid_value("box-shadow", "inset green 4px -4px 0", "green 4px -4px 0px inset");
+test_valid_value("box-shadow", "green inset 4px -4px 0", "green 4px -4px 0px inset");
+test_valid_value("box-shadow", "4px -4px 0 green inset", "green 4px -4px 0px inset");
+test_valid_value("box-shadow", "4px -4px 0 inset green", "green 4px -4px 0px inset");
+test_valid_value("box-shadow", "green 4px -4px 0 inset", "green 4px -4px 0px inset");
+test_valid_value("box-shadow", "inset 4px -4px 0 green", "green 4px -4px 0px inset");
+test_valid_value("box-shadow", "inset green 4px -4px 0 0", "green 4px -4px 0px 0px inset");
+test_valid_value("box-shadow", "green inset 4px -4px 0 0", "green 4px -4px 0px 0px inset");
+test_valid_value("box-shadow", "4px -4px 0 0 green inset", "green 4px -4px 0px 0px inset");
+test_valid_value("box-shadow", "4px -4px 0 0 inset green", "green 4px -4px 0px 0px inset");
+test_valid_value("box-shadow", "green 4px -4px 0 0 inset", "green 4px -4px 0px 0px inset");
+test_valid_value("box-shadow", "inset 4px -4px 0 0 green", "green 4px -4px 0px 0px inset");
+
+// No parse-time range-checking for <blur-radius> given as a math function
+// https://drafts.csswg.org/css-values-4/#calc-range
+test_valid_value("box-shadow", "1px 1px calc(-1px)", "1px 1px calc(-1px)");
+test_valid_value("box-shadow", "1px 1px calc(1em - 2px)", "1px 1px calc(1em - 2px)");
+</script>
+</body>
+</html>

Tests/LibWeb/Text/input/wpt-import/css/css-text-decor/text-shadow/parsing/text-shadow-valid.html

@@ -0,0 +1,22 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<title>CSS Text Decoration Test: Parsing text-shadow with valid values</title>
+<link rel="help" href="https://drafts.csswg.org/css-text-decor-3/#text-shadow-property">
+<meta name="assert" content="text-shadow supports the full grammar 'none | [ <color>? && <length>{2,3} ]#'.">
+<script src="../../../../resources/testharness.js"></script>
+<script src="../../../../resources/testharnessreport.js"></script>
+<script src="../../../../css/support/parsing-testcommon.js"></script>
+<script>
+test_valid_value("text-shadow", "none");
+
+test_valid_value("text-shadow", "10px 20px");
+test_valid_value("text-shadow", "10px 20px 30px");
+test_valid_value("text-shadow", "calc(1em + 2px) calc(3em + 4px) calc(5em + 6px)");
+test_valid_value("text-shadow", "-10px 20px 30px");
+test_valid_value("text-shadow", "10px -20px 30px");
+test_valid_value("text-shadow", "rgb(255, 0, 0) 10px 20px");
+test_valid_value("text-shadow", "10px 20px 30px lime", "lime 10px 20px 30px");
+
+test_valid_value("text-shadow", "10px 20px, 30px 40px");
+test_valid_value("text-shadow", "lime 10px 20px 30px, blue 40px 50px");
+</script>