Skip to content

Commit ae99cbe

Browse files
AtkinsSJawesomekling
authored andcommitted
LibWeb: Treat SVG fill/stroke/stroke-width attributes as CSS properties
Rather than having separate systems for the attributes and their CSS equivalents, we can treat the attributes as presentational hints and convert them to CSS properties. This means they can be inherited, as they should. :^) As noted, the `fill` and `stroke` attributes do not fully match the `fill` and `stroke` properties. The CSS spec is still an early draft and not entirely helpful, so we can just pretend they are the same for now.
1 parent f4ceea2 commit ae99cbe

File tree

2 files changed

+20
-26
lines changed

2 files changed

+20
-26
lines changed

Userland/Libraries/LibWeb/SVG/SVGGraphicsElement.cpp

Lines changed: 18 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
/*
22
* Copyright (c) 2020, Matthew Olsson <mattco@serenityos.org>
3-
* Copyright (c) 2021, Sam Atkins <atkinssj@serenityos.org>
3+
* Copyright (c) 2021-2022, Sam Atkins <atkinssj@serenityos.org>
44
*
55
* SPDX-License-Identifier: BSD-2-Clause
66
*/
77

8+
#include <LibWeb/CSS/Parser/Parser.h>
89
#include <LibWeb/SVG/SVGGraphicsElement.h>
910

1011
namespace Web::SVG {
@@ -14,25 +15,27 @@ SVGGraphicsElement::SVGGraphicsElement(DOM::Document& document, QualifiedName qu
1415
{
1516
}
1617

17-
void SVGGraphicsElement::parse_attribute(FlyString const& name, String const& value)
18+
void SVGGraphicsElement::apply_presentational_hints(CSS::StyleProperties& style) const
1819
{
19-
SVGElement::parse_attribute(name, value);
20-
21-
if (name == "fill") {
22-
m_fill_color = Gfx::Color::from_string(value).value_or(Color::Transparent);
23-
} else if (name == "stroke") {
24-
m_stroke_color = Gfx::Color::from_string(value).value_or(Color::Transparent);
25-
} else if (name == "stroke-width") {
26-
auto result = value.to_int();
27-
if (result.has_value())
28-
m_stroke_width = result.value();
29-
}
20+
CSS::ParsingContext parsing_context { document() };
21+
for_each_attribute([&](auto& name, auto& value) {
22+
if (name.equals_ignoring_case("fill")) {
23+
// FIXME: The `fill` attribute and CSS `fill` property are not the same! But our support is limited enough that they are equivalent for now.
24+
if (auto fill_value = parse_css_value(parsing_context, value, CSS::PropertyID::Fill))
25+
style.set_property(CSS::PropertyID::Fill, fill_value.release_nonnull());
26+
} else if (name.equals_ignoring_case("stroke")) {
27+
// FIXME: The `stroke` attribute and CSS `stroke` property are not the same! But our support is limited enough that they are equivalent for now.
28+
if (auto stroke_value = parse_css_value(parsing_context, value, CSS::PropertyID::Stroke))
29+
style.set_property(CSS::PropertyID::Stroke, stroke_value.release_nonnull());
30+
} else if (name.equals_ignoring_case("stroke-width")) {
31+
if (auto stroke_width_value = parse_css_value(parsing_context, value, CSS::PropertyID::StrokeWidth))
32+
style.set_property(CSS::PropertyID::StrokeWidth, stroke_width_value.release_nonnull());
33+
}
34+
});
3035
}
3136

3237
Optional<Gfx::Color> SVGGraphicsElement::fill_color() const
3338
{
34-
if (m_fill_color.has_value())
35-
return m_fill_color;
3639
if (!layout_node())
3740
return {};
3841
// FIXME: In the working-draft spec, `fill` is intended to be a shorthand, with `fill-color`
@@ -42,8 +45,6 @@ Optional<Gfx::Color> SVGGraphicsElement::fill_color() const
4245

4346
Optional<Gfx::Color> SVGGraphicsElement::stroke_color() const
4447
{
45-
if (m_stroke_color.has_value())
46-
return m_stroke_color;
4748
if (!layout_node())
4849
return {};
4950
// FIXME: In the working-draft spec, `stroke` is intended to be a shorthand, with `stroke-color`
@@ -53,8 +54,6 @@ Optional<Gfx::Color> SVGGraphicsElement::stroke_color() const
5354

5455
Optional<float> SVGGraphicsElement::stroke_width() const
5556
{
56-
if (m_stroke_width.has_value())
57-
return m_stroke_width;
5857
if (!layout_node())
5958
return {};
6059
// FIXME: Converting to pixels isn't really correct - values should be in "user units"

Userland/Libraries/LibWeb/SVG/SVGGraphicsElement.h

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/*
22
* Copyright (c) 2020, Matthew Olsson <mattco@serenityos.org>
3-
* Copyright (c) 2021, Sam Atkins <atkinssj@serenityos.org>
3+
* Copyright (c) 2021-2022, Sam Atkins <atkinssj@serenityos.org>
44
*
55
* SPDX-License-Identifier: BSD-2-Clause
66
*/
@@ -20,16 +20,11 @@ class SVGGraphicsElement : public SVGElement {
2020

2121
SVGGraphicsElement(DOM::Document&, QualifiedName);
2222

23-
virtual void parse_attribute(FlyString const& name, String const& value) override;
23+
virtual void apply_presentational_hints(CSS::StyleProperties&) const override;
2424

2525
Optional<Gfx::Color> fill_color() const;
2626
Optional<Gfx::Color> stroke_color() const;
2727
Optional<float> stroke_width() const;
28-
29-
protected:
30-
Optional<Gfx::Color> m_fill_color;
31-
Optional<Gfx::Color> m_stroke_color;
32-
Optional<float> m_stroke_width;
3328
};
3429

3530
}

0 commit comments

Comments
 (0)