From c59ab7cc8ba9c458be5fa3e73b2b8ffdfae440c0 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Sat, 26 Feb 2022 01:35:25 +0100 Subject: [PATCH] LibWeb: Make StyleValue absolutization non-destructive Instead of awkwardly visiting and mutating lengths inside StyleValues, we now simply create a new StyleValue instead. This fixes an issue where inherited relative lengths could get absolutized using a parent as reference, and then not having the correct values when used in a child context. --- .../Libraries/LibWeb/CSS/StyleComputer.cpp | 12 ++--- Userland/Libraries/LibWeb/CSS/StyleValue.cpp | 45 +++++++++++++++++++ Userland/Libraries/LibWeb/CSS/StyleValue.h | 32 +++---------- 3 files changed, 53 insertions(+), 36 deletions(-) diff --git a/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp b/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp index 8ef6a74ae8..4686931859 100644 --- a/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp +++ b/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp @@ -909,17 +909,11 @@ void StyleComputer::absolutize_values(StyleProperties& style, DOM::Element const float root_font_size = 10; float font_size = style.property(CSS::PropertyID::FontSize).value()->to_length().to_px(viewport_rect, font_metrics, root_font_size, root_font_size); - for (auto& value_slot : style.m_property_values) { + for (size_t i = 0; i < style.m_property_values.size(); ++i) { + auto& value_slot = style.m_property_values[i]; if (!value_slot) continue; - value_slot->visit_lengths([&](Length& length) { - if (length.is_px()) - return; - if (length.is_absolute() || length.is_relative()) { - auto px = length.to_px(viewport_rect, font_metrics, font_size, root_font_size); - length = Length::make_px(px); - } - }); + value_slot = value_slot->absolutized(viewport_rect, font_metrics, font_size, root_font_size); } } diff --git a/Userland/Libraries/LibWeb/CSS/StyleValue.cpp b/Userland/Libraries/LibWeb/CSS/StyleValue.cpp index 0aab0d5ab4..4b6e9ccda1 100644 --- a/Userland/Libraries/LibWeb/CSS/StyleValue.cpp +++ b/Userland/Libraries/LibWeb/CSS/StyleValue.cpp @@ -1405,4 +1405,49 @@ NonnullRefPtr LengthStyleValue::create(Length const& length) return adopt_ref(*new LengthStyleValue(length)); } +static Optional absolutized_length(CSS::Length const& length, Gfx::IntRect const& viewport_rect, Gfx::FontMetrics const& font_metrics, float font_size, float root_font_size) +{ + if (length.is_px()) + return {}; + if (length.is_absolute() || length.is_relative()) { + auto px = length.to_px(viewport_rect, font_metrics, font_size, root_font_size); + return CSS::Length::make_px(px); + } + return {}; +} + +NonnullRefPtr StyleValue::absolutized(Gfx::IntRect const&, Gfx::FontMetrics const&, float, float) const +{ + return *this; +} + +NonnullRefPtr LengthStyleValue::absolutized(Gfx::IntRect const& viewport_rect, Gfx::FontMetrics const& font_metrics, float font_size, float root_font_size) const +{ + if (auto length = absolutized_length(m_length, viewport_rect, font_metrics, font_size, root_font_size); length.has_value()) + return LengthStyleValue::create(length.release_value()); + return *this; +} + +NonnullRefPtr BoxShadowStyleValue::absolutized(Gfx::IntRect const& viewport_rect, Gfx::FontMetrics const& font_metrics, float font_size, float root_font_size) const +{ + auto absolutized_offset_x = absolutized_length(m_offset_x, viewport_rect, font_metrics, font_size, root_font_size).value_or(m_offset_x); + auto absolutized_offset_y = absolutized_length(m_offset_y, viewport_rect, font_metrics, font_size, root_font_size).value_or(m_offset_y); + auto absolutized_blur_radius = absolutized_length(m_blur_radius, viewport_rect, font_metrics, font_size, root_font_size).value_or(m_blur_radius); + auto absolutized_spread_distance = absolutized_length(m_spread_distance, viewport_rect, font_metrics, font_size, root_font_size).value_or(m_spread_distance); + return BoxShadowStyleValue::create(m_color, absolutized_offset_x, absolutized_offset_y, absolutized_blur_radius, absolutized_spread_distance, m_placement); +} + +NonnullRefPtr BorderRadiusStyleValue::absolutized(Gfx::IntRect const& viewport_rect, Gfx::FontMetrics const& font_metrics, float font_size, float root_font_size) const +{ + if (m_horizontal_radius.is_percentage() && m_vertical_radius.is_percentage()) + return *this; + auto absolutized_horizontal_radius = m_horizontal_radius; + auto absolutized_vertical_radius = m_vertical_radius; + if (!m_horizontal_radius.is_percentage()) + absolutized_horizontal_radius = absolutized_length(m_horizontal_radius.length(), viewport_rect, font_metrics, font_size, root_font_size).value_or(m_horizontal_radius.length()); + if (!m_vertical_radius.is_percentage()) + absolutized_vertical_radius = absolutized_length(m_vertical_radius.length(), viewport_rect, font_metrics, font_size, root_font_size).value_or(m_vertical_radius.length()); + return BorderRadiusStyleValue::create(absolutized_horizontal_radius, absolutized_vertical_radius); +} + } diff --git a/Userland/Libraries/LibWeb/CSS/StyleValue.h b/Userland/Libraries/LibWeb/CSS/StyleValue.h index 0217278221..8991aeb53f 100644 --- a/Userland/Libraries/LibWeb/CSS/StyleValue.h +++ b/Userland/Libraries/LibWeb/CSS/StyleValue.h @@ -444,6 +444,8 @@ public: virtual bool has_number() const { return false; } virtual bool has_integer() const { return false; } + virtual NonnullRefPtr absolutized(Gfx::IntRect const& viewport_rect, Gfx::FontMetrics const& font_metrics, float font_size, float root_font_size) const; + virtual Color to_color(Layout::NodeWithStyle const&) const { return {}; } virtual CSS::ValueID to_identifier() const { return ValueID::Invalid; } virtual Length to_length() const { VERIFY_NOT_REACHED(); } @@ -461,8 +463,6 @@ public: return to_string() == other.to_string(); } - virtual void visit_lengths(Function) { } - protected: explicit StyleValue(Type); @@ -669,19 +669,7 @@ private: m_is_elliptical = (m_horizontal_radius != m_vertical_radius); } - virtual void visit_lengths(Function visitor) override - { - if (!m_horizontal_radius.is_percentage()) { - Length temp = m_horizontal_radius.length(); - visitor(temp); - m_horizontal_radius = move(temp); - } - if (!m_vertical_radius.is_percentage()) { - Length temp = m_vertical_radius.length(); - visitor(temp); - m_vertical_radius = move(temp); - } - } + virtual NonnullRefPtr absolutized(Gfx::IntRect const& viewport_rect, Gfx::FontMetrics const& font_metrics, float font_size, float root_font_size) const override; bool m_is_elliptical; LengthPercentage m_horizontal_radius; @@ -718,13 +706,7 @@ private: { } - virtual void visit_lengths(Function visitor) override - { - visitor(m_offset_x); - visitor(m_offset_y); - visitor(m_blur_radius); - visitor(m_spread_distance); - } + virtual NonnullRefPtr absolutized(Gfx::IntRect const& viewport_rect, Gfx::FontMetrics const& font_metrics, float font_size, float root_font_size) const override; Color m_color; Length m_offset_x; @@ -1235,6 +1217,7 @@ public: virtual String to_string() const override { return m_length.to_string(); } virtual Length to_length() const override { return m_length; } virtual ValueID to_identifier() const override { return has_auto() ? ValueID::Auto : ValueID::Invalid; } + virtual NonnullRefPtr absolutized(Gfx::IntRect const& viewport_rect, Gfx::FontMetrics const& font_metrics, float font_size, float root_font_size) const override; virtual bool equals(StyleValue const& other) const override { @@ -1250,11 +1233,6 @@ private: { } - virtual void visit_lengths(Function visitor) override - { - visitor(m_length); - } - Length m_length; };