Skip to content

Commit 629b7db

Browse files
mrobinsonmrobinson
authored
script: Reduce code duplication in the implementation of scrollIntoView (servo#39407)
- Expose a couple helpers on `ScrollingBox` that will also be used for keyboard scrolling. - Calculate `scrollIntoView` positions using points rather than doing things by axis components. This greatly reduces the amount of code in the implementation. Testing: This is just a refactor so shouldn't change any tests. Signed-off-by: Martin Robinson <[email protected]>
1 parent bdf6305 commit 629b7db

File tree

1 file changed

+79
-134
lines changed

1 file changed

+79
-134
lines changed

‎components/script/dom/element.rs‎

Lines changed: 79 additions & 134 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ use std::rc::Rc;
1212
use std::str::FromStr;
1313
use std::{fmt, mem};
1414

15+
use app_units::Au;
1516
use cssparser::match_ignore_ascii_case;
1617
use devtools_traits::AttrInfo;
1718
use dom_struct::dom_struct;
@@ -61,7 +62,7 @@ use style::values::{AtomIdent, AtomString, CSSFloat, computed, specified};
6162
use style::{ArcSlice, CaseSensitivityExt, dom_apis, thread_state};
6263
use stylo_atoms::Atom;
6364
use stylo_dom::ElementState;
64-
use webrender_api::units::LayoutVector2D;
65+
use webrender_api::units::{LayoutSize, LayoutVector2D};
6566
use xml5ever::serialize::TraversalScope::{
6667
ChildrenOnly as XmlChildrenOnly, IncludeNode as XmlIncludeNode,
6768
};
@@ -291,6 +292,15 @@ impl ScrollingBox {
291292
}
292293
}
293294

295+
fn size(&self) -> LayoutSize {
296+
match self {
297+
ScrollingBox::Element(element) => element.client_rect().size.to_f32().cast_unit(),
298+
ScrollingBox::Viewport(document) => {
299+
document.window().viewport_details().size.cast_unit()
300+
},
301+
}
302+
}
303+
294304
fn parent(&self) -> Option<ScrollingBox> {
295305
match self {
296306
ScrollingBox::Element(element) => element.scrolling_box(),
@@ -304,6 +314,19 @@ impl ScrollingBox {
304314
ScrollingBox::Viewport(document) => document.upcast(),
305315
}
306316
}
317+
318+
pub(crate) fn scroll_to(&self, position: LayoutVector2D, behavior: ScrollBehavior) {
319+
match self {
320+
ScrollingBox::Element(element) => {
321+
element
322+
.owner_window()
323+
.scroll_an_element(element, position.x, position.y, behavior);
324+
},
325+
ScrollingBox::Viewport(document) => {
326+
document.window().scroll(position.x, position.y, behavior);
327+
},
328+
}
329+
}
307330
}
308331

309332
//
@@ -916,26 +939,17 @@ impl Element {
916939
//
917940
// TODO: Handle smooth scrolling.
918941
if position != scrolling_box.scroll_position() {
919-
match &scrolling_box {
920-
// ↪ If `scrolling box` is associated with an element
921-
ScrollingBox::Element(element) => {
922-
// Perform a scroll of the element’s scrolling box to `position`,
923-
// with the `element` as the associated element and `behavior` as the
924-
// scroll behavior.
925-
element
926-
.owner_window()
927-
.scroll_an_element(element, position.x, position.y, behavior);
928-
},
929-
// ↪ If `scrolling box` is associated with a viewport
930-
ScrollingBox::Viewport(document) => {
931-
// Step 1: Let `document` be the viewport’s associated Document.
932-
// Step 2: Let `root element` be document’s root element, if there is one, or
933-
// null otherwise.
934-
// Step 3: Perform a scroll of the viewport to `position`, with `root element`
935-
// as the associated element and `behavior` as the scroll behavior.
936-
document.window().scroll(position.x, position.y, behavior);
937-
},
938-
}
942+
// ↪ If `scrolling box` is associated with an element
943+
// Perform a scroll of the element’s scrolling box to `position`,
944+
// with the `element` as the associated element and `behavior` as the
945+
// scroll behavior.
946+
// ↪ If `scrolling box` is associated with a viewport
947+
// Step 1: Let `document` be the viewport’s associated Document.
948+
// Step 2: Let `root element` be document’s root element, if there is one, or
949+
// null otherwise.
950+
// Step 3: Perform a scroll of the viewport to `position`, with `root element`
951+
// as the associated element and `behavior` as the scroll behavior.
952+
scrolling_box.scroll_to(position, behavior);
939953
}
940954

941955
// Step 1.4: If `container` is not null and either `scrolling box` is a shadow-including
@@ -969,96 +983,40 @@ impl Element {
969983
.get();
970984

971985
// Target element bounds
972-
let element_left = target_bounding_box
973-
.origin
974-
.x
975-
.to_nearest_pixel(device_pixel_ratio);
976-
let element_top = target_bounding_box
977-
.origin
978-
.y
979-
.to_nearest_pixel(device_pixel_ratio);
980-
let element_width = target_bounding_box
981-
.size
982-
.width
983-
.to_nearest_pixel(device_pixel_ratio);
984-
let element_height = target_bounding_box
985-
.size
986-
.height
987-
.to_nearest_pixel(device_pixel_ratio);
988-
let element_right = element_left + element_width;
989-
let element_bottom = element_top + element_height;
990-
991-
let (target_x, target_y) = match scrolling_box {
992-
ScrollingBox::Viewport(document) => {
993-
let window = document.window();
994-
let viewport_width = window.InnerWidth() as f32;
995-
let viewport_height = window.InnerHeight() as f32;
996-
let current_scroll_x = window.ScrollX() as f32;
997-
let current_scroll_y = window.ScrollY() as f32;
998-
986+
let to_pixel = |value: Au| value.to_nearest_pixel(device_pixel_ratio);
987+
let element_top_left = target_bounding_box.origin.map(to_pixel).to_untyped();
988+
let element_bottom_right = target_bounding_box.max().map(to_pixel);
989+
let element_size = target_bounding_box.size.to_vector().map(to_pixel);
990+
991+
let scrolling_box_size = scrolling_box.size();
992+
let current_scroll_position = scrolling_box.scroll_position().to_untyped();
993+
let (adjusted_element_top_left, adjusted_element_bottom_right) = match scrolling_box {
994+
ScrollingBox::Viewport(_) => {
999995
// For viewport scrolling, we need to add current scroll to get document-relative positions
1000-
let document_element_left = element_left + current_scroll_x;
1001-
let document_element_top = element_top + current_scroll_y;
1002-
let document_element_right = element_right + current_scroll_x;
1003-
let document_element_bottom = element_bottom + current_scroll_y;
1004-
1005996
(
1006-
self.calculate_scroll_position_one_axis(
1007-
inline,
1008-
document_element_left,
1009-
document_element_right,
1010-
element_width,
1011-
viewport_width,
1012-
current_scroll_x,
1013-
),
1014-
self.calculate_scroll_position_one_axis(
1015-
block,
1016-
document_element_top,
1017-
document_element_bottom,
1018-
element_height,
1019-
viewport_height,
1020-
current_scroll_y,
1021-
),
997+
element_top_left + current_scroll_position,
998+
element_bottom_right + current_scroll_position,
1022999
)
10231000
},
10241001
ScrollingBox::Element(scrolling_element) => {
10251002
let scrolling_node = scrolling_element.upcast::<Node>();
10261003
let scrolling_box = scrolling_node.border_box().unwrap_or_default();
1027-
let scrolling_left = scrolling_box.origin.x.to_nearest_pixel(device_pixel_ratio);
1028-
let scrolling_top = scrolling_box.origin.y.to_nearest_pixel(device_pixel_ratio);
1029-
let scrolling_width = scrolling_box
1030-
.size
1031-
.width
1032-
.to_nearest_pixel(device_pixel_ratio);
1033-
let scrolling_height = scrolling_box
1034-
.size
1035-
.height
1036-
.to_nearest_pixel(device_pixel_ratio);
1004+
let scrolling_top_left = scrolling_box.origin.map(to_pixel);
10371005

10381006
// Calculate element position in scroller's content coordinate system
10391007
// Element's viewport position relative to scroller, then add scroll offset to get content position
1040-
let viewport_relative_left = element_left - scrolling_left;
1041-
let viewport_relative_top = element_top - scrolling_top;
1042-
let viewport_relative_right = element_right - scrolling_left;
1043-
let viewport_relative_bottom = element_bottom - scrolling_top;
1008+
let viewport_relative_top_left = element_top_left - scrolling_top_left.to_vector();
1009+
let viewport_relative_bottom_right =
1010+
element_bottom_right - scrolling_top_left.to_vector();
10441011

10451012
// For absolutely positioned elements, we need to account for the positioning context
10461013
// If the element is positioned relative to an ancestor that's within the scrolling container,
10471014
// we need to adjust coordinates accordingly
1048-
let (
1049-
adjusted_relative_left,
1050-
adjusted_relative_top,
1051-
adjusted_relative_right,
1052-
adjusted_relative_bottom,
1053-
) = {
1015+
let (adjusted_relative_top_left, adjusted_relative_bottom_right) = {
10541016
// Check if this element has a positioned ancestor between it and the scrolling container
10551017
let mut current_node = self.upcast::<Node>().GetParentNode();
1056-
let mut final_coords = (
1057-
viewport_relative_left,
1058-
viewport_relative_top,
1059-
viewport_relative_right,
1060-
viewport_relative_bottom,
1061-
);
1018+
let mut final_coords =
1019+
(viewport_relative_top_left, viewport_relative_bottom_right);
10621020

10631021
while let Some(node) = current_node {
10641022
// Stop if we reach the scrolling container
@@ -1075,25 +1033,15 @@ impl Element {
10751033
// If this element establishes a positioning context,
10761034
// Get its bounding box to calculate the offset
10771035
let positioning_box = node.border_box().unwrap_or_default();
1078-
let positioning_left = positioning_box
1079-
.origin
1080-
.x
1081-
.to_nearest_pixel(device_pixel_ratio);
1082-
let positioning_top = positioning_box
1083-
.origin
1084-
.y
1085-
.to_nearest_pixel(device_pixel_ratio);
1036+
let positioning_top_left = positioning_box.origin.map(to_pixel);
10861037

10871038
// Calculate the offset of the positioning context relative to the scrolling container
1088-
let offset_left = positioning_left - scrolling_left;
1089-
let offset_top = positioning_top - scrolling_top;
1039+
let offset_top_left = positioning_top_left - scrolling_top_left;
10901040

10911041
// Adjust the coordinates by subtracting the positioning context offset
10921042
final_coords = (
1093-
viewport_relative_left - offset_left,
1094-
viewport_relative_top - offset_top,
1095-
viewport_relative_right - offset_left,
1096-
viewport_relative_bottom - offset_top,
1043+
viewport_relative_top_left - offset_top_left,
1044+
viewport_relative_bottom_right - offset_top_left,
10971045
);
10981046
break;
10991047
}
@@ -1106,35 +1054,32 @@ impl Element {
11061054
final_coords
11071055
};
11081056

1109-
let current_scroll_x = scrolling_element.ScrollLeft() as f32;
1110-
let current_scroll_y = scrolling_element.ScrollTop() as f32;
1111-
let content_element_left = adjusted_relative_left + current_scroll_x;
1112-
let content_element_top = adjusted_relative_top + current_scroll_y;
1113-
let content_element_right = adjusted_relative_right + current_scroll_x;
1114-
let content_element_bottom = adjusted_relative_bottom + current_scroll_y;
1057+
let content_element_top_left = adjusted_relative_top_left + current_scroll_position;
1058+
let content_element_bottom_right =
1059+
adjusted_relative_bottom_right + current_scroll_position;
11151060

1116-
(
1117-
self.calculate_scroll_position_one_axis(
1118-
inline,
1119-
content_element_left,
1120-
content_element_right,
1121-
element_width,
1122-
scrolling_width,
1123-
current_scroll_x,
1124-
),
1125-
self.calculate_scroll_position_one_axis(
1126-
block,
1127-
content_element_top,
1128-
content_element_bottom,
1129-
element_height,
1130-
scrolling_height,
1131-
current_scroll_y,
1132-
),
1133-
)
1061+
(content_element_top_left, content_element_bottom_right)
11341062
},
11351063
};
11361064

1137-
Vector2D::new(target_x, target_y)
1065+
Vector2D::new(
1066+
self.calculate_scroll_position_one_axis(
1067+
inline,
1068+
adjusted_element_top_left.x,
1069+
adjusted_element_bottom_right.x,
1070+
element_size.x,
1071+
scrolling_box_size.width,
1072+
current_scroll_position.x,
1073+
),
1074+
self.calculate_scroll_position_one_axis(
1075+
block,
1076+
adjusted_element_top_left.y,
1077+
adjusted_element_bottom_right.y,
1078+
element_size.y,
1079+
scrolling_box_size.height,
1080+
current_scroll_position.y,
1081+
),
1082+
)
11381083
}
11391084

11401085
fn calculate_scroll_position_one_axis(

0 commit comments

Comments
 (0)