Bug 2071197 - Prevent two scroll events being sent on the same frame after bug 2015089. r=layout-reviewers,dshin

Save and restore scroll event generations to prevent sending two events
on the same frame.

Differential Revision: https://phabricator.services.mozilla.com/D325321
This commit is contained in:
Emilio Cobos Álvarez
2026-09-12 15:13:20 +00:00
committed by ealvarez@mozilla.com
parent 39dcc5beb7
commit 35a9baf9bd
10 changed files with 96 additions and 19 deletions
+2 -1
View File
@@ -4303,7 +4303,8 @@ nsresult PresShell::CaptureHistoryState(nsILayoutHistoryState** aState) {
return NS_OK;
}
mFrameConstructor->CaptureFrameState(rootFrame, historyState);
mFrameConstructor->CaptureFrameState(rootFrame, historyState,
{CaptureStateFlag::ForSessionHistory});
return NS_OK;
}
+3 -1
View File
@@ -48,8 +48,10 @@ struct PresState {
// For frames where layout and visual viewport aren't one and the same thing,
// scrollState will store the position of the *visual* viewport.
nsPoint scrollState;
bool allowScrollOriginDowngrade;
uint32_t scrollEventGeneration;
uint32_t scrollEndEventGeneration;
float resolution;
bool allowScrollOriginDowngrade;
bool disabledSet;
bool disabled;
bool droppedDown;
+1 -1
View File
@@ -7621,7 +7621,7 @@ void nsCSSFrameConstructor::CaptureStateForFramesOf(
}
for (; frame;
frame = nsLayoutUtils::GetNextContinuationOrIBSplitSibling(frame)) {
CaptureFrameState(frame, aHistoryState);
CaptureFrameState(frame, aHistoryState, {});
}
}
+7 -5
View File
@@ -125,7 +125,8 @@ void nsFrameManager::RemoveFrame(DestroyContext& aContext,
// Accept a content id here, in some cases we may not have content (scroll
// position)
void nsFrameManager::CaptureFrameStateFor(nsIFrame* aFrame,
nsILayoutHistoryState* aState) {
nsILayoutHistoryState* aState,
CaptureStateFlags aFlags) {
if (!aFrame || !aState) {
NS_WARNING("null frame, or state");
return;
@@ -138,7 +139,7 @@ void nsFrameManager::CaptureFrameStateFor(nsIFrame* aFrame,
}
// Capture the state, exit early if we get null (nothing to save)
UniquePtr<PresState> frameState = statefulFrame->SaveState();
UniquePtr<PresState> frameState = statefulFrame->SaveState(aFlags);
if (!frameState) {
return;
}
@@ -158,11 +159,12 @@ void nsFrameManager::CaptureFrameStateFor(nsIFrame* aFrame,
}
void nsFrameManager::CaptureFrameState(nsIFrame* aFrame,
nsILayoutHistoryState* aState) {
nsILayoutHistoryState* aState,
CaptureStateFlags aFlags) {
MOZ_ASSERT(nullptr != aFrame && nullptr != aState,
"null parameters passed in");
CaptureFrameStateFor(aFrame, aState);
CaptureFrameStateFor(aFrame, aState, aFlags);
// Now capture state recursively for the frame hierarchy rooted at aFrame
for (const auto& childList : aFrame->ChildLists()) {
@@ -182,7 +184,7 @@ void nsFrameManager::CaptureFrameState(nsIFrame* aFrame,
// out-of-flows and fragmentation. We handle that unexpected situation by
// silently skipping this frame, rather than crashing.
if (MOZ_LIKELY(realChild)) {
CaptureFrameState(realChild, aState);
CaptureFrameState(realChild, aState, aFlags);
}
}
}
+6 -5
View File
@@ -10,6 +10,7 @@
#include "mozilla/Attributes.h"
#include "nsDebug.h"
#include "nsFrameList.h"
#include "nsIStatefulFrame.h"
class nsContainerFrame;
class nsIFrame;
@@ -73,12 +74,12 @@ class nsFrameManager {
* needed; this method will only work with actual frametree descendants
* of aFrame.
*/
void CaptureFrameState(nsIFrame* aFrame, nsILayoutHistoryState* aState);
void CaptureFrameState(nsIFrame*, nsILayoutHistoryState*,
mozilla::CaptureStateFlags);
/*
* Add/restore state for one frame
*/
void CaptureFrameStateFor(nsIFrame* aFrame, nsILayoutHistoryState* aState);
// Capture state for a single frame.
void CaptureFrameStateFor(nsIFrame*, nsILayoutHistoryState*,
mozilla::CaptureStateFlags);
void RestoreFrameStateFor(nsIFrame* aFrame, nsILayoutHistoryState* aState);
+5 -3
View File
@@ -18,14 +18,14 @@ using namespace mozilla;
class nsLayoutHistoryState final : public nsILayoutHistoryState,
public nsSupportsWeakReference {
public:
nsLayoutHistoryState() : mScrollPositionOnly(false) {}
nsLayoutHistoryState() = default;
NS_DECL_ISUPPORTS
NS_DECL_NSILAYOUTHISTORYSTATE
private:
~nsLayoutHistoryState() = default;
bool mScrollPositionOnly;
bool mScrollPositionOnly = false;
nsTHashMap<nsCString, UniquePtr<PresState>> mStates;
};
@@ -168,6 +168,8 @@ UniquePtr<PresState> NewPresState() {
/* resolution */ 1.0,
/* disabledSet */ false,
/* disabled */ false,
/* droppedDown */ false);
/* droppedDown */ false,
/* scrollEventGeneration */ 0,
/* scrollEndEventGeneration */ 0);
}
} // namespace mozilla
+9 -1
View File
@@ -7397,7 +7397,7 @@ void ScrollContainerFrame::ResetScrollInfoIfNeeded(
mInScrollingGesture = aInScrollingGesture;
}
UniquePtr<PresState> ScrollContainerFrame::SaveState() {
UniquePtr<PresState> ScrollContainerFrame::SaveState(CaptureStateFlags aFlags) {
nsIScrollbarMediator* mediator = do_QueryFrame(GetScrolledFrame());
if (mediator) {
// child handles its own scroll state, so don't bother saving state here
@@ -7441,6 +7441,12 @@ UniquePtr<PresState> ScrollContainerFrame::SaveState() {
}
state->scrollState() = pt;
state->allowScrollOriginDowngrade() = allowScrollOriginDowngrade;
// Scroll event generations are per-PresShell, so they're meaningless when
// restoring from session history.
if (!aFlags.contains(CaptureStateFlag::ForSessionHistory)) {
state->scrollEventGeneration() = mScrollEventGeneration;
state->scrollEndEventGeneration() = mScrollEndEventGeneration;
}
if (mIsRoot) {
// Only save resolution properties for root scroll frames
state->resolution() = PresShell()->GetResolution();
@@ -7463,6 +7469,8 @@ NS_IMETHODIMP ScrollContainerFrame::RestoreState(PresState* aState) {
// future or if we tinker with this code more.
mLastScrollOrigin = ScrollOrigin::Other;
mDidHistoryRestore = true;
mScrollEventGeneration = aState->scrollEventGeneration();
mScrollEndEventGeneration = aState->scrollEndEventGeneration();
mLastPos = mScrolledFrame ? GetLogicalVisualViewportOffset() : nsPoint(0, 0);
SCROLLRESTORE_LOG("%p: RestoreState, set mRestorePos=%s mLastPos=%s\n", this,
ToString(mRestorePos).c_str(), ToString(mLastPos).c_str());
+1 -1
View File
@@ -952,7 +952,7 @@ class ScrollContainerFrame : public nsContainerFrame,
void ReflowCallbackCanceled() final;
// nsIStatefulFrame
UniquePtr<PresState> SaveState() final;
UniquePtr<PresState> SaveState(CaptureStateFlags aFlags) final;
NS_IMETHOD RestoreState(PresState* aState) final;
// nsIScrollbarMediator
+11 -1
View File
@@ -10,11 +10,20 @@
#ifndef _nsIStatefulFrame_h
#define _nsIStatefulFrame_h
#include "mozilla/EnumSet.h"
#include "nsContentUtils.h"
#include "nsQueryFrame.h"
namespace mozilla {
class PresState;
enum class CaptureStateFlag : uint8_t {
// Whether we're capturing frame state for session history, rather than for
// frame reconstruction within the same PresShell.
ForSessionHistory,
};
using CaptureStateFlags = EnumSet<CaptureStateFlag>;
} // namespace mozilla
class nsIStatefulFrame {
@@ -22,7 +31,8 @@ class nsIStatefulFrame {
NS_DECL_QUERYFRAME_TARGET(nsIStatefulFrame)
// Save the state for this frame.
virtual mozilla::UniquePtr<mozilla::PresState> SaveState() = 0;
virtual mozilla::UniquePtr<mozilla::PresState> SaveState(
mozilla::CaptureStateFlags aFlags) = 0;
// Restore the state for this frame from aState
NS_IMETHOD RestoreState(mozilla::PresState* aState) = 0;
@@ -0,0 +1,51 @@
<!doctype html>
<meta charset="utf-8">
<title>Only one scroll / scrollend event is fired per frame, even if the scroller is hidden and shown again in between scrolls</title>
<link rel="author" title="Emilio Cobos Álvarez" href="mailto:emilio@crisal.io">
<link rel="author" title="Mozilla" href="https://mozilla.org">
<link rel="help" href="https://drafts.csswg.org/cssom-view/#document-run-the-scroll-steps">
<link rel="help" href="https://bugzilla.mozilla.org/show_bug.cgi?id=2015089">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="scroll_support.js"></script>
<style>
#scroller {
overflow: auto;
width: 100px;
height: 100px;
}
#scroller::before {
display: block;
content: "";
height: 300px;
}
</style>
<div id="scroller"></div>
<script>
for (const eventName of ["scroll", "scrollend"]) {
promise_test(async t => {
const scroller = document.getElementById("scroller");
scroller.scrollTop = 0;
await waitForNextFrame();
await waitForNextFrame();
let count = 0;
const listener = () => count++;
scroller.addEventListener(eventName, listener);
t.add_cleanup(() => scroller.removeEventListener(eventName, listener));
scroller.scrollTop = 10;
// Hide the scroller and force layout, so that its layout box gets
// destroyed and re-created before the second scroll.
scroller.style.display = "none";
scroller.getBoundingClientRect();
scroller.style.display = "";
scroller.scrollTop = 20;
assert_equals(scroller.scrollTop, 20);
await waitForNextFrame();
await waitForNextFrame();
assert_equals(count, 1, `Should get exactly one ${eventName} event`);
}, `Only one ${eventName} event fires when scrolling, toggling display: none, and scrolling again in the same frame`);
}
</script>