dmaax/ladybird
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

LibWeb: Preserve scroll offset for nearest scrollIntoView

Browse source 
Compute scrollIntoView positions from the current scroll position and
only add the delta required to align an edge. The previous calculation
mixed viewport coordinates with scroller content coordinates, so the
nearest case could move a focused element that was already visible.

Add a CSSOM View text regression test that clicks a focused control in
a scroller already positioned at the bottom.
This commit is contained in:
Andreas Kling 2026-05-30 19:05:33 +02:00 committed by Andreas Kling
parent 5b3d17be26
commit 76da9aa116
3 changed files with 62 additions and 34 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

58
Libraries/LibWeb/DOM/Element.cpp
View file

@ -2811,13 +2811,14 @@ static CSSPixelPoint determine_the_scroll_into_view_position(Element& target, Bi
// run the following steps:
CSSPixelRect scrolling_box_rect;
CSSPixelPoint scroll_offset;
CSSPixelPoint current_scroll_position;
if (scrolling_box.is_document()) {
// NOTE: For a viewport scrolling box is initial containing block
scrolling_box_rect = scrolling_box.document().viewport_rect();
// NOTE: Element::getBoundingClientRect() returns coordinates relative to the viewport.
scrolling_box_rect = { {}, scrolling_box.document().viewport_rect().size() };
current_scroll_position = scrolling_box.document().navigable()->viewport_scroll_offset();
} else if (auto paintable_box = scrolling_box.paintable_box()) {
scroll_offset = paintable_box->scroll_offset();
scrolling_box_rect = paintable_box->absolute_rect().translated(scroll_offset);
current_scroll_position = paintable_box->scroll_offset();
scrolling_box_rect = paintable_box->absolute_rect();
} else {
return {};
}
@ -2829,13 +2830,6 @@ static CSSPixelPoint determine_the_scroll_into_view_position(Element& target, Bi
// if target is a Range.
auto target_bounding_border_box = target.get_bounding_client_rect();
// AD-HOC: If scrolling_box is a box, translate target bounding box so that the following math works for both the
// viewport and a regular scrollable box.
if (!scrolling_box.is_document()) {
target_bounding_border_box.translate_by(scroll_offset);
target_bounding_border_box.translate_by(-scrolling_box.paintable_box()->absolute_rect().top_left());
}
// AD-HOC: The spec doesn't specify when to do this, but we need to apply scroll-margin and scroll-margin to target
// bounding border box (https://drafts.csswg.org/cssom-view-1/#example-51af1565).
auto scroll_margin = target.computed_properties()->length_box(CSS::PropertyID::ScrollMarginLeft, CSS::PropertyID::ScrollMarginTop, CSS::PropertyID::ScrollMarginRight, CSS::PropertyID::ScrollMarginBottom, CSS::Length::make_px(0));
@ -2916,72 +2910,72 @@ static CSSPixelPoint determine_the_scroll_into_view_position(Element& target, Bi
// 10. Let position be the scroll position scrolling box would have by following these steps:
auto position = [&]() -> CSSPixelPoint {
CSSPixels x = 0;
CSSPixels y = 0;
auto x = current_scroll_position.x();
auto y = current_scroll_position.y();
// 1. If block is "start", then align element edge A with scrolling box edge A.
if (block == Bindings::ScrollLogicalPosition::Start) {
y = element_edge_a;
y += element_edge_a - scrolling_box_edge_a;
}
// 2. Otherwise, if block is "end", then align element edge B with scrolling box edge B.
else if (block == Bindings::ScrollLogicalPosition::End) {
y = element_edge_b - scrolling_box_height;
y += element_edge_b - scrolling_box_edge_b;
}
// 3. Otherwise, if block is "center", then align the center of target bounding border box with the center of
// scrolling box in scrolling box’s block flow direction.
else if (block == Bindings::ScrollLogicalPosition::Center) {
y = element_edge_a + (element_height / 2) - (scrolling_box_height / 2);
y += (element_edge_a + element_height / 2) - (scrolling_box_edge_a + scrolling_box_height / 2);
}
// 4. Otherwise, block is "nearest":
else {
// If element edge A and element edge B are both outside scrolling box edge A and scrolling box edge B
if (element_edge_a <= 0 && element_edge_b >= scrolling_box_height) {
if (element_edge_a <= scrolling_box_edge_a && element_edge_b >= scrolling_box_edge_b) {
// Do nothing.
}
// If element edge A is outside scrolling box edge A and element height is less than scrolling box height
// If element edge B is outside scrolling box edge B and element height is greater than scrolling box height
else if ((element_edge_a <= 0 && element_height < scrolling_box_height) || (element_edge_b >= scrolling_box_height && element_height > scrolling_box_height)) {
else if ((element_edge_a <= scrolling_box_edge_a && element_height < scrolling_box_height) || (element_edge_b >= scrolling_box_edge_b && element_height > scrolling_box_height)) {
// Align element edge A with scrolling box edge A.
y = element_edge_a;
y += element_edge_a - scrolling_box_edge_a;
}
// If element edge A is outside scrolling box edge A and element height is greater than scrolling box height
// If element edge B is outside scrolling box edge B and element height is less than scrolling box height
else if ((element_edge_b >= scrolling_box_height && element_height < scrolling_box_height) || (element_edge_a <= 0 && element_height > scrolling_box_height)) {
else if ((element_edge_b >= scrolling_box_edge_b && element_height < scrolling_box_height) || (element_edge_a <= scrolling_box_edge_a && element_height > scrolling_box_height)) {
// Align element edge B with scrolling box edge B.
y = element_edge_b - scrolling_box_height;
y += element_edge_b - scrolling_box_edge_b;
}
}
// 5. If inline is "start", then align element edge C with scrolling box edge C.
if (inline_ == Bindings::ScrollLogicalPosition::Start) {
x = element_edge_c;
x += element_edge_c - scrolling_box_edge_c;
}
// 6. Otherwise, if inline is "end", then align element edge D with scrolling box edge D.
else if (inline_ == Bindings::ScrollLogicalPosition::End) {
x = element_edge_d - scrolling_box_width;
x += element_edge_d - scrolling_box_edge_d;
}
// 7. Otherwise, if inline is "center", then align the center of target bounding border box with the center of
// scrolling box in scrolling box’s inline base direction.
else if (inline_ == Bindings::ScrollLogicalPosition::Center) {
x = element_edge_c + (element_width / 2) - (scrolling_box_width / 2);
x += (element_edge_c + element_width / 2) - (scrolling_box_edge_c + scrolling_box_width / 2);
}
// 8. Otherwise, inline is "nearest":
else {
// If element edge C and element edge D are both outside scrolling box edge C and scrolling box edge D
if (element_edge_c <= 0 && element_edge_d >= scrolling_box_width) {
if (element_edge_c <= scrolling_box_edge_c && element_edge_d >= scrolling_box_edge_d) {
// Do nothing.
}
// If element edge C is outside scrolling box edge C and element width is less than scrolling box width
// If element edge D is outside scrolling box edge D and element width is greater than scrolling box width
else if ((element_edge_c <= 0 && element_width < scrolling_box_width) || (element_edge_d >= scrolling_box_width && element_width > scrolling_box_width)) {
else if ((element_edge_c <= scrolling_box_edge_c && element_width < scrolling_box_width) || (element_edge_d >= scrolling_box_edge_d && element_width > scrolling_box_width)) {
// Align element edge C with scrolling box edge C.
x = element_edge_c;
x += element_edge_c - scrolling_box_edge_c;
}
// If element edge C is outside scrolling box edge C and element width is greater than scrolling box width
// If element edge D is outside scrolling box edge D and element width is less than scrolling box width
else if ((element_edge_d >= scrolling_box_width && element_width < scrolling_box_width) || (element_edge_c <= 0 && element_width > scrolling_box_width)) {
else if ((element_edge_d >= scrolling_box_edge_d && element_width < scrolling_box_width) || (element_edge_c <= scrolling_box_edge_c && element_width > scrolling_box_width)) {
// Align element edge D with scrolling box edge D.
x = element_edge_d - scrolling_box_width;
x += element_edge_d - scrolling_box_edge_d;
}
}
@ -3043,10 +3037,6 @@ static GC::Ref<WebIDL::Promise> scroll_an_element_into_view(Element& target, Bin
// Add the Promise returned from this step in the set ancestorPromises.
(void)behavior;
// AD-HOC:
// NOTE: Since calculated position is relative to the viewport, we need to add the viewport's position to it
// before passing to perform_a_scroll_of_the_viewport() that expects a position relative to the page.
position.set_y(position.y() + document.viewport_rect().y());
document.navigable()->perform_a_scroll_of_the_viewport(position);
}
}

2
Tests/LibWeb/Text/expected/CSSOMView/click-focused-element-in-scroller-should-not-scroll.txt Normal file
View file

@ -0,0 +1,2 @@
scrollTop before click: 342
scrollTop after click: 342

36
Tests/LibWeb/Text/input/CSSOMView/click-focused-element-in-scroller-should-not-scroll.html Normal file
View file

@ -0,0 +1,36 @@
<!DOCTYPE html>
<script src="../include.js"></script>
<style>
#scroller {
height: 100px;
overflow: auto;
width: 200px;
}
.spacer {
height: 400px;
}
</style>
<div id="scroller">
<div class="spacer"></div>
<button id="target">target</button>
<div style="height: 20px"></div>
</div>
<script>
asyncTest(done => {
const scroller = document.getElementById("scroller");
const target = document.getElementById("target");
scroller.scrollTop = scroller.scrollHeight;
const before = scroller.scrollTop;
const rect = target.getBoundingClientRect();
internals.click(rect.left + 5, rect.top + rect.height / 2);
setTimeout(() => {
println(`scrollTop before click: ${before}`);
println(`scrollTop after click: ${scroller.scrollTop}`);
done();
}, 0);
});
</script>