Bladeren bron

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.
Andreas Kling 3 jaren geleden
bovenliggende
commit
c59ab7cc8b

+ 3 - 9
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);
     }
 }
 

+ 45 - 0
Userland/Libraries/LibWeb/CSS/StyleValue.cpp

@@ -1405,4 +1405,49 @@ NonnullRefPtr<LengthStyleValue> LengthStyleValue::create(Length const& length)
     return adopt_ref(*new LengthStyleValue(length));
 }
 
+static Optional<CSS::Length> 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> StyleValue::absolutized(Gfx::IntRect const&, Gfx::FontMetrics const&, float, float) const
+{
+    return *this;
+}
+
+NonnullRefPtr<StyleValue> 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<StyleValue> 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<StyleValue> 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);
+}
+
 }

+ 5 - 27
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<StyleValue> 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<void(CSS::Length&)>) { }
-
 protected:
     explicit StyleValue(Type);
 
@@ -669,19 +669,7 @@ private:
         m_is_elliptical = (m_horizontal_radius != m_vertical_radius);
     }
 
-    virtual void visit_lengths(Function<void(CSS::Length&)> 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<StyleValue> 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<void(CSS::Length&)> visitor) override
-    {
-        visitor(m_offset_x);
-        visitor(m_offset_y);
-        visitor(m_blur_radius);
-        visitor(m_spread_distance);
-    }
+    virtual NonnullRefPtr<StyleValue> 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<StyleValue> 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<void(CSS::Length&)> visitor) override
-    {
-        visitor(m_length);
-    }
-
     Length m_length;
 };