Bug 1247074 - When a compositor-based smooth scroll animation is in progress and the scrollframe is reconstructed, restore to the animation destination. r?tnikkel
MozReview-Commit-ID: 73juHWNfoQy
--- a/layout/generic/nsGfxScrollFrame.cpp
+++ b/layout/generic/nsGfxScrollFrame.cpp
@@ -5726,28 +5726,36 @@ ScrollFrameHelper::SaveState() const
{
nsIScrollbarMediator* mediator = do_QueryFrame(GetScrolledFrame());
if (mediator) {
// child handles its own scroll state, so don't bother saving state here
return nullptr;
}
// Don't store a scroll state if we never have been scrolled or restored
- // a previous scroll state.
- if (!mHasBeenScrolled && !mDidHistoryRestore) {
+ // a previous scroll state, and we're not in the middle of a smooth scroll.
+ bool isInSmoothScroll = IsProcessingAsyncScroll() || mLastSmoothScrollOrigin;
+ if (!mHasBeenScrolled && !mDidHistoryRestore && !isInSmoothScroll) {
return nullptr;
}
nsPresState* state = new nsPresState();
// Save mRestorePos instead of our actual current scroll position, if it's
// valid and we haven't moved since the last update of mLastPos (same check
// that ScrollToRestoredPosition uses). This ensures if a reframe occurs
// while we're in the process of loading content to scroll to a restored
- // position, we'll keep trying after the reframe.
+ // position, we'll keep trying after the reframe. Similarly, if we're in the
+ // middle of a smooth scroll, store the destination so that when we restore
+ // we'll jump straight to the end of the scroll animation, rather than
+ // effectively dropping it. Note that the mRestorePos will override the
+ // smooth scroll destination if both are present.
nsPoint pt = GetLogicalScrollPosition();
+ if (isInSmoothScroll) {
+ pt = mDestination;
+ }
if (mRestorePos.y != -1 && pt == mLastPos) {
pt = mRestorePos;
}
state->SetScrollState(pt);
if (mIsRoot) {
// Only save resolution properties for root scroll frames
nsIPresShell* shell = mOuter->PresContext()->PresShell();
state->SetResolution(shell->GetResolution());
--- a/layout/generic/test/mochitest.ini
+++ b/layout/generic/test/mochitest.ini
@@ -140,8 +140,9 @@ support-files = selection_expanding_xbl.
[test_selection_preventDefault.html]
skip-if = buildapp == 'mulet'
[test_selection_splitText-normalize.html]
[test_selection_touchevents.html]
[test_taintedfilters.html]
support-files = file_taintedfilters_feDisplacementMap-tainted-1.svg file_taintedfilters_feDisplacementMap-tainted-2.svg file_taintedfilters_feDisplacementMap-tainted-3.svg file_taintedfilters_feDisplacementMap-tainted-ref.svg file_taintedfilters_feDisplacementMap-untainted-ref.svg file_taintedfilters_feDisplacementMap-untainted-1.svg file_taintedfilters_feDisplacementMap-untainted-2.svg file_taintedfilters_red-flood-for-feImage-cors.svg file_taintedfilters_red-flood-for-feImage-cors.svg^headers^ file_taintedfilters_red-flood-for-feImage.svg
[test_scroll_position_restore.html]
support-files = file_scroll_position_restore.html
+[test_scroll_animation_restore.html]
new file mode 100644
--- /dev/null
+++ b/layout/generic/test/test_scroll_animation_restore.html
@@ -0,0 +1,128 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=1247074
+-->
+<head>
+ <meta charset="utf-8">
+ <title>Test for Bug 1247074</title>
+ <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+ <script type="text/javascript" src="/tests/SimpleTest/paint_listener.js"></script>
+ <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+ <style>
+ .outer {
+ direction: ltr;
+ height: 400px;
+ width: 415px;
+ overflow: hidden;
+ position: relative;
+ }
+ .inner {
+ height: 100%;
+ outline: none;
+ overflow-x: hidden;
+ overflow-y: scroll;
+ position: relative;
+ scroll-behavior: smooth;
+ }
+ .outer.contentBefore::before {
+ top: 0;
+ content: '';
+ display: block;
+ height: 2px;
+ position: absolute;
+ width: 100%;
+ z-index: 99;
+ }
+ </style>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1247074">Mozilla Bug 1247074</a>
+<p id="display"></p>
+<div class="outer">
+ <div class="inner">
+ <ol>
+ <li>Some text</li>
+ <li>Some text</li>
+ <li>Some text</li>
+ <li>Some text</li>
+ <li>Some text</li>
+ <li>Some text</li>
+ <li>Some text</li>
+ <li>Some text</li>
+ <li>Some text</li>
+ <li>Some text</li>
+ <li>Some text</li>
+ <li>Some text</li>
+ <li>Some text</li>
+ <li>Some text</li>
+ <li>Some text</li>
+ <li>Some text</li>
+ <li>Some text</li>
+ <li>Some text</li>
+ <li>Some text</li>
+ <li>Some text</li>
+ <li>Some text</li>
+ <li>Some text</li>
+ <li>Some text</li>
+ <li>Some text</li>
+ <li>Some text</li>
+ <li>Some text</li>
+ <li>Some text</li>
+ <li>Some text</li>
+ <li>Some text</li>
+ <li>Some text</li>
+ <li>Some text</li>
+ <li>Some text</li>
+ <li>Some text</li>
+ <li>Some text</li>
+ <li>Some text</li>
+ <li>Some text</li>
+ <li>Some text</li>
+ <li>Some text</li>
+ <li>Some text</li>
+ <li>Some text</li>
+ <li>Some text</li>
+ <li>Some text</li>
+ <li>Some text</li>
+ <li>Some text</li>
+ <li>Some text</li>
+ <li>Some text</li>
+ <li>Some text</li>
+ <li>Some text</li>
+ <li>Some text</li>
+ <li>Some text</li>
+ <li>Some text</li>
+ <li>Some text</li>
+ </ol>
+ </div>
+</div>
+<script>
+SimpleTest.waitForExplicitFinish();
+window.onload = function() {
+ var elm = document.getElementsByClassName('inner')[0];
+
+ // Take control of the refresh driver
+ var utils = SpecialPowers.DOMWindowUtils;
+ utils.advanceTimeAndRefresh(0);
+
+ // Start a smooth scroll and advance a couple of frames so we're in the
+ // middle of the scroll animation
+ elm.scrollTop = 500;
+ utils.advanceTimeAndRefresh(16);
+ utils.advanceTimeAndRefresh(16);
+
+ // Trigger a frame reconstruction
+ elm.parentNode.classList.add('contentBefore');
+
+ // Reach a stable state and verify the scroll position is 500
+ utils.restoreNormalRefresh();
+ waitForAllPaintsFlushed(function() {
+ SimpleTest.is(elm.scrollTop, 500, "Scroll position ended up at 500");
+ SimpleTest.finish();
+ });
+}
+
+</script>
+</body>
+</html>