From 20f68106a7ec938a157989c02f51ac69f0fb07a2 Mon Sep 17 00:00:00 2001 From: Aliaksandr Kalenik Date: Sun, 1 Sep 2024 16:57:43 +0200 Subject: [PATCH] LibWeb: Fix getBoundingClientRect() for elements with "position: sticky" Use offset from ScrollFrame which is an actual value a box is shifted by while painting. Also change `update_paint_and_hit_testing_properties_if_needed()` to refresh scroll frames state, because `getBoundingClientRect()` now depends on them. Fixes wrong file tree sidebar location and excessive layout invalidations caused by some miscalculation on JS-side when wrong bounding client rect is provided on Github PR pages like https://github.com/LadybirdBrowser/ladybird/pull/1232/files --- ...ent-get-bounding-client-rect-of-sticky.txt | 1 + ...nt-get-bounding-client-rect-of-sticky.html | 47 +++++++++++++++++++ Userland/Libraries/LibWeb/DOM/Document.cpp | 7 ++- Userland/Libraries/LibWeb/DOM/Element.cpp | 15 ++++-- 4 files changed, 64 insertions(+), 6 deletions(-) create mode 100644 Tests/LibWeb/Text/expected/element-get-bounding-client-rect-of-sticky.txt create mode 100644 Tests/LibWeb/Text/input/element-get-bounding-client-rect-of-sticky.html diff --git a/Tests/LibWeb/Text/expected/element-get-bounding-client-rect-of-sticky.txt b/Tests/LibWeb/Text/expected/element-get-bounding-client-rect-of-sticky.txt new file mode 100644 index 00000000000..e8da9ba4d9d --- /dev/null +++ b/Tests/LibWeb/Text/expected/element-get-bounding-client-rect-of-sticky.txt @@ -0,0 +1 @@ + Sticky Element Bounding Client Rect: top=50, left=10, width=500, height=57 diff --git a/Tests/LibWeb/Text/input/element-get-bounding-client-rect-of-sticky.html b/Tests/LibWeb/Text/input/element-get-bounding-client-rect-of-sticky.html new file mode 100644 index 00000000000..0d181e067a2 --- /dev/null +++ b/Tests/LibWeb/Text/input/element-get-bounding-client-rect-of-sticky.html @@ -0,0 +1,47 @@ + + +
+
+
Sticky Element
+
+
+
+ + diff --git a/Userland/Libraries/LibWeb/DOM/Document.cpp b/Userland/Libraries/LibWeb/DOM/Document.cpp index 64fd3af8935..5c3f2e51ad8 100644 --- a/Userland/Libraries/LibWeb/DOM/Document.cpp +++ b/Userland/Libraries/LibWeb/DOM/Document.cpp @@ -1291,11 +1291,16 @@ void Document::update_animated_style_if_needed() void Document::update_paint_and_hit_testing_properties_if_needed() { + if (auto* paintable = this->paintable()) { + paintable->refresh_scroll_state(); + } + if (!m_needs_to_resolve_paint_only_properties) return; m_needs_to_resolve_paint_only_properties = false; - if (auto* paintable = this->paintable()) + if (auto* paintable = this->paintable()) { paintable->resolve_paint_only_properties(); + } } void Document::set_normal_link_color(Color color) diff --git a/Userland/Libraries/LibWeb/DOM/Element.cpp b/Userland/Libraries/LibWeb/DOM/Element.cpp index 27e98062a56..0d0e43bd55a 100644 --- a/Userland/Libraries/LibWeb/DOM/Element.cpp +++ b/Userland/Libraries/LibWeb/DOM/Element.cpp @@ -974,7 +974,6 @@ JS::NonnullGCPtr Element::get_client_rects() const // or inline-table include both the table box and the caption box, if any, but not the anonymous container box. // FIXME: - Replace each anonymous block box with its child box(es) and repeat this until no anonymous block boxes // are left in the final list. - auto viewport_offset = navigable->viewport_scroll_offset(); // NOTE: Make sure CSS transforms are resolved before it is used to calculate the rect position. const_cast(document()).update_paint_and_hit_testing_properties_if_needed(); @@ -988,21 +987,27 @@ JS::NonnullGCPtr Element::get_client_rects() const transform = Gfx::extract_2d_affine_transform(paintable_box->transform()); for (auto const* containing_block = paintable->containing_block(); !containing_block->is_viewport(); containing_block = containing_block->containing_block()) { transform = Gfx::extract_2d_affine_transform(containing_block->transform()).multiply(transform); - scroll_offset.translate_by(containing_block->scroll_offset()); + } + + if (auto enclosing_scroll_offset = paintable_box->enclosing_scroll_frame(); enclosing_scroll_offset) { + scroll_offset.translate_by(-enclosing_scroll_offset->cumulative_offset()); } auto absolute_rect = paintable_box->absolute_border_box_rect(); auto transformed_rect = transform.map(absolute_rect.translated(-paintable_box->transform_origin()).to_type()) .to_type() .translated(paintable_box->transform_origin()) - .translated(-scroll_offset) - .translated(-viewport_offset); + .translated(-scroll_offset); rects.append(Geometry::DOMRect::create(realm(), transformed_rect.to_type())); } else if (paintable && is(*paintable)) { auto const& inline_paintable = static_cast(*paintable); + + if (auto enclosing_scroll_offset = inline_paintable.enclosing_scroll_frame(); enclosing_scroll_offset) { + scroll_offset.translate_by(-enclosing_scroll_offset->cumulative_offset()); + } + auto absolute_rect = inline_paintable.bounding_rect(); absolute_rect.translate_by(-scroll_offset); - absolute_rect.translate_by(-viewport_offset); rects.append(Geometry::DOMRect::create(realm(), transform.map(absolute_rect.to_type()))); } else if (paintable) { dbgln("FIXME: Failed to get client rects for element ({})", debug_description());