From 1f69e9cddfd095549d56c387c819a5296e658608 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Mon, 18 Sep 2023 13:08:20 +0200 Subject: [PATCH] LibWeb: Remove Layout::Node::m_visible and compute it on the fly This fixes an issue where the value would be out of sync with reality in anonymous wrapper block boxes, since we forgot to compute m_visible after assigning the computed values to them. Fixes #21106 --- .../LibWeb/Ref/anonymous-wrapper-css-visibility.html | 12 ++++++++++++ .../anonymous-wrapper-css-visibility-ref.html | 12 ++++++++++++ Userland/Libraries/LibWeb/Layout/Node.cpp | 2 -- Userland/Libraries/LibWeb/Layout/Node.h | 4 ---- .../Libraries/LibWeb/Painting/CanvasPaintable.cpp | 2 +- .../Painting/NestedBrowsingContextPaintable.cpp | 2 +- Userland/Libraries/LibWeb/Painting/PaintableBox.cpp | 5 +++++ Userland/Libraries/LibWeb/Painting/PaintableBox.h | 2 +- 8 files changed, 32 insertions(+), 9 deletions(-) create mode 100644 Tests/LibWeb/Ref/anonymous-wrapper-css-visibility.html create mode 100644 Tests/LibWeb/Ref/reference/anonymous-wrapper-css-visibility-ref.html diff --git a/Tests/LibWeb/Ref/anonymous-wrapper-css-visibility.html b/Tests/LibWeb/Ref/anonymous-wrapper-css-visibility.html new file mode 100644 index 0000000000..08cd11b3d3 --- /dev/null +++ b/Tests/LibWeb/Ref/anonymous-wrapper-css-visibility.html @@ -0,0 +1,12 @@ + + +Hidden diff --git a/Tests/LibWeb/Ref/reference/anonymous-wrapper-css-visibility-ref.html b/Tests/LibWeb/Ref/reference/anonymous-wrapper-css-visibility-ref.html new file mode 100644 index 0000000000..eca160cbe3 --- /dev/null +++ b/Tests/LibWeb/Ref/reference/anonymous-wrapper-css-visibility-ref.html @@ -0,0 +1,12 @@ + + +Hidden diff --git a/Userland/Libraries/LibWeb/Layout/Node.cpp b/Userland/Libraries/LibWeb/Layout/Node.cpp index 19e16ecf4b..9d76e958ca 100644 --- a/Userland/Libraries/LibWeb/Layout/Node.cpp +++ b/Userland/Libraries/LibWeb/Layout/Node.cpp @@ -640,8 +640,6 @@ void NodeWithStyle::apply_style(const CSS::StyleProperties& computed_style) if (auto maybe_visibility = computed_style.visibility(); maybe_visibility.has_value()) computed_values.set_visibility(maybe_visibility.release_value()); - m_visible = computed_values.opacity() != 0 && computed_values.visibility() == CSS::Visibility::Visible; - computed_values.set_width(computed_style.size_value(CSS::PropertyID::Width)); computed_values.set_min_width(computed_style.size_value(CSS::PropertyID::MinWidth)); computed_values.set_max_width(computed_style.size_value(CSS::PropertyID::MaxWidth)); diff --git a/Userland/Libraries/LibWeb/Layout/Node.h b/Userland/Libraries/LibWeb/Layout/Node.h index 2183db88b2..55b7e33a1e 100644 --- a/Userland/Libraries/LibWeb/Layout/Node.h +++ b/Userland/Libraries/LibWeb/Layout/Node.h @@ -156,9 +156,6 @@ public: void removed_from(Node&) { } void children_changed() { } - bool is_visible() const { return m_visible; } - void set_visible(bool visible) { m_visible = visible; } - virtual void set_needs_display(); bool children_are_inline() const { return m_children_are_inline; } @@ -194,7 +191,6 @@ private: bool m_anonymous { false }; bool m_has_style { false }; - bool m_visible { true }; bool m_children_are_inline { false }; SelectionState m_selection_state { SelectionState::None }; diff --git a/Userland/Libraries/LibWeb/Painting/CanvasPaintable.cpp b/Userland/Libraries/LibWeb/Painting/CanvasPaintable.cpp index 83e2735610..de08c1c5b8 100644 --- a/Userland/Libraries/LibWeb/Painting/CanvasPaintable.cpp +++ b/Userland/Libraries/LibWeb/Painting/CanvasPaintable.cpp @@ -25,7 +25,7 @@ Layout::CanvasBox const& CanvasPaintable::layout_box() const void CanvasPaintable::paint(PaintContext& context, PaintPhase phase) const { - if (!layout_box().is_visible()) + if (!is_visible()) return; PaintableBox::paint(context, phase); diff --git a/Userland/Libraries/LibWeb/Painting/NestedBrowsingContextPaintable.cpp b/Userland/Libraries/LibWeb/Painting/NestedBrowsingContextPaintable.cpp index 81953d80d3..1324de9e54 100644 --- a/Userland/Libraries/LibWeb/Painting/NestedBrowsingContextPaintable.cpp +++ b/Userland/Libraries/LibWeb/Painting/NestedBrowsingContextPaintable.cpp @@ -31,7 +31,7 @@ Layout::FrameBox const& NestedBrowsingContextPaintable::layout_box() const void NestedBrowsingContextPaintable::paint(PaintContext& context, PaintPhase phase) const { - if (!layout_box().is_visible()) + if (!is_visible()) return; PaintableBox::paint(context, phase); diff --git a/Userland/Libraries/LibWeb/Painting/PaintableBox.cpp b/Userland/Libraries/LibWeb/Painting/PaintableBox.cpp index b1ca76a794..acb2b40909 100644 --- a/Userland/Libraries/LibWeb/Painting/PaintableBox.cpp +++ b/Userland/Libraries/LibWeb/Painting/PaintableBox.cpp @@ -40,6 +40,11 @@ PaintableBox::~PaintableBox() { } +bool PaintableBox::is_visible() const +{ + return computed_values().visibility() == CSS::Visibility::Visible && computed_values().opacity() != 0; +} + void PaintableBox::invalidate_stacking_context() { m_stacking_context = nullptr; diff --git a/Userland/Libraries/LibWeb/Painting/PaintableBox.h b/Userland/Libraries/LibWeb/Painting/PaintableBox.h index b20d38d7c7..330887da22 100644 --- a/Userland/Libraries/LibWeb/Painting/PaintableBox.h +++ b/Userland/Libraries/LibWeb/Painting/PaintableBox.h @@ -25,7 +25,7 @@ public: virtual void paint(PaintContext&, PaintPhase) const override; - bool is_visible() const { return layout_box().is_visible(); } + [[nodiscard]] bool is_visible() const; Layout::Box& layout_box() { return static_cast(Paintable::layout_node()); } Layout::Box const& layout_box() const { return static_cast(Paintable::layout_node()); }