Overflow regions with scroll snap points are not reliably rubber banding
authorbfulgham@apple.com <bfulgham@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 18 Jun 2015 00:32:33 +0000 (00:32 +0000)
committerbfulgham@apple.com <bfulgham@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 18 Jun 2015 00:32:33 +0000 (00:32 +0000)
https://bugs.webkit.org/show_bug.cgi?id=142522
<rdar://problem/20100726>

Reviewed by Darin Adler.

Source/WebCore:

When computing the target scroll destination, update the nearest snap point index
and other bookkeeping, but keep the original gesture target if it would have taken
us beyond either limit of the scroll container.

* page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.h:
* page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.mm:
(WebCore::ScrollingTreeFrameScrollingNodeMac::scrollExtents): Add new method
to support client API.
* platform/ScrollAnimator.cpp:
(WebCore::ScrollAnimator::scrollExtents): Add new method to support client API.
* platform/ScrollAnimator.h:
* platform/cocoa/ScrollController.h:
(WebCore::ScrollControllerClient::scrollExtents): Added new pure virtual method to API.
* platform/cocoa/ScrollController.mm:
(WebCore::ScrollController::beginScrollSnapAnimation): Hold onto original user gesture
target, and use that instead of our nearest snap point if the gesture takes us past
either extreme of the scroll container.

Source/WebKit2:

Make sure we don't block rubberbanding behavior when a scroll gesture should take us past
the end of the scroll container.

* UIProcess/Scrolling/ios/ScrollingTreeOverflowScrollingNodeIOS.mm:
(-[WKOverflowScrollViewDelegate scrollViewWillEndDragging:withVelocity:targetContentOffset:]): Don't adjust
target point if we were going to scroll past the edges of the scroll container.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@185681 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.h
Source/WebCore/page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.mm
Source/WebCore/platform/ScrollAnimator.cpp
Source/WebCore/platform/ScrollAnimator.h
Source/WebCore/platform/cocoa/ScrollController.h
Source/WebCore/platform/cocoa/ScrollController.mm
Source/WebKit2/ChangeLog
Source/WebKit2/UIProcess/Scrolling/ios/ScrollingTreeOverflowScrollingNodeIOS.mm

index ea16da6..4a30c75 100644 (file)
@@ -1,3 +1,29 @@
+2015-06-17  Brent Fulgham  <bfulgham@apple.com>
+
+        Overflow regions with scroll snap points are not reliably rubber banding
+        https://bugs.webkit.org/show_bug.cgi?id=142522
+        <rdar://problem/20100726>
+
+        Reviewed by Darin Adler.
+
+        When computing the target scroll destination, update the nearest snap point index
+        and other bookkeeping, but keep the original gesture target if it would have taken
+        us beyond either limit of the scroll container.
+
+        * page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.h:
+        * page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.mm:
+        (WebCore::ScrollingTreeFrameScrollingNodeMac::scrollExtents): Add new method
+        to support client API.
+        * platform/ScrollAnimator.cpp:
+        (WebCore::ScrollAnimator::scrollExtents): Add new method to support client API.
+        * platform/ScrollAnimator.h:
+        * platform/cocoa/ScrollController.h:
+        (WebCore::ScrollControllerClient::scrollExtents): Added new pure virtual method to API.
+        * platform/cocoa/ScrollController.mm:
+        (WebCore::ScrollController::beginScrollSnapAnimation): Hold onto original user gesture
+        target, and use that instead of our nearest snap point if the gesture takes us past
+        either extreme of the scroll container.
+
 2015-06-17  Tim Horton  <timothy_horton@apple.com>
 
         Swipe gesture can get stuck, preventing scrolling and other gestures
index b9d52a0..760f6cf 100644 (file)
@@ -88,6 +88,7 @@ private:
     float pageScaleFactor() const override;
     void startScrollSnapTimer(ScrollEventAxis) override;
     void stopScrollSnapTimer(ScrollEventAxis) override;
+    LayoutSize scrollExtent() const override;
 #endif
 
     void logExposedUnfilledArea();
index dd85555..08630d8 100644 (file)
@@ -29,6 +29,7 @@
 #if ENABLE(ASYNC_SCROLLING) && PLATFORM(MAC)
 
 #import "FrameView.h"
+#import "LayoutSize.h"
 #import "Logging.h"
 #import "NSScrollerImpDetails.h"
 #import "PlatformWheelEvent.h"
@@ -587,6 +588,11 @@ void ScrollingTreeFrameScrollingNodeMac::stopScrollSnapTimer(ScrollEventAxis axi
     if (!m_scrollController.hasActiveScrollSnapTimerForAxis(otherAxis))
         scrollingTree().setMainFrameIsScrollSnapping(false);
 }
+    
+LayoutSize ScrollingTreeFrameScrollingNodeMac::scrollExtent() const
+{
+    return LayoutSize(totalContentsSize());
+}
 #endif
 
 void ScrollingTreeFrameScrollingNodeMac::deferTestsForReason(WheelEventTestTrigger::ScrollableAreaIdentifier identifier, WheelEventTestTrigger::DeferTestTriggerReason reason) const
index c9dac12..5a9d813 100644 (file)
@@ -33,6 +33,7 @@
 #include "ScrollAnimator.h"
 
 #include "FloatPoint.h"
+#include "LayoutSize.h"
 #include "PlatformWheelEvent.h"
 #include "ScrollableArea.h"
 #include <algorithm>
@@ -213,6 +214,11 @@ void ScrollAnimator::immediateScrollOnAxis(ScrollEventAxis axis, float delta)
     else
         scrollToOffsetWithoutAnimation(FloatPoint(currentPosition.x(), currentPosition.y() + delta));
 }
+
+LayoutSize ScrollAnimator::scrollExtent() const
+{
+    return m_scrollableArea.contentsSize();
+}
 #endif
 
 #if (ENABLE(CSS_SCROLL_SNAP) || ENABLE(RUBBER_BANDING)) && PLATFORM(MAC)
index 032d3ea..d4b1bc9 100644 (file)
@@ -135,6 +135,7 @@ public:
     void immediateScrollOnAxis(ScrollEventAxis, float delta) override;
     bool activeScrollSnapIndexDidChange() const;
     unsigned activeScrollSnapIndexForAxis(ScrollEventAxis) const;
+    LayoutSize scrollExtent() const override;
 #endif
 
 protected:
index e5cf421..452c728 100644 (file)
@@ -41,6 +41,7 @@
 
 namespace WebCore {
 
+class LayoutSize;
 class PlatformWheelEvent;
 class ScrollableArea;
 class WheelEventTestTrigger;
@@ -102,6 +103,8 @@ public:
     {
         return 0;
     }
+
+    virtual LayoutSize scrollExtent() const = 0;
 #endif
 };
 
index 6feffa9..e592fbe 100644 (file)
@@ -26,6 +26,7 @@
 #include "config.h"
 #include "ScrollController.h"
 
+#include "LayoutSize.h"
 #include "PlatformWheelEvent.h"
 #include "WebCoreSystemInterface.h"
 #include "WheelEventTestTrigger.h"
@@ -752,18 +753,24 @@ void ScrollController::beginScrollSnapAnimation(ScrollEventAxis axis, ScrollSnap
 
     LayoutUnit offset = m_client.scrollOffsetOnAxis(axis);
     float initialWheelDelta = newState == ScrollSnapState::Gliding ? snapState.averageInitialWheelDelta() : 0;
-    LayoutUnit projectedScrollDestination = newState == ScrollSnapState::Gliding ? snapState.m_beginTrackingWheelDeltaOffset + LayoutUnit(projectedInertialScrollDistance(initialWheelDelta)) : offset;
+    LayoutUnit scaledProjectedScrollDestination = newState == ScrollSnapState::Gliding ? snapState.m_beginTrackingWheelDeltaOffset + LayoutUnit(projectedInertialScrollDistance(initialWheelDelta)) : offset;
     if (snapState.m_snapOffsets.isEmpty())
         return;
 
     float scaleFactor = m_client.pageScaleFactor();
+    LayoutUnit originalProjectedScrollDestination = scaledProjectedScrollDestination / scaleFactor;
     
-    projectedScrollDestination = std::min(std::max(LayoutUnit(projectedScrollDestination / scaleFactor), snapState.m_snapOffsets.first()), snapState.m_snapOffsets.last());
+    LayoutUnit clampedScrollDestination = std::min(std::max(originalProjectedScrollDestination, snapState.m_snapOffsets.first()), snapState.m_snapOffsets.last());
     snapState.m_initialOffset = offset;
     m_activeScrollSnapIndexDidChange = false;
-    snapState.m_targetOffset = scaleFactor * closestSnapOffset<LayoutUnit, float>(snapState.m_snapOffsets, projectedScrollDestination, initialWheelDelta, snapState.m_activeSnapIndex);
+    snapState.m_targetOffset = scaleFactor * closestSnapOffset<LayoutUnit, float>(snapState.m_snapOffsets, clampedScrollDestination, initialWheelDelta, snapState.m_activeSnapIndex);
     if (snapState.m_initialOffset == snapState.m_targetOffset)
         return;
+
+    LayoutUnit scrollExtent = (axis == ScrollEventAxis::Horizontal) ? m_client.scrollExtent().width() : m_client.scrollExtent().height();
+    LayoutUnit projectedScrollDestination = clampedScrollDestination;
+    if (originalProjectedScrollDestination < 0 || originalProjectedScrollDestination > scrollExtent)
+        projectedScrollDestination = originalProjectedScrollDestination;
     
     m_activeScrollSnapIndexDidChange = true;
     snapState.m_currentState = newState;
index 43332be..e3903ff 100644 (file)
@@ -1,3 +1,18 @@
+2015-06-17  Brent Fulgham  <bfulgham@apple.com>
+
+        Overflow regions with scroll snap points are not reliably rubber banding
+        https://bugs.webkit.org/show_bug.cgi?id=142522
+        <rdar://problem/20100726>
+
+        Reviewed by Darin Adler.
+
+        Make sure we don't block rubberbanding behavior when a scroll gesture should take us past
+        the end of the scroll container.
+
+        * UIProcess/Scrolling/ios/ScrollingTreeOverflowScrollingNodeIOS.mm:
+        (-[WKOverflowScrollViewDelegate scrollViewWillEndDragging:withVelocity:targetContentOffset:]): Don't adjust
+        target point if we were going to scroll past the edges of the scroll container.
+
 2015-06-17  Anders Carlsson  <andersca@apple.com>
 
         Would like a way, in the API, to get notified about a web process crash
index 773b18d..0c24bfc 100644 (file)
@@ -80,11 +80,14 @@ using namespace WebCore;
 #if ENABLE(CSS_SCROLL_SNAP)
 - (void)scrollViewWillEndDragging:(UIScrollView *)scrollView withVelocity:(CGPoint)velocity targetContentOffset:(inout CGPoint *)targetContentOffset
 {
+    CGFloat horizontalTarget = targetContentOffset->x;
+    CGFloat verticalTarget = targetContentOffset->y;
+
     unsigned ignore;
-    if (!_scrollingTreeNode->horizontalSnapOffsets().isEmpty())
-        targetContentOffset->x = closestSnapOffset<float, CGFloat>(_scrollingTreeNode->horizontalSnapOffsets(), targetContentOffset->x, velocity.x, ignore);
-    if (!_scrollingTreeNode->verticalSnapOffsets().isEmpty())
-        targetContentOffset->y = closestSnapOffset<float, CGFloat>(_scrollingTreeNode->verticalSnapOffsets(), targetContentOffset->y, velocity.y, ignore);
+    if (!_scrollingTreeNode->horizontalSnapOffsets().isEmpty() && horizontalTarget >= 0 && horizontalTarget <= scrollView.contentSize.width)
+        targetContentOffset->x = closestSnapOffset<float, CGFloat>(_scrollingTreeNode->horizontalSnapOffsets(), horizontalTarget, velocity.x, ignore);
+    if (!_scrollingTreeNode->verticalSnapOffsets().isEmpty() && verticalTarget >= 0 && verticalTarget <= scrollView.contentSize.height)
+        targetContentOffset->y = closestSnapOffset<float, CGFloat>(_scrollingTreeNode->verticalSnapOffsets(), verticalTarget, velocity.y, ignore);
 }
 #endif