Bug 1519621 - Make sure ScrollToRestoredPosition() restores both the layout and visual scroll positions. r=tnikkel
authorBotond Ballo <botond@mozilla.com>
Fri, 01 Mar 2019 20:18:35 +0000
changeset 461990 d5c0cad448f90eae7cf255f9a5e5ce45bbc0b299
parent 461989 83a0072bff1b0066b1d58f59bcd4aaa1842ddff2
child 461991 cd326f5e4eb883c0daa8dd5022c99fc886dda9bf
push id35634
push userrmaries@mozilla.com
push dateSat, 02 Mar 2019 09:26:10 +0000
treeherdermozilla-central@4166cae81546 [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
gfx/layers/apz/test/reftest/reftest.list
layout/generic/nsGfxScrollFrame.cpp
testing/web-platform/meta/css/cssom-view/scroll-behavior-smooth.html.ini
--- a/gfx/layers/apz/test/reftest/reftest.list
+++ b/gfx/layers/apz/test/reftest/reftest.list
@@ -17,17 +17,17 @@ fuzzy-if(Android,0-45,0-27) skip-if(!And
 # As above, the end of the scrollthumb won't match perfectly, but the bulk of the scrollbar should be present and identical.
 fuzzy-if(Android,0-54,0-14) skip-if(!Android) pref(apz.allow_zooming,true) == scrollbar-zoom-resolution-1.html scrollbar-zoom-resolution-1-ref.html
 fuzzy-if(Android,0-51,0-22) skip-if(!Android) pref(apz.allow_zooming,true) == scrollbar-zoom-resolution-2.html scrollbar-zoom-resolution-2-ref.html
 
 # Meta-viewport tag support
 skip-if(!Android) pref(apz.allow_zooming,true) == initial-scale-1.html initial-scale-1-ref.html
 
 # Bug 1520320 is tracking the effort to make this test pass again.
-skip-if(!asyncPan) fails-if(Android) == frame-reconstruction-scroll-clamping.html frame-reconstruction-scroll-clamping-ref.html
+skip-if(!asyncPan) random-if(Android) == frame-reconstruction-scroll-clamping.html frame-reconstruction-scroll-clamping-ref.html
 
 # Test that position:fixed and position:sticky elements are attached to the
 # layout viewport.
 #
 # We skip these tests on Desktop platforms since they require container
 # scrolling, which is enabled by default on Android, but behind a "Once" pref
 # and cannot be enabled for individual reftests.
 skip-if(!Android) pref(apz.allow_zooming,true) == pinch-zoom-position-fixed.html pinch-zoom-position-fixed-ref.html
--- a/layout/generic/nsGfxScrollFrame.cpp
+++ b/layout/generic/nsGfxScrollFrame.cpp
@@ -4418,22 +4418,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
-