From 8eacfc8f10256f29fed0405a6bf1cb95e73b9bef Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Thu, 22 Aug 2024 13:32:25 +0200 Subject: [PATCH] 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 root elements. This meant that elements where the attribute and computed value were different values would end up with incorrect natural size. --- .../svg/natural-width-from-svg-attributes.txt | 14 +++ .../natural-width-from-svg-attributes.html | 14 +++ .../Libraries/LibWeb/Layout/SVGSVGBox.cpp | 86 +++++++++---------- .../Libraries/LibWeb/SVG/SVGSVGElement.cpp | 55 ++++++++---- Userland/Libraries/LibWeb/SVG/SVGSVGElement.h | 7 +- 5 files changed, 109 insertions(+), 67 deletions(-) create mode 100644 Tests/LibWeb/Layout/expected/svg/natural-width-from-svg-attributes.txt create mode 100644 Tests/LibWeb/Layout/input/svg/natural-width-from-svg-attributes.html diff --git a/Tests/LibWeb/Layout/expected/svg/natural-width-from-svg-attributes.txt b/Tests/LibWeb/Layout/expected/svg/natural-width-from-svg-attributes.txt new file mode 100644 index 00000000000..db8fa622614 --- /dev/null +++ b/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 at (0,0) content-size 800x144 [BFC] children: not-inline + BlockContainer at (8,8) content-size 128x128 children: not-inline + SVGSVGBox at (8,8) content-size 64x64 [SVG] children: not-inline + SVGGeometryBox 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) [0,0 800x144] + PaintableWithLines (BlockContainer) [8,8 128x128] + SVGSVGPaintable (SVGSVGBox) [8,8 64x64] + SVGPathPaintable (SVGGeometryBox) [8,8 64x64] + PaintableWithLines (BlockContainer(anonymous)) [8,72 128x0] diff --git a/Tests/LibWeb/Layout/input/svg/natural-width-from-svg-attributes.html b/Tests/LibWeb/Layout/input/svg/natural-width-from-svg-attributes.html new file mode 100644 index 00000000000..32b1686db61 --- /dev/null +++ b/Tests/LibWeb/Layout/input/svg/natural-width-from-svg-attributes.html @@ -0,0 +1,14 @@ + diff --git a/Userland/Libraries/LibWeb/Layout/SVGSVGBox.cpp b/Userland/Libraries/LibWeb/Layout/SVGSVGBox.cpp index d25f7ff55b6..57092de820a 100644 --- a/Userland/Libraries/LibWeb/Layout/SVGSVGBox.cpp +++ b/Userland/Libraries/LibWeb/Layout/SVGSVGBox.cpp @@ -1,11 +1,12 @@ /* * Copyright (c) 2020, Matthew Olsson - * Copyright (c) 2022, Andreas Kling + * Copyright (c) 2022-2024, Andreas Kling * * SPDX-License-Identifier: BSD-2-Clause */ #include +#include #include #include #include @@ -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)); + + Optional 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(); } - 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 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(); } - set_natural_aspect_ratio(calculate_intrinsic_aspect_ratio()); -} - -Optional 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 { + // 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 {}; } + // 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(); + + // 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 {}; - } + }(); - // 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(); - - // 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 {}; + set_natural_width(natural_width); + set_natural_height(natural_height); + set_natural_aspect_ratio(natural_aspect_ratio); } } diff --git a/Userland/Libraries/LibWeb/SVG/SVGSVGElement.cpp b/Userland/Libraries/LibWeb/SVG/SVGSVGElement.cpp index 3515ee0f416..adec27c3f64 100644 --- a/Userland/Libraries/LibWeb/SVG/SVGSVGElement.cpp +++ b/Userland/Libraries/LibWeb/SVG/SVGSVGElement.cpp @@ -47,6 +47,38 @@ JS::GCPtr SVGSVGElement::create_layout_node(NonnullRefPtr(document(), *this, move(style)); } +RefPtr 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 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 const& old_value, Optional const& value) diff --git a/Userland/Libraries/LibWeb/SVG/SVGSVGElement.h b/Userland/Libraries/LibWeb/SVG/SVGSVGElement.h index c3b53e9b336..7b90256cde4 100644 --- a/Userland/Libraries/LibWeb/SVG/SVGSVGElement.h +++ b/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 width_style_value_from_attribute() const; + [[nodiscard]] RefPtr height_style_value_from_attribute() const; private: SVGSVGElement(DOM::Document&, DOM::QualifiedName);