Browse Source

LibWeb: Derive SVG root's natural size from width/height attributes

We were incorrectly looking at the CSS computed values for width and
height to determine the natural size of <svg> root elements.
This meant that elements where the attribute and computed value were
different values would end up with incorrect natural size.
Andreas Kling 10 months ago
parent
commit
8eacfc8f10

+ 14 - 0
Tests/LibWeb/Layout/expected/svg/natural-width-from-svg-attributes.txt

@@ -0,0 +1,14 @@
+Viewport <#document> at (0,0) content-size 800x600 children: not-inline
+  BlockContainer <html> at (0,0) content-size 800x144 [BFC] children: not-inline
+    BlockContainer <body> at (8,8) content-size 128x128 children: not-inline
+      SVGSVGBox <svg> at (8,8) content-size 64x64 [SVG] children: not-inline
+        SVGGeometryBox <rect> at (8,8) content-size 64x64 children: not-inline
+      BlockContainer <(anonymous)> at (8,72) content-size 128x0 children: inline
+        TextNode <#text>
+
+ViewportPaintable (Viewport<#document>) [0,0 800x600]
+  PaintableWithLines (BlockContainer<HTML>) [0,0 800x144]
+    PaintableWithLines (BlockContainer<BODY>) [8,8 128x128]
+      SVGSVGPaintable (SVGSVGBox<svg>) [8,8 64x64]
+        SVGPathPaintable (SVGGeometryBox<rect>) [8,8 64x64]
+      PaintableWithLines (BlockContainer(anonymous)) [8,72 128x0]

+ 14 - 0
Tests/LibWeb/Layout/input/svg/natural-width-from-svg-attributes.html

@@ -0,0 +1,14 @@
+<!doctype html><style>
+  * {
+    outline: 1px solid black;
+  }
+  body {
+    height: 128px;
+    width: 128px;
+  }
+  svg {
+    width: auto;
+    height: auto;
+    display: block;
+  }
+</style><body><svg width="64" height="64" viewBox="0 0 10 10" ><rect x="0" y="0" width="10" height="10" fill="green"></rect></svg>

+ 36 - 42
Userland/Libraries/LibWeb/Layout/SVGSVGBox.cpp

@@ -1,11 +1,12 @@
 /*
  * Copyright (c) 2020, Matthew Olsson <mattco@serenityos.org>
- * Copyright (c) 2022, Andreas Kling <kling@serenityos.org>
+ * Copyright (c) 2022-2024, Andreas Kling <andreas@ladybird.org>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
 
 #include <LibWeb/CSS/Parser/Parser.h>
+#include <LibWeb/CSS/StyleValues/LengthStyleValue.h>
 #include <LibWeb/Layout/ReplacedBox.h>
 #include <LibWeb/Layout/SVGGeometryBox.h>
 #include <LibWeb/Painting/SVGSVGPaintable.h>
@@ -31,60 +32,53 @@ void SVGSVGBox::prepare_for_replaced_layout()
     // The intrinsic dimensions must also be determined from the width and height sizing properties.
     // If either width or height are not specified, the used value is the initial value 'auto'.
     // 'auto' and percentage lengths must not be used to determine an intrinsic width or intrinsic height.
-    auto const& computed_width = computed_values().width();
-    if (computed_width.is_length() && !computed_width.contains_percentage()) {
-        set_natural_width(computed_width.to_px(*this, 0));
-    }
 
-    auto const& computed_height = computed_values().height();
-    if (computed_height.is_length() && !computed_height.contains_percentage()) {
-        set_natural_height(computed_height.to_px(*this, 0));
+    Optional<CSSPixels> natural_width;
+    if (auto width = dom_node().width_style_value_from_attribute(); width && width->is_length() && width->as_length().length().is_absolute()) {
+        natural_width = width->as_length().length().absolute_length_to_px();
     }
 
-    set_natural_aspect_ratio(calculate_intrinsic_aspect_ratio());
-}
+    Optional<CSSPixels> natural_height;
+    if (auto height = dom_node().height_style_value_from_attribute(); height && height->is_length() && height->as_length().length().is_absolute()) {
+        natural_height = height->as_length().length().absolute_length_to_px();
+    }
 
-Optional<CSSPixelFraction> SVGSVGBox::calculate_intrinsic_aspect_ratio() const
-{
-    // https://www.w3.org/TR/SVG2/coords.html#SizingSVGInCSS
     // The intrinsic aspect ratio must be calculated using the following algorithm. If the algorithm returns null, then there is no intrinsic aspect ratio.
-
-    auto const& computed_width = computed_values().width();
-    auto const& computed_height = computed_values().height();
-
-    // 1. If the width and height sizing properties on the ‘svg’ element are both absolute values:
-    if (computed_width.is_length() && !computed_width.contains_percentage() && computed_height.is_length() && !computed_height.contains_percentage()) {
-        auto width = computed_width.to_px(*this, 0);
-        auto height = computed_height.to_px(*this, 0);
-
-        if (width != 0 && height != 0) {
-            // 1. return width / height
-            return width / height;
+    auto natural_aspect_ratio = [&]() -> Optional<CSSPixelFraction> {
+        // 1. If the width and height sizing properties on the ‘svg’ element are both absolute values:
+        if (natural_width.has_value() && natural_height.has_value()) {
+            if (natural_width != 0 && natural_height != 0) {
+                // 1. return width / height
+                return *natural_width / *natural_height;
+            }
+            return {};
         }
 
-        return {};
-    }
+        // FIXME: 2. If an SVG View is active:
+        // FIXME:    1. let viewbox be the viewbox defined by the active SVG View
+        // FIXME:    2. return viewbox.width / viewbox.height
 
-    // FIXME: 2. If an SVG View is active:
-    // FIXME:    1. let viewbox be the viewbox defined by the active SVG View
-    // FIXME:    2. return viewbox.width / viewbox.height
+        // 3. If the ‘viewBox’ on the ‘svg’ element is correctly specified:
+        if (dom_node().view_box().has_value()) {
+            // 1. let viewbox be the viewbox defined by the ‘viewBox’ attribute on the ‘svg’ element
+            auto const& viewbox = dom_node().view_box().value();
 
-    // 3. If the ‘viewBox’ on the ‘svg’ element is correctly specified:
-    if (dom_node().view_box().has_value()) {
-        // 1. let viewbox be the viewbox defined by the ‘viewBox’ attribute on the ‘svg’ element
-        auto const& viewbox = dom_node().view_box().value();
+            // 2. return viewbox.width / viewbox.height
+            auto viewbox_width = CSSPixels::nearest_value_for(viewbox.width);
+            auto viewbox_height = CSSPixels::nearest_value_for(viewbox.height);
+            if (viewbox_width != 0 && viewbox_height != 0)
+                return viewbox_width / viewbox_height;
 
-        // 2. return viewbox.width / viewbox.height
-        auto viewbox_width = CSSPixels::nearest_value_for(viewbox.width);
-        auto viewbox_height = CSSPixels::nearest_value_for(viewbox.height);
-        if (viewbox_width != 0 && viewbox_height != 0)
-            return viewbox_width / viewbox_height;
+            return {};
+        }
 
+        // 4. return null
         return {};
-    }
+    }();
 
-    // 4. return null
-    return {};
+    set_natural_width(natural_width);
+    set_natural_height(natural_height);
+    set_natural_aspect_ratio(natural_aspect_ratio);
 }
 
 }

+ 36 - 19
Userland/Libraries/LibWeb/SVG/SVGSVGElement.cpp

@@ -47,6 +47,38 @@ JS::GCPtr<Layout::Node> SVGSVGElement::create_layout_node(NonnullRefPtr<CSS::Sty
     return heap().allocate_without_realm<Layout::SVGSVGBox>(document(), *this, move(style));
 }
 
+RefPtr<CSS::CSSStyleValue> SVGSVGElement::width_style_value_from_attribute() const
+{
+    auto parsing_context = CSS::Parser::ParsingContext { document(), CSS::Parser::ParsingContext::Mode::SVGPresentationAttribute };
+    auto width_attribute = attribute(SVG::AttributeNames::width);
+    if (auto width_value = parse_css_value(parsing_context, width_attribute.value_or(String {}), CSS::PropertyID::Width)) {
+        return width_value.release_nonnull();
+    }
+    if (width_attribute == "") {
+        // If the `width` attribute is an empty string, it defaults to 100%.
+        // This matches WebKit and Blink, but not Firefox. The spec is unclear.
+        // FIXME: Figure out what to do here.
+        return CSS::PercentageStyleValue::create(CSS::Percentage { 100 });
+    }
+    return nullptr;
+}
+
+RefPtr<CSS::CSSStyleValue> SVGSVGElement::height_style_value_from_attribute() const
+{
+    auto parsing_context = CSS::Parser::ParsingContext { document(), CSS::Parser::ParsingContext::Mode::SVGPresentationAttribute };
+    auto height_attribute = attribute(SVG::AttributeNames::height);
+    if (auto height_value = parse_css_value(parsing_context, height_attribute.value_or(String {}), CSS::PropertyID::Height)) {
+        return height_value.release_nonnull();
+    }
+    if (height_attribute == "") {
+        // If the `height` attribute is an empty string, it defaults to 100%.
+        // This matches WebKit and Blink, but not Firefox. The spec is unclear.
+        // FIXME: Figure out what to do here.
+        return CSS::PercentageStyleValue::create(CSS::Percentage { 100 });
+    }
+    return nullptr;
+}
+
 void SVGSVGElement::apply_presentational_hints(CSS::StyleProperties& style) const
 {
     Base::apply_presentational_hints(style);
@@ -62,26 +94,11 @@ void SVGSVGElement::apply_presentational_hints(CSS::StyleProperties& style) cons
         style.set_property(CSS::PropertyID::Y, y_value.release_nonnull());
     }
 
-    auto width_attribute = attribute(SVG::AttributeNames::width);
-    if (auto width_value = parse_css_value(parsing_context, width_attribute.value_or(String {}), CSS::PropertyID::Width)) {
-        style.set_property(CSS::PropertyID::Width, width_value.release_nonnull());
-    } else if (width_attribute == "") {
-        // If the `width` attribute is an empty string, it defaults to 100%.
-        // This matches WebKit and Blink, but not Firefox. The spec is unclear.
-        // FIXME: Figure out what to do here.
-        style.set_property(CSS::PropertyID::Width, CSS::PercentageStyleValue::create(CSS::Percentage { 100 }));
-    }
+    if (auto width = width_style_value_from_attribute())
+        style.set_property(CSS::PropertyID::Width, width.release_nonnull());
 
-    // Height defaults to 100%
-    auto height_attribute = attribute(SVG::AttributeNames::height);
-    if (auto height_value = parse_css_value(parsing_context, height_attribute.value_or(String {}), CSS::PropertyID::Height)) {
-        style.set_property(CSS::PropertyID::Height, height_value.release_nonnull());
-    } else if (height_attribute == "") {
-        // If the `height` attribute is an empty string, it defaults to 100%.
-        // This matches WebKit and Blink, but not Firefox. The spec is unclear.
-        // FIXME: Figure out what to do here.
-        style.set_property(CSS::PropertyID::Height, CSS::PercentageStyleValue::create(CSS::Percentage { 100 }));
-    }
+    if (auto height = height_style_value_from_attribute())
+        style.set_property(CSS::PropertyID::Height, height.release_nonnull());
 }
 
 void SVGSVGElement::attribute_changed(FlyString const& name, Optional<String> const& old_value, Optional<String> const& value)

+ 5 - 2
Userland/Libraries/LibWeb/SVG/SVGSVGElement.h

@@ -77,8 +77,11 @@ public:
     {
         (void)suspend_handle_id;
     }
-    void unsuspend_redraw_all() const {};
-    void force_redraw() const {};
+    void unsuspend_redraw_all() const { }
+    void force_redraw() const { }
+
+    [[nodiscard]] RefPtr<CSS::CSSStyleValue> width_style_value_from_attribute() const;
+    [[nodiscard]] RefPtr<CSS::CSSStyleValue> height_style_value_from_attribute() const;
 
 private:
     SVGSVGElement(DOM::Document&, DOM::QualifiedName);