From 70919dbed795a7b7bc355f0e67862dbe8142152e Mon Sep 17 00:00:00 2001 From: martinfalisse Date: Wed, 16 Aug 2023 17:47:52 +0200 Subject: [PATCH] LibWeb: Fix parsing bug for SVG attributes This doesn't seem to actually have fixed any bugs, as having FillOpacity instead of StrokeOpacity in the call to parse_css_value doesn't seem to have actually been causing bugs. But, I still think it's worthwhile correcting. The reason that it wasn't causing bugs is that having FillOpacity instead of StrokeOpacity in the call to parse_css_value means that when parsing the value is compared to the acceptable values for that property (for example the value can only be a percentage, or a number, etc.). In this case both FillOpacity and StrokeOpacity seem to accept the same values. --- .../svg/svg-different-types-of-opacity.txt | 16 ++++++++++++++++ .../svg/svg-different-types-of-opacity.html | 11 +++++++++++ .../Libraries/LibWeb/SVG/SVGGraphicsElement.cpp | 6 +++--- 3 files changed, 30 insertions(+), 3 deletions(-) create mode 100644 Tests/LibWeb/Layout/expected/svg/svg-different-types-of-opacity.txt create mode 100644 Tests/LibWeb/Layout/input/svg/svg-different-types-of-opacity.html diff --git a/Tests/LibWeb/Layout/expected/svg/svg-different-types-of-opacity.txt b/Tests/LibWeb/Layout/expected/svg/svg-different-types-of-opacity.txt new file mode 100644 index 0000000000..83c274c7f9 --- /dev/null +++ b/Tests/LibWeb/Layout/expected/svg/svg-different-types-of-opacity.txt @@ -0,0 +1,16 @@ +Viewport <#document> at (0,0) content-size 800x600 children: not-inline + BlockContainer at (0,0) content-size 800x800 [BFC] children: not-inline + BlockContainer at (8,8) content-size 784x784 children: inline + line 0 width: 784, height: 784, bottom: 784, baseline: 784 + frag 0 from SVGSVGBox start: 0, length: 0, rect: [8,8 784x784] + SVGSVGBox at (8,8) content-size 784x784 [SVG] children: inline + TextNode <#text> + SVGGeometryBox at (47.203125,47.203125) content-size 548.796875x548.796875 children: not-inline + TextNode <#text> + TextNode <#text> + +PaintableWithLines (Viewport<#document>) [0,0 800x600] overflow: [0,0 800x800] + PaintableWithLines (BlockContainer) [0,0 800x800] + PaintableWithLines (BlockContainer) [8,8 784x784] + SVGSVGPaintable (SVGSVGBox) [8,8 784x784] + SVGGeometryPaintable (SVGGeometryBox) [47.203125,47.203125 548.796875x548.796875] diff --git a/Tests/LibWeb/Layout/input/svg/svg-different-types-of-opacity.html b/Tests/LibWeb/Layout/input/svg/svg-different-types-of-opacity.html new file mode 100644 index 0000000000..2b27d64ae4 --- /dev/null +++ b/Tests/LibWeb/Layout/input/svg/svg-different-types-of-opacity.html @@ -0,0 +1,11 @@ + + + diff --git a/Userland/Libraries/LibWeb/SVG/SVGGraphicsElement.cpp b/Userland/Libraries/LibWeb/SVG/SVGGraphicsElement.cpp index 5bd34d3aeb..779fe10ad2 100644 --- a/Userland/Libraries/LibWeb/SVG/SVGGraphicsElement.cpp +++ b/Userland/Libraries/LibWeb/SVG/SVGGraphicsElement.cpp @@ -141,11 +141,11 @@ void SVGGraphicsElement::apply_presentational_hints(CSS::StyleProperties& style) if (auto fill_opacity_value = parse_css_value(parsing_context, value, CSS::PropertyID::FillOpacity).release_value_but_fixme_should_propagate_errors()) style.set_property(CSS::PropertyID::FillOpacity, fill_opacity_value.release_nonnull()); } else if (name.equals_ignoring_ascii_case("stroke-opacity"sv)) { - if (auto stroke_opacity_value = parse_css_value(parsing_context, value, CSS::PropertyID::FillOpacity).release_value_but_fixme_should_propagate_errors()) + if (auto stroke_opacity_value = parse_css_value(parsing_context, value, CSS::PropertyID::StrokeOpacity).release_value_but_fixme_should_propagate_errors()) style.set_property(CSS::PropertyID::StrokeOpacity, stroke_opacity_value.release_nonnull()); } else if (name.equals_ignoring_ascii_case(SVG::AttributeNames::opacity)) { - if (auto stroke_opacity_value = parse_css_value(parsing_context, value, CSS::PropertyID::Opacity).release_value_but_fixme_should_propagate_errors()) - style.set_property(CSS::PropertyID::Opacity, stroke_opacity_value.release_nonnull()); + if (auto opacity_value = parse_css_value(parsing_context, value, CSS::PropertyID::Opacity).release_value_but_fixme_should_propagate_errors()) + style.set_property(CSS::PropertyID::Opacity, opacity_value.release_nonnull()); } else if (name.equals_ignoring_ascii_case("text-anchor"sv)) { if (auto text_anchor_value = parse_css_value(parsing_context, value, CSS::PropertyID::TextAnchor).release_value_but_fixme_should_propagate_errors()) style.set_property(CSS::PropertyID::TextAnchor, text_anchor_value.release_nonnull());