Bug 1950560 - Do not clobber restoring the scroll position on re-snap. r=botond

Differential Revision: https://phabricator.services.mozilla.com/D239677
This commit is contained in:
Hiroyuki Ikezoe
2025-02-27 02:34:52 +00:00
parent b2b048a314
commit 602f35e441
3 changed files with 65 additions and 2 deletions

View File

@@ -5178,7 +5178,7 @@ nsSize ScrollContainerFrame::GetPageScrollAmount() const {
* when we reach our new position. * when we reach our new position.
*/ */
void ScrollContainerFrame::ScrollToRestoredPosition() { void ScrollContainerFrame::ScrollToRestoredPosition() {
if (mRestorePos.y == -1 || mLastPos.x == -1 || mLastPos.y == -1) { if (!NeedRestorePosition()) {
return; return;
} }
// make sure our scroll position did not change for where we last put // make sure our scroll position did not change for where we last put
@@ -7638,8 +7638,13 @@ Maybe<SnapDestination> ScrollContainerFrame::GetSnapPointForDestination(
Maybe<SnapDestination> ScrollContainerFrame::GetSnapPointForResnap() { Maybe<SnapDestination> ScrollContainerFrame::GetSnapPointForResnap() {
nsIContent* focusedContent = nsIContent* focusedContent =
GetContent()->GetComposedDoc()->GetUnretargetedFocusedContent(); GetContent()->GetComposedDoc()->GetUnretargetedFocusedContent();
// While we are reconstructing this scroll container, we might be in the
// process of restoring the scroll position, we need to respect it.
nsPoint currentOrRestorePos =
NeedRestorePosition() ? mRestorePos : GetScrollPosition();
return ScrollSnapUtils::GetSnapPointForResnap( return ScrollSnapUtils::GetSnapPointForResnap(
ComputeScrollSnapInfo(), GetLayoutScrollRange(), GetScrollPosition(), ComputeScrollSnapInfo(), GetLayoutScrollRange(), currentOrRestorePos,
mLastSnapTargetIds, focusedContent); mLastSnapTargetIds, focusedContent);
} }

View File

@@ -503,6 +503,10 @@ class ScrollContainerFrame : public nsContainerFrame,
*/ */
void ScrollToRestoredPosition(); void ScrollToRestoredPosition();
bool NeedRestorePosition() const {
return mRestorePos.y != -1 && mLastPos.x != -1 && mLastPos.y != -1;
}
/** /**
* Add a scroll position listener. This listener must be removed * Add a scroll position listener. This listener must be removed
* before it is destroyed. * before it is destroyed.

View File

@@ -0,0 +1,54 @@
<!DOCTYPE html>
<link rel="help" href="https://drafts.csswg.org/css-scroll-snap/#re-snap" />
<link rel="help" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1948861">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<style>
#scroller {
scroll-snap-type: y mandatory;
width: 500px;
height: 200px;
overflow-y: scroll;
overflow-x: hidden;
scrollbar-width: none;
}
.child {
scroll-snap-align: start;
width: 500px;
height: 100%;
}
video {
height: 100%;
}
</style>
<div id="container">
<div id="scroller">
<div class="child">
<video src="dummy.webm" controls autoplay>
</div>
<div class="child">
<video src="dummy.webm" controls autoplay>
</div>
<div class="child">
<video src="dummy.webm" controls autoplay>
</div>
</div>
</div>
<script>
promise_test(async () => {
assert_equals(scroller.scrollTop, 0);
const scrollendPromise = new Promise(resolve => {
scroller.addEventListener("scrollend", resolve);
});
// Try to scroll downward, it will snap to the second green box.
scroller.scrollBy({ top: 100, behavior: "smooth" });
await scrollendPromise;
// Change the parent position style so that the scroll container will
// be reconstructed.
container.style.position = "fixed";
assert_equals(scroller.scrollTop, 200, "Should stay at the last snap point");
}, "Stay at the last snap point even after reconstrucing the scroll container");
</script>