From 443f9e5afb714d73b53d9c2388d4ddd816a5f8fb Mon Sep 17 00:00:00 2001 From: Sam Atkins Date: Fri, 16 May 2025 20:02:16 +0100 Subject: [PATCH] LibWeb/CSS: Make dimension types serialize in resolved form Some dimensions would always serialize in a canonical unit, others never did, and others we manually would do so in their StyleValue. This commit moves all of that into the dimension types, which means for example that Length can apply its special rounding. Our local serialization test now produces the same output as other browsers. :^) --- Libraries/LibWeb/CSS/Angle.cpp | 8 +++++--- Libraries/LibWeb/CSS/Angle.h | 7 ++++--- Libraries/LibWeb/CSS/Flex.cpp | 10 ++++++---- Libraries/LibWeb/CSS/Flex.h | 7 ++++--- Libraries/LibWeb/CSS/Frequency.cpp | 10 ++++++---- Libraries/LibWeb/CSS/Frequency.h | 7 ++++--- Libraries/LibWeb/CSS/Length.cpp | 8 ++++++-- Libraries/LibWeb/CSS/Length.h | 7 ++++--- Libraries/LibWeb/CSS/Resolution.cpp | 10 ++++++---- Libraries/LibWeb/CSS/Resolution.h | 8 ++++---- Libraries/LibWeb/CSS/StyleValues/AngleStyleValue.cpp | 4 +--- Libraries/LibWeb/CSS/StyleValues/FlexStyleValue.h | 2 +- Libraries/LibWeb/CSS/StyleValues/FrequencyStyleValue.h | 2 +- Libraries/LibWeb/CSS/StyleValues/LengthStyleValue.h | 2 +- .../LibWeb/CSS/StyleValues/ResolutionStyleValue.h | 2 +- Libraries/LibWeb/CSS/StyleValues/TimeStyleValue.h | 7 +------ Libraries/LibWeb/CSS/Time.cpp | 8 +++++--- Libraries/LibWeb/CSS/Time.h | 7 ++++--- .../expected/css/media-query-serialization-basic.txt | 4 ++-- .../css/mediaqueries/match-media-parsing.txt | 8 ++++---- 20 files changed, 70 insertions(+), 58 deletions(-) diff --git a/Libraries/LibWeb/CSS/Angle.cpp b/Libraries/LibWeb/CSS/Angle.cpp index 68eb0023201..5985313364e 100644 --- a/Libraries/LibWeb/CSS/Angle.cpp +++ b/Libraries/LibWeb/CSS/Angle.cpp @@ -1,11 +1,11 @@ /* - * Copyright (c) 2022-2023, Sam Atkins + * Copyright (c) 2022-2025, Sam Atkins * * SPDX-License-Identifier: BSD-2-Clause */ -#include "Angle.h" #include +#include #include #include @@ -27,8 +27,10 @@ Angle Angle::percentage_of(Percentage const& percentage) const return Angle { percentage.as_fraction() * m_value, m_type }; } -String Angle::to_string() const +String Angle::to_string(SerializationMode serialization_mode) const { + if (serialization_mode == SerializationMode::ResolvedValue) + return MUST(String::formatted("{}deg", to_degrees())); return MUST(String::formatted("{}{}", raw_value(), unit_name())); } diff --git a/Libraries/LibWeb/CSS/Angle.h b/Libraries/LibWeb/CSS/Angle.h index 5972b2d6935..a3e4cc55d1e 100644 --- a/Libraries/LibWeb/CSS/Angle.h +++ b/Libraries/LibWeb/CSS/Angle.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2022-2023, Sam Atkins + * Copyright (c) 2022-2025, Sam Atkins * * SPDX-License-Identifier: BSD-2-Clause */ @@ -7,13 +7,14 @@ #pragma once #include +#include #include namespace Web::CSS { class Angle { public: - enum class Type { + enum class Type : u8 { Deg, Grad, Rad, @@ -26,7 +27,7 @@ public: static Angle make_degrees(double); Angle percentage_of(Percentage const&) const; - String to_string() const; + String to_string(SerializationMode = SerializationMode::Normal) const; double to_degrees() const; double to_radians() const; diff --git a/Libraries/LibWeb/CSS/Flex.cpp b/Libraries/LibWeb/CSS/Flex.cpp index f33cfa6645a..cd581362c3d 100644 --- a/Libraries/LibWeb/CSS/Flex.cpp +++ b/Libraries/LibWeb/CSS/Flex.cpp @@ -1,10 +1,10 @@ /* - * Copyright (c) 2023, Sam Atkins + * Copyright (c) 2023-2025, Sam Atkins * * SPDX-License-Identifier: BSD-2-Clause */ -#include "Flex.h" +#include #include namespace Web::CSS { @@ -25,9 +25,11 @@ Flex Flex::percentage_of(Percentage const& percentage) const return Flex { percentage.as_fraction() * m_value, m_type }; } -String Flex::to_string() const +String Flex::to_string(SerializationMode serialization_mode) const { - return MUST(String::formatted("{}fr", to_fr())); + if (serialization_mode == SerializationMode::ResolvedValue) + return MUST(String::formatted("{}fr", to_fr())); + return MUST(String::formatted("{}{}", raw_value(), unit_name())); } double Flex::to_fr() const diff --git a/Libraries/LibWeb/CSS/Flex.h b/Libraries/LibWeb/CSS/Flex.h index 941cb83ca3d..e8f8dcb9b2b 100644 --- a/Libraries/LibWeb/CSS/Flex.h +++ b/Libraries/LibWeb/CSS/Flex.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2023, Sam Atkins + * Copyright (c) 2023-2025, Sam Atkins * * SPDX-License-Identifier: BSD-2-Clause */ @@ -8,6 +8,7 @@ #include #include +#include #include namespace Web::CSS { @@ -15,7 +16,7 @@ namespace Web::CSS { // https://drafts.csswg.org/css-grid-2/#typedef-flex class Flex { public: - enum class Type { + enum class Type : u8 { Fr, }; @@ -25,7 +26,7 @@ public: static Flex make_fr(double); Flex percentage_of(Percentage const&) const; - String to_string() const; + String to_string(SerializationMode = SerializationMode::Normal) const; double to_fr() const; Type type() const { return m_type; } diff --git a/Libraries/LibWeb/CSS/Frequency.cpp b/Libraries/LibWeb/CSS/Frequency.cpp index 6996a6d3317..c50f8a60cab 100644 --- a/Libraries/LibWeb/CSS/Frequency.cpp +++ b/Libraries/LibWeb/CSS/Frequency.cpp @@ -1,10 +1,10 @@ /* - * Copyright (c) 2022-2023, Sam Atkins + * Copyright (c) 2022-2025, Sam Atkins * * SPDX-License-Identifier: BSD-2-Clause */ -#include "Frequency.h" +#include #include #include @@ -26,9 +26,11 @@ Frequency Frequency::percentage_of(Percentage const& percentage) const return Frequency { percentage.as_fraction() * m_value, m_type }; } -String Frequency::to_string() const +String Frequency::to_string(SerializationMode serialization_mode) const { - return MUST(String::formatted("{}hz", to_hertz())); + if (serialization_mode == SerializationMode::ResolvedValue) + return MUST(String::formatted("{}hz", to_hertz())); + return MUST(String::formatted("{}{}", raw_value(), unit_name())); } double Frequency::to_hertz() const diff --git a/Libraries/LibWeb/CSS/Frequency.h b/Libraries/LibWeb/CSS/Frequency.h index 04bd8256f88..f4a50550842 100644 --- a/Libraries/LibWeb/CSS/Frequency.h +++ b/Libraries/LibWeb/CSS/Frequency.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2022-2023, Sam Atkins + * Copyright (c) 2022-2025, Sam Atkins * * SPDX-License-Identifier: BSD-2-Clause */ @@ -7,13 +7,14 @@ #pragma once #include +#include #include namespace Web::CSS { class Frequency { public: - enum class Type { + enum class Type : u8 { Hz, kHz }; @@ -24,7 +25,7 @@ public: static Frequency make_hertz(double); Frequency percentage_of(Percentage const&) const; - String to_string() const; + String to_string(SerializationMode = SerializationMode::Normal) const; double to_hertz() const; Type type() const { return m_type; } diff --git a/Libraries/LibWeb/CSS/Length.cpp b/Libraries/LibWeb/CSS/Length.cpp index 6c5805e6e83..7ef46861b14 100644 --- a/Libraries/LibWeb/CSS/Length.cpp +++ b/Libraries/LibWeb/CSS/Length.cpp @@ -1,7 +1,7 @@ /* * Copyright (c) 2020-2024, Andreas Kling * Copyright (c) 2021, Tobias Christiansen - * Copyright (c) 2022-2023, Sam Atkins + * Copyright (c) 2022-2025, Sam Atkins * * SPDX-License-Identifier: BSD-2-Clause */ @@ -198,10 +198,14 @@ CSSPixels Length::to_px_slow_case(Layout::Node const& layout_node) const return viewport_relative_length_to_px(viewport_rect); } -String Length::to_string() const +String Length::to_string(SerializationMode serialization_mode) const { if (is_auto()) return "auto"_string; + // FIXME: Manually skip this for px so we avoid rounding errors in absolute_length_to_px. + // Maybe provide alternative functions that don't produce CSSPixels? + if (serialization_mode == SerializationMode::ResolvedValue && is_absolute() && m_type != Type::Px) + return MUST(String::formatted("{:.5}px", absolute_length_to_px())); return MUST(String::formatted("{:.5}{}", m_value, unit_name())); } diff --git a/Libraries/LibWeb/CSS/Length.h b/Libraries/LibWeb/CSS/Length.h index 686561e4ba2..b11b8a8d28c 100644 --- a/Libraries/LibWeb/CSS/Length.h +++ b/Libraries/LibWeb/CSS/Length.h @@ -1,6 +1,6 @@ /* * Copyright (c) 2018-2024, Andreas Kling - * Copyright (c) 2021-2023, Sam Atkins + * Copyright (c) 2021-2025, Sam Atkins * * SPDX-License-Identifier: BSD-2-Clause */ @@ -10,6 +10,7 @@ #include #include #include +#include #include #include @@ -17,7 +18,7 @@ namespace Web::CSS { class Length { public: - enum class Type { + enum class Type : u8 { // Font-relative Em, Rem, @@ -218,7 +219,7 @@ public: } } - String to_string() const; + String to_string(SerializationMode = SerializationMode::Normal) const; bool operator==(Length const& other) const { diff --git a/Libraries/LibWeb/CSS/Resolution.cpp b/Libraries/LibWeb/CSS/Resolution.cpp index ec70da265b0..4a16d554994 100644 --- a/Libraries/LibWeb/CSS/Resolution.cpp +++ b/Libraries/LibWeb/CSS/Resolution.cpp @@ -1,11 +1,11 @@ /* - * Copyright (c) 2022-2023, Sam Atkins + * Copyright (c) 2022-2025, Sam Atkins * Copyright (c) 2024, Glenn Skrzypczak * * SPDX-License-Identifier: BSD-2-Clause */ -#include "Resolution.h" +#include namespace Web::CSS { @@ -20,9 +20,11 @@ Resolution Resolution::make_dots_per_pixel(double value) return { value, Type::Dppx }; } -String Resolution::to_string() const +String Resolution::to_string(SerializationMode serialization_mode) const { - return MUST(String::formatted("{}dppx", to_dots_per_pixel())); + if (serialization_mode == SerializationMode::ResolvedValue) + return MUST(String::formatted("{}dppx", to_dots_per_pixel())); + return MUST(String::formatted("{}{}", raw_value(), unit_name())); } double Resolution::to_dots_per_pixel() const diff --git a/Libraries/LibWeb/CSS/Resolution.h b/Libraries/LibWeb/CSS/Resolution.h index 8287146a71a..f62012b6107 100644 --- a/Libraries/LibWeb/CSS/Resolution.h +++ b/Libraries/LibWeb/CSS/Resolution.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2022-2023, Sam Atkins + * Copyright (c) 2022-2025, Sam Atkins * * SPDX-License-Identifier: BSD-2-Clause */ @@ -7,13 +7,13 @@ #pragma once #include -#include +#include namespace Web::CSS { class Resolution { public: - enum class Type { + enum class Type : u8 { Dpi, Dpcm, Dppx, @@ -24,7 +24,7 @@ public: Resolution(double value, Type type); static Resolution make_dots_per_pixel(double); - String to_string() const; + String to_string(SerializationMode = SerializationMode::Normal) const; double to_dots_per_pixel() const; Type type() const { return m_type; } diff --git a/Libraries/LibWeb/CSS/StyleValues/AngleStyleValue.cpp b/Libraries/LibWeb/CSS/StyleValues/AngleStyleValue.cpp index 9df73ce5b52..02adabfde23 100644 --- a/Libraries/LibWeb/CSS/StyleValues/AngleStyleValue.cpp +++ b/Libraries/LibWeb/CSS/StyleValues/AngleStyleValue.cpp @@ -21,9 +21,7 @@ AngleStyleValue::~AngleStyleValue() = default; String AngleStyleValue::to_string(SerializationMode serialization_mode) const { - if (serialization_mode == SerializationMode::ResolvedValue) - return MUST(String::formatted("{}deg", m_angle.to_degrees())); - return m_angle.to_string(); + return m_angle.to_string(serialization_mode); } bool AngleStyleValue::equals(CSSStyleValue const& other) const diff --git a/Libraries/LibWeb/CSS/StyleValues/FlexStyleValue.h b/Libraries/LibWeb/CSS/StyleValues/FlexStyleValue.h index e671ad8fdff..2f6f913778d 100644 --- a/Libraries/LibWeb/CSS/StyleValues/FlexStyleValue.h +++ b/Libraries/LibWeb/CSS/StyleValues/FlexStyleValue.h @@ -23,7 +23,7 @@ public: virtual double value() const override { return m_flex.raw_value(); } virtual StringView unit() const override { return m_flex.unit_name(); } - virtual String to_string(SerializationMode) const override { return m_flex.to_string(); } + virtual String to_string(SerializationMode serialization_mode) const override { return m_flex.to_string(serialization_mode); } bool equals(CSSStyleValue const& other) const override { diff --git a/Libraries/LibWeb/CSS/StyleValues/FrequencyStyleValue.h b/Libraries/LibWeb/CSS/StyleValues/FrequencyStyleValue.h index e54967e3a20..3adf46ae3d0 100644 --- a/Libraries/LibWeb/CSS/StyleValues/FrequencyStyleValue.h +++ b/Libraries/LibWeb/CSS/StyleValues/FrequencyStyleValue.h @@ -26,7 +26,7 @@ public: virtual double value() const override { return m_frequency.raw_value(); } virtual StringView unit() const override { return m_frequency.unit_name(); } - virtual String to_string(SerializationMode) const override { return m_frequency.to_string(); } + virtual String to_string(SerializationMode serialization_mode) const override { return m_frequency.to_string(serialization_mode); } bool equals(CSSStyleValue const& other) const override { diff --git a/Libraries/LibWeb/CSS/StyleValues/LengthStyleValue.h b/Libraries/LibWeb/CSS/StyleValues/LengthStyleValue.h index 43472de8928..03392511362 100644 --- a/Libraries/LibWeb/CSS/StyleValues/LengthStyleValue.h +++ b/Libraries/LibWeb/CSS/StyleValues/LengthStyleValue.h @@ -23,7 +23,7 @@ public: virtual double value() const override { return m_length.raw_value(); } virtual StringView unit() const override { return m_length.unit_name(); } - virtual String to_string(SerializationMode) const override { return m_length.to_string(); } + virtual String to_string(SerializationMode serialization_mode) const override { return m_length.to_string(serialization_mode); } virtual ValueComparingNonnullRefPtr absolutized(CSSPixelRect const& viewport_rect, Length::FontMetrics const& font_metrics, Length::FontMetrics const& root_font_metrics) const override; bool equals(CSSStyleValue const& other) const override; diff --git a/Libraries/LibWeb/CSS/StyleValues/ResolutionStyleValue.h b/Libraries/LibWeb/CSS/StyleValues/ResolutionStyleValue.h index bf0b2df1ab0..ab742152a8f 100644 --- a/Libraries/LibWeb/CSS/StyleValues/ResolutionStyleValue.h +++ b/Libraries/LibWeb/CSS/StyleValues/ResolutionStyleValue.h @@ -23,7 +23,7 @@ public: virtual double value() const override { return m_resolution.raw_value(); } virtual StringView unit() const override { return m_resolution.unit_name(); } - virtual String to_string(SerializationMode) const override { return m_resolution.to_string(); } + virtual String to_string(SerializationMode serialization_mode) const override { return m_resolution.to_string(serialization_mode); } bool equals(CSSStyleValue const& other) const override { diff --git a/Libraries/LibWeb/CSS/StyleValues/TimeStyleValue.h b/Libraries/LibWeb/CSS/StyleValues/TimeStyleValue.h index 0608234b9b4..620a962c4cc 100644 --- a/Libraries/LibWeb/CSS/StyleValues/TimeStyleValue.h +++ b/Libraries/LibWeb/CSS/StyleValues/TimeStyleValue.h @@ -26,12 +26,7 @@ public: virtual double value() const override { return m_time.raw_value(); } virtual StringView unit() const override { return m_time.unit_name(); } - virtual String to_string(SerializationMode serialization_mode) const override - { - if (serialization_mode == SerializationMode::ResolvedValue) - return MUST(String::formatted("{}s", m_time.to_seconds())); - return m_time.to_string(); - } + virtual String to_string(SerializationMode serialization_mode) const override { return m_time.to_string(serialization_mode); } bool equals(CSSStyleValue const& other) const override { diff --git a/Libraries/LibWeb/CSS/Time.cpp b/Libraries/LibWeb/CSS/Time.cpp index 1fdfcdfbea9..dbacbdaae38 100644 --- a/Libraries/LibWeb/CSS/Time.cpp +++ b/Libraries/LibWeb/CSS/Time.cpp @@ -1,12 +1,12 @@ /* - * Copyright (c) 2022-2023, Sam Atkins + * Copyright (c) 2022-2025, Sam Atkins * * SPDX-License-Identifier: BSD-2-Clause */ -#include "Time.h" #include #include +#include namespace Web::CSS { @@ -26,8 +26,10 @@ Time Time::percentage_of(Percentage const& percentage) const return Time { percentage.as_fraction() * m_value, m_type }; } -String Time::to_string() const +String Time::to_string(SerializationMode serialization_mode) const { + if (serialization_mode == SerializationMode::ResolvedValue) + return MUST(String::formatted("{}s", to_seconds())); return MUST(String::formatted("{}{}", raw_value(), unit_name())); } diff --git a/Libraries/LibWeb/CSS/Time.h b/Libraries/LibWeb/CSS/Time.h index 6a82c02a70a..c8ce4b63d66 100644 --- a/Libraries/LibWeb/CSS/Time.h +++ b/Libraries/LibWeb/CSS/Time.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2022-2023, Sam Atkins + * Copyright (c) 2022-2025, Sam Atkins * * SPDX-License-Identifier: BSD-2-Clause */ @@ -7,13 +7,14 @@ #pragma once #include +#include #include namespace Web::CSS { class Time { public: - enum class Type { + enum class Type : u8 { S, Ms, }; @@ -24,7 +25,7 @@ public: static Time make_seconds(double); Time percentage_of(Percentage const&) const; - String to_string() const; + String to_string(SerializationMode = SerializationMode::Normal) const; double to_milliseconds() const; double to_seconds() const; diff --git a/Tests/LibWeb/Text/expected/css/media-query-serialization-basic.txt b/Tests/LibWeb/Text/expected/css/media-query-serialization-basic.txt index 300c90b8cd3..4d6ea371e67 100644 --- a/Tests/LibWeb/Text/expected/css/media-query-serialization-basic.txt +++ b/Tests/LibWeb/Text/expected/css/media-query-serialization-basic.txt @@ -2,9 +2,9 @@ } @media screen and (min-width: 20px) and (max-width: 40px) { } -@media screen and (min-resolution: 1dppx) { +@media screen and (min-resolution: 96dpi) { } @media screen and (min-resolution: 2dppx) { } -@media screen and (min-resolution: 2.54dppx) { +@media screen and (min-resolution: 96dpcm) { } diff --git a/Tests/LibWeb/Text/expected/wpt-import/css/mediaqueries/match-media-parsing.txt b/Tests/LibWeb/Text/expected/wpt-import/css/mediaqueries/match-media-parsing.txt index 83c8fafd82e..9d4b9a5453d 100644 --- a/Tests/LibWeb/Text/expected/wpt-import/css/mediaqueries/match-media-parsing.txt +++ b/Tests/LibWeb/Text/expected/wpt-import/css/mediaqueries/match-media-parsing.txt @@ -2,8 +2,8 @@ Harness status: OK Found 34 tests -25 Pass -9 Fail +27 Pass +7 Fail Pass Test parsing '' with matchMedia Pass Test parsing ' ' with matchMedia Pass Test parsing 'all' with matchMedia @@ -32,8 +32,8 @@ Pass Test parsing '(resolution: calc(2x))' with matchMedia Fail Test parsing '(max-resolution: 7x)' with matchMedia Pass Test parsing '(max-resolution: calc(7x))' with matchMedia Pass Test parsing '(resolution: 2dppx)' with matchMedia -Fail Test parsing '(resolution: 600dpi)' with matchMedia -Fail Test parsing '(resolution: 77dpcm)' with matchMedia +Pass Test parsing '(resolution: 600dpi)' with matchMedia +Pass Test parsing '(resolution: 77dpcm)' with matchMedia Pass Test parsing '(resolution: calc(1x + 2x))' with matchMedia Pass Test parsing '(resolution: calc(5x - 2x))' with matchMedia Pass Test parsing '(resolution: calc(1x * 3))' with matchMedia