Bug 1519621 - Make sure ScrollToRestoredPosition() restores both the layout and visual scroll positions. r=tnikkel
☠☠ backed out by 1b973064ca57 ☠ ☠
authorBotond Ballo <botond@mozilla.com>
Wed, 27 Feb 2019 20:19:33 +0000
changeset 519396 2c0ca241bd4b662f7022beb535509164ab7cde51
parent 519395 30aad4a580eb65ad099c1c4c2d897db85b7a76ec
child 519397 da83e69c73310999b5c68b769b614351f279ca34
push id10862
push userffxbld-merge
push dateMon, 11 Mar 2019 13:01:11 +0000
treeherdermozilla-beta@a2e7f5c935da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstnikkel
bugs1519621
milestone67.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1519621 - Make sure ScrollToRestoredPosition() restores both the layout and visual scroll positions. r=tnikkel Differential Revision: https://phabricator.services.mozilla.com/D18367
layout/generic/nsGfxScrollFrame.cpp
testing/web-platform/meta/css/cssom-view/scroll-behavior-smooth.html.ini
--- a/layout/generic/nsGfxScrollFrame.cpp
+++ b/layout/generic/nsGfxScrollFrame.cpp
@@ -4411,22 +4411,32 @@ void ScrollFrameHelper::ScrollToRestored
   // make sure our scroll position did not change for where we last put
   // it. if it does then the user must have moved it, and we no longer
   // need to restore.
   //
   // In the RTL case, we check whether the scroll position changed using the
   // logical scroll position, but we scroll to the physical scroll position in
   // all cases
 
+  // Continue restoring until both the layout and visual scroll positions
+  // reach the destination. (Note that the two can only be different for
+  // the root content document's root scroll frame, and when zoomed in).
+  // This is necessary to avoid situations where the two offsets get stuck
+  // at different values and nothing reconciles them (see bug 1519621 comment
+  // 8).
+  nsPoint logicalLayoutScrollPos = GetLogicalScrollPosition();
+
   // if we didn't move, we still need to restore
-  if (GetLogicalVisualViewportOffset() == mLastPos) {
+  if (GetLogicalVisualViewportOffset() == mLastPos ||
+      logicalLayoutScrollPos == mLastPos) {
     // if our desired position is different to the scroll position, scroll.
     // remember that we could be incrementally loading so we may enter
     // and scroll many times.
-    if (mRestorePos != mLastPos /* GetLogicalVisualViewportOffset() */) {
+    if (mRestorePos != mLastPos /* GetLogicalVisualViewportOffset() */ ||
+        mRestorePos != logicalLayoutScrollPos) {
       LoadingState state = GetPageLoadingState();
       if (state == LoadingState::Stopped && !NS_SUBTREE_DIRTY(mOuter)) {
         return;
       }
       nsPoint scrollToPos = mRestorePos;
       if (!IsPhysicalLTR()) {
         // convert from logical to physical scroll position
         scrollToPos.x -=
--- a/testing/web-platform/meta/css/cssom-view/scroll-behavior-smooth.html.ini
+++ b/testing/web-platform/meta/css/cssom-view/scroll-behavior-smooth.html.ini
@@ -1,8 +1,3 @@
 [scroll-behavior-smooth.html]
   [BODY element scroll-behavior should not propagate to viewport]
     expected: FAIL
-
-  [Smooth scrolling while doing history navigation.]
-    expected:
-      if (os == "android"): FAIL
-