Magic-Mirror/gecko-dev
1
0
Fork 0
mirror of https://github.com/mozilla/gecko-dev.git synced 2024-10-20 08:45:46 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Bug 1561450 - Suppress scroll anchoring adjustments from scroll event listeners. r=dholbert

Browse Source 
See https://github.com/w3c/csswg-drafts/issues/4239

This fixes what is in my opinion one of the biggest issues of scroll anchoring
as specified / currently implemented.

It's trivial to flush layout on a scroll handler and create scroll adjustments,
which in turn would trigger other scroll events to be fired. This situation,
which is what has happened in most of the scroll anchoring regressions I've
fixed, at best gets pages to get stuck in an infinite CPU loop. At worst, it
causes scrolling to be unusable because the page keeps reacting to scroll
events.

An alternative, slightly more elegant but not clear to me if 100% implementable
approach would be bug 1529702.

This should enormously reduce the need for scroll anchoring suppression
triggers[1], I'd think, which in turn would allow me to investigate removing
some of that complexity.

https://drafts.csswg.org/css-scroll-anchoring/#suppression-triggers

The setup to set / unset the boolean is a bit awkward but I couldn't come up
with anything better.

Differential Revision: https://phabricator.services.mozilla.com/D43233

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Emilio Cobos Álvarez 2019-08-23 22:29:49 +00:00
parent f4fb9ab137
commit 31c8224bf5
4 changed files with 57 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
layout/generic/ScrollAnchorContainer.cpp
View File

@ -320,13 +320,15 @@ void ScrollAnchorContainer::Destroy() {
void ScrollAnchorContainer::ApplyAdjustments() {
if (!mAnchorNode || mAnchorNodeIsDirty ||
mScrollFrame->HasPendingScrollRestoration() ||
mScrollFrame->IsProcessingScrollEvent() ||
mScrollFrame->IsProcessingAsyncScroll()) {
mSuppressAnchorAdjustment = false;
ANCHOR_LOG(
"Ignoring post-reflow (anchor=%p, dirty=%d, pendingRestoration=%d, "
"asyncScroll=%d container=%p).\n",
"scrollevent=%d asyncScroll=%d container=%p).\n",
mAnchorNode, mAnchorNodeIsDirty,
mScrollFrame->HasPendingScrollRestoration(),
mScrollFrame->IsProcessingScrollEvent(),
mScrollFrame->IsProcessingAsyncScroll(), this);
return;
}

11
layout/generic/nsGfxScrollFrame.cpp
View File

@ -2114,6 +2114,7 @@ ScrollFrameHelper::ScrollFrameHelper(nsContainerFrame* aOuter, bool aIsRoot)
mSuppressScrollbarRepaints(false),
mIsUsingMinimumScaleSize(false),
mMinimumScaleSizeChanged(false),
mProcessingScrollEvent(false),
mVelocityQueue(aOuter->PresContext()) {
if (LookAndFeel::GetInt(LookAndFeel::eIntID_UseOverlayScrollbars) != 0) {
mScrollbarActivity = new ScrollbarActivity(do_QueryFrame(aOuter));
@ -5227,6 +5228,16 @@ void ScrollFrameHelper::FireScrollEvent() {
return;
}
bool oldProcessing = mProcessingScrollEvent;
AutoWeakFrame weakFrame(mOuter);
auto RestoreProcessingScrollEvent = mozilla::MakeScopeExit([&] {
if (weakFrame.IsAlive()) { // Otherwise `this` will be dead too.
mProcessingScrollEvent = oldProcessing;
}
});
mProcessingScrollEvent = true;
ActiveLayerTracker::SetCurrentScrollHandlerFrame(mOuter);
WidgetGUIEvent event(true, eScroll, nullptr);
nsEventStatus status = nsEventStatus_eIgnore;

5
layout/generic/nsGfxScrollFrame.h
View File

@ -205,6 +205,8 @@ class ScrollFrameHelper : public nsIReflowCallback {
return mRestorePos != nsPoint(-1, -1);
}
bool IsProcessingScrollEvent() const { return mProcessingScrollEvent; }
protected:
nsRect GetVisualScrollRange() const;
@ -700,6 +702,9 @@ class ScrollFrameHelper : public nsIReflowCallback {
// True if the minimum scale size has been changed since the last reflow.
bool mMinimumScaleSizeChanged : 1;
// True if we're processing an scroll event.
bool mProcessingScrollEvent : 1;
mozilla::layout::ScrollVelocityQueue mVelocityQueue;
protected:

38
testing/web-platform/tests/css/css-scroll-anchoring/infinite-scroll-event.tentative.html Normal file
View File

@ -0,0 +1,38 @@
<!DOCTYPE html>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<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://bugzilla.mozilla.org/show_bug.cgi?id=1561450">
<link rel="help" href="https://drafts.csswg.org/css-scroll-anchoring/#suppression-triggers">
<style>
body { margin: 0 }
.content {
height: 45vh;
background: lightblue;
}
</style>
<div class="content"></div>
<div id="hidden" style="display: none; height: 200px"></div>
<div class="content"></div>
<div class="content"></div>
<div class="content"></div>
<script>
let first = true;
const t = async_test("Scroll adjustments don't happen if triggered from scroll event listeners");
onscroll = t.step_func(function() {
assert_true(first, "Should only get one event");
first = false;
hidden.style.display = "block";
hidden.offsetTop;
hidden.style.display = "none";
requestAnimationFrame(t.step_func(function() {
requestAnimationFrame(t.step_func(function() {
t.done();
}));
}));
});
window.onload = t.step_func(function() {
window.scrollTo(0, document.scrollingElement.scrollTopMax - 200);
});
</script>