mirror of
https://github.com/servo/servo.git
synced 2025-09-25 14:20:08 +01:00
script: Remove absolute positioning workaround from scrollIntoView implementation (#39441)
This isn't needed as the border box query already takes into account the containing block chain. Instead, consistently calculate the new scroll position for a scroller relative to its current scroll offset. In addition, fix a small bug where the border of a scroll container was considered part of scrollport. Testing: A new WPT test is added. Signed-off-by: Martin Robinson <mrobinson@igalia.com> Co-authored-by: Oriol Brufau <obrufau@igalia.com>
This commit is contained in:
Martin Robinson
GitHub
parent
16d2e030eb
commit
c63311af02
5 changed files with 104 additions and 119 deletions
|
|
@ -36,7 +36,6 @@ use selectors::sink::Push;
|
|||
use servo_arc::Arc;
|
||||
use style::applicable_declarations::ApplicableDeclarationBlock;
|
||||
use style::attr::{AttrValue, LengthOrPercentageOrAuto};
|
||||
use style::computed_values::position::T as Position;
|
||||
use style::context::QuirksMode;
|
||||
use style::invalidation::element::restyle_hints::RestyleHint;
|
||||
use style::properties::longhands::{
|
||||
|
|
@ -966,108 +965,55 @@ impl Element {
|
|||
}
|
||||
}
|
||||
|
||||
/// <https://drafts.csswg.org/cssom-view/#element-scrolling-members>
|
||||
/// <https://drafts.csswg.org/cssom-view/#determine-the-scroll-into-view-position>
|
||||
fn determine_scroll_into_view_position(
|
||||
&self,
|
||||
scrolling_box: &ScrollingBox,
|
||||
block: ScrollLogicalPosition,
|
||||
inline: ScrollLogicalPosition,
|
||||
) -> LayoutVector2D {
|
||||
let target_bounding_box = self.upcast::<Node>().border_box().unwrap_or_default();
|
||||
|
||||
let device_pixel_ratio = self
|
||||
.upcast::<Node>()
|
||||
.owner_doc()
|
||||
.window()
|
||||
.device_pixel_ratio()
|
||||
.get();
|
||||
|
||||
// Target element bounds
|
||||
let to_pixel = |value: Au| value.to_nearest_pixel(device_pixel_ratio);
|
||||
let element_top_left = target_bounding_box.origin.map(to_pixel).to_untyped();
|
||||
let element_bottom_right = target_bounding_box.max().map(to_pixel);
|
||||
let element_size = target_bounding_box.size.to_vector().map(to_pixel);
|
||||
|
||||
let scrolling_box_size = scrolling_box.size();
|
||||
let current_scroll_position = scrolling_box.scroll_position().to_untyped();
|
||||
// > Step 1: Let target bounding border box be the box represented by the return
|
||||
// > value of invoking Element’s getBoundingClientRect(), if target is an Element,
|
||||
// > or Range’s getBoundingClientRect(), if target is a Range.
|
||||
let target_border_box = self.upcast::<Node>().border_box().unwrap_or_default();
|
||||
let target_top_left = target_border_box.origin.map(to_pixel).to_untyped();
|
||||
let target_bottom_right = target_border_box.max().map(to_pixel);
|
||||
|
||||
// The rest of the steps diverge from the specification here, but essentially try
|
||||
// to follow it using our own geometry types.
|
||||
//
|
||||
// TODO: This makes the code below wrong for the purposes of writing modes.
|
||||
let (adjusted_element_top_left, adjusted_element_bottom_right) = match scrolling_box {
|
||||
ScrollingBox::Viewport(_) => {
|
||||
// For viewport scrolling, we need to add current scroll to get document-relative positions
|
||||
ScrollingBox::Viewport(_) => (target_top_left, target_bottom_right),
|
||||
ScrollingBox::Element(scrolling_element) => {
|
||||
let scrolling_padding_rect_top_left = scrolling_element
|
||||
.upcast::<Node>()
|
||||
.padding_box()
|
||||
.unwrap_or_default()
|
||||
.origin
|
||||
.map(to_pixel);
|
||||
(
|
||||
element_top_left + current_scroll_position,
|
||||
element_bottom_right + current_scroll_position,
|
||||
target_top_left - scrolling_padding_rect_top_left.to_vector(),
|
||||
target_bottom_right - scrolling_padding_rect_top_left.to_vector(),
|
||||
)
|
||||
},
|
||||
ScrollingBox::Element(scrolling_element) => {
|
||||
let scrolling_node = scrolling_element.upcast::<Node>();
|
||||
let scrolling_box = scrolling_node.border_box().unwrap_or_default();
|
||||
let scrolling_top_left = scrolling_box.origin.map(to_pixel);
|
||||
|
||||
// Calculate element position in scroller's content coordinate system
|
||||
// Element's viewport position relative to scroller, then add scroll offset to get content position
|
||||
let viewport_relative_top_left = element_top_left - scrolling_top_left.to_vector();
|
||||
let viewport_relative_bottom_right =
|
||||
element_bottom_right - scrolling_top_left.to_vector();
|
||||
|
||||
// For absolutely positioned elements, we need to account for the positioning context
|
||||
// If the element is positioned relative to an ancestor that's within the scrolling container,
|
||||
// we need to adjust coordinates accordingly
|
||||
let (adjusted_relative_top_left, adjusted_relative_bottom_right) = {
|
||||
// Check if this element has a positioned ancestor between it and the scrolling container
|
||||
let mut current_node = self.upcast::<Node>().GetParentNode();
|
||||
let mut final_coords =
|
||||
(viewport_relative_top_left, viewport_relative_bottom_right);
|
||||
|
||||
while let Some(node) = current_node {
|
||||
// Stop if we reach the scrolling container
|
||||
if &*node == scrolling_node {
|
||||
break;
|
||||
}
|
||||
|
||||
// Check if this node establishes a positioning context and has position relative/absolute
|
||||
if let Some(element) = node.downcast::<Element>() {
|
||||
if let Some(computed_style) = element.style() {
|
||||
let position = computed_style.get_box().position;
|
||||
|
||||
if matches!(position, Position::Relative | Position::Absolute) {
|
||||
// If this element establishes a positioning context,
|
||||
// Get its bounding box to calculate the offset
|
||||
let positioning_box = node.border_box().unwrap_or_default();
|
||||
let positioning_top_left = positioning_box.origin.map(to_pixel);
|
||||
|
||||
// Calculate the offset of the positioning context relative to the scrolling container
|
||||
let offset_top_left = positioning_top_left - scrolling_top_left;
|
||||
|
||||
// Adjust the coordinates by subtracting the positioning context offset
|
||||
final_coords = (
|
||||
viewport_relative_top_left - offset_top_left,
|
||||
viewport_relative_bottom_right - offset_top_left,
|
||||
);
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
current_node = node.GetParentNode();
|
||||
}
|
||||
|
||||
final_coords
|
||||
};
|
||||
|
||||
let content_element_top_left = adjusted_relative_top_left + current_scroll_position;
|
||||
let content_element_bottom_right =
|
||||
adjusted_relative_bottom_right + current_scroll_position;
|
||||
|
||||
(content_element_top_left, content_element_bottom_right)
|
||||
},
|
||||
};
|
||||
|
||||
let scrolling_box_size = scrolling_box.size();
|
||||
let current_scroll_position = scrolling_box.scroll_position();
|
||||
Vector2D::new(
|
||||
self.calculate_scroll_position_one_axis(
|
||||
inline,
|
||||
adjusted_element_top_left.x,
|
||||
adjusted_element_bottom_right.x,
|
||||
element_size.x,
|
||||
scrolling_box_size.width,
|
||||
current_scroll_position.x,
|
||||
),
|
||||
|
|
@ -1075,22 +1021,23 @@ impl Element {
|
|||
block,
|
||||
adjusted_element_top_left.y,
|
||||
adjusted_element_bottom_right.y,
|
||||
element_size.y,
|
||||
scrolling_box_size.height,
|
||||
current_scroll_position.y,
|
||||
),
|
||||
)
|
||||
}
|
||||
|
||||
/// Step 10 from <https://drafts.csswg.org/cssom-view/#determine-the-scroll-into-view-position>:
|
||||
fn calculate_scroll_position_one_axis(
|
||||
&self,
|
||||
alignment: ScrollLogicalPosition,
|
||||
element_start: f32,
|
||||
element_end: f32,
|
||||
element_size: f32,
|
||||
container_size: f32,
|
||||
current_scroll_offset: f32,
|
||||
) -> f32 {
|
||||
let element_size = element_end - element_start;
|
||||
current_scroll_offset +
|
||||
match alignment {
|
||||
// Step 1 & 5: If inline is "start", then align element start edge with scrolling box start edge.
|
||||
ScrollLogicalPosition::Start => element_start,
|
||||
|
|
@ -1099,7 +1046,9 @@ impl Element {
|
|||
ScrollLogicalPosition::End => element_end - container_size,
|
||||
// Step 3 & 7: 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.
|
||||
ScrollLogicalPosition::Center => element_start + (element_size - container_size) / 2.0,
|
||||
ScrollLogicalPosition::Center => {
|
||||
element_start + (element_size - container_size) / 2.0
|
||||
},
|
||||
// Step 4 & 8: If inline is "nearest",
|
||||
ScrollLogicalPosition::Nearest => {
|
||||
let viewport_start = current_scroll_offset;
|
||||
|
|
|
|||
|
|
@ -956,6 +956,11 @@ impl Node {
|
|||
.box_area_query(self, BoxAreaType::Border)
|
||||
}
|
||||
|
||||
pub(crate) fn padding_box(&self) -> Option<Rect<Au>> {
|
||||
self.owner_window()
|
||||
.box_area_query(self, BoxAreaType::Padding)
|
||||
}
|
||||
|
||||
pub(crate) fn border_boxes(&self) -> Vec<Rect<Au>> {
|
||||
self.owner_window()
|
||||
.box_areas_query(self, BoxAreaType::Border)
|
||||
|
|
|
|||
7
tests/wpt/meta/MANIFEST.json
vendored
7
tests/wpt/meta/MANIFEST.json
vendored
|
|
@ -632462,6 +632462,13 @@
|
|||
{}
|
||||
]
|
||||
],
|
||||
"scrollIntoView-scrolling-box-with-large-border.html": [
|
||||
"424c6f56bd0b8fb01e268184533a6b081253be71",
|
||||
[
|
||||
null,
|
||||
{}
|
||||
]
|
||||
],
|
||||
"scrollIntoView-scrolling-container.html": [
|
||||
"9f7c88aaeb39610d5adffd046f72bb3a10491fb4",
|
||||
[
|
||||
|
|
|
|||
2
tests/wpt/meta/focus/focus-large-element-in-overflow-hidden-container.html.ini
vendored
Normal file
2
tests/wpt/meta/focus/focus-large-element-in-overflow-hidden-container.html.ini
vendored
Normal file
|
|
@ -0,0 +1,2 @@
|
|||
[focus-large-element-in-overflow-hidden-container.html]
|
||||
expected: FAIL
|
||||
22
tests/wpt/tests/css/cssom-view/scrollIntoView-scrolling-box-with-large-border.html
vendored
Normal file
22
tests/wpt/tests/css/cssom-view/scrollIntoView-scrolling-box-with-large-border.html
vendored
Normal file
|
|
@ -0,0 +1,22 @@
|
|||
<!DOCTYPE html>
|
||||
|
||||
<title>CSSOM View - scrollIntoView should account for the border of scrollable elements.</title>
|
||||
<link rel="help" href="https://drafts.csswg.org/cssom-view/#dom-element-scrollintoview">
|
||||
<link rel="author" title="Martin Robinson" href="mailto:mrobinson@igalia.com">
|
||||
<link rel="author" title="Oriol Brufau" href="mailto:obrufau@igalia.com">
|
||||
|
||||
<script src="/resources/testharness.js"></script>
|
||||
<script src="/resources/testharnessreport.js"></script>
|
||||
|
||||
<div id="scroller" style="overflow: scroll; border: solid 100px; width: 100px; height: 100px;">
|
||||
<div style="height: 1000px;"></div>
|
||||
<div id="target" style="background: green; width: 100px; height: 100px"></div>
|
||||
<div style="height: 1000px;"></div>
|
||||
</div>
|
||||
|
||||
<script>
|
||||
test(function() {
|
||||
target.scrollIntoView();
|
||||
assert_equals(1000, scroller.scrollTop, "Should scroll the target into view.");
|
||||
}, "scrollIntoView takes into account the border of the scroller");
|
||||
</script>
|
||||
Loading…
Add table
Add a link
Reference in a new issue