From d43ef2776119949efc97f85739904710ce9ec305 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Thu, 22 Sep 2022 14:17:11 +0200 Subject: [PATCH] LibWeb: Only include containing blocks in coordinate space translation Layout box offset coordinates are always relative to their containing block. Therefore, the functions that convert between coordinate spaces should only visit containing blocks and apply their offsets, not *every* box in the parent chain. This fixes an issue where some floating boxes were unexpectedly far away from their containing block. --- Base/res/html/misc/float-stress-3.html | 52 +++++++++++++++++++ .../Libraries/LibWeb/Layout/LayoutState.cpp | 45 ++++++++-------- 2 files changed, 76 insertions(+), 21 deletions(-) create mode 100644 Base/res/html/misc/float-stress-3.html diff --git a/Base/res/html/misc/float-stress-3.html b/Base/res/html/misc/float-stress-3.html new file mode 100644 index 0000000000..4dbff57ca7 --- /dev/null +++ b/Base/res/html/misc/float-stress-3.html @@ -0,0 +1,52 @@ + + + + float horror show + +
+ foo bar baz foo bar baz +
+
+
diff --git a/Userland/Libraries/LibWeb/Layout/LayoutState.cpp b/Userland/Libraries/LibWeb/Layout/LayoutState.cpp index ccd54fbe78..085294650a 100644 --- a/Userland/Libraries/LibWeb/Layout/LayoutState.cpp +++ b/Userland/Libraries/LibWeb/Layout/LayoutState.cpp @@ -121,15 +121,16 @@ Gfx::FloatRect border_box_rect(Box const& box, LayoutState const& state) Gfx::FloatRect border_box_rect_in_ancestor_coordinate_space(Box const& box, Box const& ancestor_box, LayoutState const& state) { auto rect = border_box_rect(box, state); - for (auto const* current = box.parent(); current; current = current->parent()) { + if (&box == &ancestor_box) + return rect; + for (auto const* current = box.containing_block(); current; current = current->containing_block()) { if (current == &ancestor_box) - break; - if (is(*current)) { - auto const& current_state = state.get(static_cast(*current)); - rect.translate_by(current_state.offset); - } + return rect; + auto const& current_state = state.get(static_cast(*current)); + rect.translate_by(current_state.offset); } - return rect; + // If we get here, ancestor_box was not a containing block ancestor of `box`! + VERIFY_NOT_REACHED(); } Gfx::FloatRect content_box_rect(Box const& box, LayoutState const& state) @@ -141,29 +142,31 @@ Gfx::FloatRect content_box_rect(Box const& box, LayoutState const& state) Gfx::FloatRect content_box_rect_in_ancestor_coordinate_space(Box const& box, Box const& ancestor_box, LayoutState const& state) { auto rect = content_box_rect(box, state); - for (auto const* current = box.parent(); current; current = current->parent()) { + if (&box == &ancestor_box) + return rect; + for (auto const* current = box.containing_block(); current; current = current->containing_block()) { if (current == &ancestor_box) - break; - if (is(*current)) { - auto const& current_state = state.get(static_cast(*current)); - rect.translate_by(current_state.offset); - } + return rect; + auto const& current_state = state.get(static_cast(*current)); + rect.translate_by(current_state.offset); } - return rect; + // If we get here, ancestor_box was not a containing block ancestor of `box`! + VERIFY_NOT_REACHED(); } Gfx::FloatRect margin_box_rect_in_ancestor_coordinate_space(Box const& box, Box const& ancestor_box, LayoutState const& state) { auto rect = margin_box_rect(box, state); - for (auto const* current = box.parent(); current; current = current->parent()) { + if (&box == &ancestor_box) + return rect; + for (auto const* current = box.containing_block(); current; current = current->containing_block()) { if (current == &ancestor_box) - break; - if (is(*current)) { - auto const& current_state = state.get(static_cast(*current)); - rect.translate_by(current_state.offset); - } + return rect; + auto const& current_state = state.get(static_cast(*current)); + rect.translate_by(current_state.offset); } - return rect; + // If we get here, ancestor_box was not a containing block ancestor of `box`! + VERIFY_NOT_REACHED(); } Gfx::FloatRect absolute_content_rect(Box const& box, LayoutState const& state)