Bug 1416391 - Fix Element.scrollIntoView in "nearest" mode to behave according to CSSOM spec. r=tnikkel a=gchang
authorOriol Brufau <oriol-bugzilla@hotmail.com>
Sat, 11 Nov 2017 01:39:33 +0100
changeset 445056 71956196206bba1b9fab23b31ce5201eb0c52d0d
parent 445055 122e73935baa8d5dc0fcd0f473ff28deb22a5fd6
child 445057 b9d29c50dff5b02df9aa33af951baf0981eb485a
push id1618
push userCallek@gmail.com
push dateThu, 11 Jan 2018 17:45:48 +0000
treeherdermozilla-release@882ca853e05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstnikkel, gchang
bugs1416391
milestone58.0
Bug 1416391 - Fix Element.scrollIntoView in "nearest" mode to behave according to CSSOM spec. r=tnikkel a=gchang MozReview-Commit-ID: 5xhqKQ1ZDV6
layout/base/PresShell.cpp
layout/base/nsIPresShell.h
layout/base/tests/test_scroll_selection_into_view.html
--- a/layout/base/PresShell.cpp
+++ b/layout/base/PresShell.cpp
@@ -3385,37 +3385,29 @@ ComputeWhereToScroll(int16_t aWhereToScr
                      nscoord aOriginalCoord,
                      nscoord aRectMin,
                      nscoord aRectMax,
                      nscoord aViewMin,
                      nscoord aViewMax,
                      nscoord* aRangeMin,
                      nscoord* aRangeMax) {
   nscoord resultCoord = aOriginalCoord;
-  // Allow the scroll operation to land anywhere that
-  // makes the whole rectangle visible.
+  nscoord scrollPortLength = aViewMax - aViewMin;
   if (nsIPresShell::SCROLL_MINIMUM == aWhereToScroll) {
-    if (aRectMin < aViewMin) {
-      // Scroll up so the frame's top edge is visible
-      resultCoord = aRectMin;
-    } else if (aRectMax > aViewMax) {
-      // Scroll down so the frame's bottom edge is visible. Make sure the
-      // frame's top edge is still visible
-      resultCoord = aOriginalCoord + aRectMax - aViewMax;
-      if (resultCoord > aRectMin) {
-        resultCoord = aRectMin;
-      }
-    }
+    // Scroll the minimum amount necessary to show as much as possible of the frame.
+    // If the frame is too large, don't hide any initially visible part of it.
+    nscoord min = std::min(aRectMin, aRectMax - scrollPortLength);
+    nscoord max = std::max(aRectMin, aRectMax - scrollPortLength);
+    resultCoord = std::min(std::max(aOriginalCoord, min), max);
   } else {
     nscoord frameAlignCoord =
       NSToCoordRound(aRectMin + (aRectMax - aRectMin) * (aWhereToScroll / 100.0f));
-    resultCoord =  NSToCoordRound(frameAlignCoord - (aViewMax - aViewMin) * (
+    resultCoord =  NSToCoordRound(frameAlignCoord - scrollPortLength * (
                                   aWhereToScroll / 100.0f));
   }
-  nscoord scrollPortLength = aViewMax - aViewMin;
   // Force the scroll range to extend to include resultCoord.
   *aRangeMin = std::min(resultCoord, aRectMax - scrollPortLength);
   *aRangeMax = std::max(resultCoord, aRectMin);
   return resultCoord;
 }
 
 /**
  * This function takes a scrollable frame, a rect in the coordinate system
--- a/layout/base/nsIPresShell.h
+++ b/layout/base/nsIPresShell.h
@@ -680,19 +680,19 @@ public:
   };
   typedef struct ScrollAxis {
     int16_t mWhereToScroll;
     WhenToScroll mWhenToScroll : 8;
     bool mOnlyIfPerceivedScrollableDirection : 1;
   /**
    * @param aWhere: Either a percentage or a special value.
    *                nsIPresShell defines:
-   *                * (Default) SCROLL_MINIMUM = -1: The visible area is
-   *                scrolled to show the entire frame. If the frame is too
-   *                large, the top and left edges are given precedence.
+   *                * (Default) SCROLL_MINIMUM = -1: The visible area is scrolled
+   *                the minimum amount to show as much as possible of the frame.
+   *                This won't hide any initially visible part of the frame.
    *                * SCROLL_TOP = 0: The frame's upper edge is aligned with the
    *                top edge of the visible area.
    *                * SCROLL_BOTTOM = 100: The frame's bottom edge is aligned
    *                with the bottom edge of the visible area.
    *                * SCROLL_LEFT = 0: The frame's left edge is aligned with the
    *                left edge of the visible area.
    *                * SCROLL_RIGHT = 100: The frame's right edge is aligned with
    *                the right edge of the visible area.
--- a/layout/base/tests/test_scroll_selection_into_view.html
+++ b/layout/base/tests/test_scroll_selection_into_view.html
@@ -51,29 +51,29 @@ function doTest() {
   testCollapsed("2", -1, 500, 500);
 
   // Test scrolling an element larger than the scrollport
   testCollapsed("3", 0, 0, 400);
   testCollapsed("3", 100, 0, 500);
   testCollapsed("3", -1, 0, 400);
   testCollapsed("3", 0, 1000, 400);
   testCollapsed("3", 100, 1000, 500);
-  // If the element can't be completely visible, we make the top edge
-  // visible.
-  testCollapsed("3", -1, 1000, 400);
+  // If the element can't be completely visible, show as much as possible,
+  // and don't hide anything which was initially visible.
+  testCollapsed("3", -1, 1000, 500);
 
   // Test scrolling an element larger than the scrollport
   testCollapsed("4", 0, 0, 400);
   testCollapsed("4", 100, 0, 500);
   testCollapsed("4", -1, 0, 400);
   testCollapsed("4", 0, 1000, 400);
   testCollapsed("4", 100, 1000, 500);
-  // If the element can't be completely visible, we make the top edge
-  // visible.
-  testCollapsed("4", -1, 1000, 400);
+  // If the element can't be completely visible, show as much as possible,
+  // and don't hide anything which was initially visible.
+  testCollapsed("4", -1, 1000, 500);
 
   // Test that scrolling a translated element into view takes
   // account of the transform.
   testCollapsed("5", 0, 0, 400);
 
   // Test that scrolling a scaled element into view takes
   // account of the transform.
   testCollapsed("6", 0, 0, 150);