REGRESSION(142590): Scroll-snap points are improperly snapping to earlier index values
authorbfulgham@apple.com <bfulgham@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 18 May 2015 22:01:23 +0000 (22:01 +0000)
committerbfulgham@apple.com <bfulgham@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 18 May 2015 22:01:23 +0000 (22:01 +0000)
https://bugs.webkit.org/show_bug.cgi?id=145140
<rdar://problem/21006738>

Reviewed by Beth Dakin.

The new "nearestActiveSnapPoint" logic is firing while scroll snap animations are running. We need
to add an "isScrollSnapInProgress" predicate, much like the existing "isRubberBandInProgress" to avoid
certain "fix-up" logic that we don't want running while we are in the process of moving to a new position.

* platform/ScrollAnimator.h:
(WebCore::ScrollAnimator::ScrollAnimator::isScrollSnapInProgress): Added.
* platform/ScrollableArea.cpp:
(WebCore::ScrollableArea::updateScrollSnapState): If we are in the midst of a scroll snap operation,
do not attempt to reset position to the current active snap point.
* platform/cocoa/ScrollController.h:
* platform/cocoa/ScrollController.mm:
(WebCore::ScrollController::isScrollSnapInProgress): Added.
* platform/mac/ScrollAnimatorMac.h:
* platform/mac/ScrollAnimatorMac.mm:
(WebCore::ScrollAnimatorMac::isScrollSnapInProgress): Added.

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

Source/WebCore/ChangeLog
Source/WebCore/platform/ScrollAnimator.h
Source/WebCore/platform/ScrollableArea.cpp
Source/WebCore/platform/cocoa/ScrollController.h
Source/WebCore/platform/cocoa/ScrollController.mm
Source/WebCore/platform/mac/ScrollAnimatorMac.h
Source/WebCore/platform/mac/ScrollAnimatorMac.mm

index 5c4795e..61f25aa 100644 (file)
@@ -1,3 +1,27 @@
+2015-05-18  Brent Fulgham  <bfulgham@apple.com>
+
+        REGRESSION(142590): Scroll-snap points are improperly snapping to earlier index values
+        https://bugs.webkit.org/show_bug.cgi?id=145140
+        <rdar://problem/21006738>
+
+        Reviewed by Beth Dakin.
+
+        The new "nearestActiveSnapPoint" logic is firing while scroll snap animations are running. We need
+        to add an "isScrollSnapInProgress" predicate, much like the existing "isRubberBandInProgress" to avoid
+        certain "fix-up" logic that we don't want running while we are in the process of moving to a new position.
+
+        * platform/ScrollAnimator.h:
+        (WebCore::ScrollAnimator::ScrollAnimator::isScrollSnapInProgress): Added.
+        * platform/ScrollableArea.cpp:
+        (WebCore::ScrollableArea::updateScrollSnapState): If we are in the midst of a scroll snap operation,
+        do not attempt to reset position to the current active snap point.
+        * platform/cocoa/ScrollController.h:
+        * platform/cocoa/ScrollController.mm:
+        (WebCore::ScrollController::isScrollSnapInProgress): Added.
+        * platform/mac/ScrollAnimatorMac.h:
+        * platform/mac/ScrollAnimatorMac.mm:
+        (WebCore::ScrollAnimatorMac::isScrollSnapInProgress): Added.
+
 2015-05-17  Manuel Rego Casasnovas  <rego@igalia.com>
 
         [CSS Grid Layout] Add scrollbar width in intrinsic logical widths computation
index 9d75f20..aec89dd 100644 (file)
@@ -118,6 +118,7 @@ public:
     virtual void notifyContentAreaScrolled(const FloatSize& delta) { UNUSED_PARAM(delta); }
 
     virtual bool isRubberBandInProgress() const { return false; }
+    virtual bool isScrollSnapInProgress() const { return false; }
 
     void setWheelEventTestTrigger(RefPtr<WheelEventTestTrigger>&& testTrigger) { m_wheelEventTestTrigger = testTrigger; }
 #if (ENABLE(CSS_SCROLL_SNAP) || ENABLE(RUBBER_BANDING)) && PLATFORM(MAC)
index f090bfb..0bc3947 100644 (file)
@@ -458,8 +458,11 @@ IntPoint ScrollableArea::nearestActiveSnapPoint(const IntPoint& currentPosition)
 void ScrollableArea::updateScrollSnapState()
 {
 #if PLATFORM(MAC)
-    if (ScrollAnimator* scrollAnimator = existingScrollAnimator())
+    if (ScrollAnimator* scrollAnimator = existingScrollAnimator()) {
         scrollAnimator->updateScrollAnimatorsAndTimers();
+        if (scrollAnimator->isScrollSnapInProgress())
+            return;
+    }
 #endif
 
     IntPoint currentPosition = scrollPosition();
index c3a3f2d..30a0b44 100644 (file)
@@ -114,6 +114,7 @@ public:
     bool handleWheelEvent(const PlatformWheelEvent&);
 
     bool isRubberBandInProgress() const;
+    bool isScrollSnapInProgress() const;
 
 #if ENABLE(CSS_SCROLL_SNAP) && PLATFORM(MAC)
     bool processWheelEventForScrollSnap(const PlatformWheelEvent&);
index 0f188e0..48230fa 100644 (file)
@@ -404,6 +404,15 @@ bool ScrollController::isRubberBandInProgress() const
     return !m_client.stretchAmount().isZero();
 }
 
+bool ScrollController::isScrollSnapInProgress() const
+{
+#if ENABLE(CSS_SCROLL_SNAP) && PLATFORM(MAC)
+    if (m_inScrollGesture || m_momentumScrollInProgress || m_horizontalScrollSnapTimer.isActive() || m_verticalScrollSnapTimer.isActive())
+        return true;
+#endif
+    return false;
+}
+
 void ScrollController::startSnapRubberbandTimer()
 {
     m_client.startSnapRubberbandTimer();
index a369a24..9dcfb18 100644 (file)
@@ -132,7 +132,8 @@ private:
 
     void immediateScrollTo(const FloatPoint&);
 
-    virtual bool isRubberBandInProgress() const override;
+    bool isRubberBandInProgress() const override;
+    bool isScrollSnapInProgress() const override;
 
 #if ENABLE(RUBBER_BANDING)
     /// ScrollControllerClient member functions.
index b45b81d..e24a406 100644 (file)
@@ -769,6 +769,15 @@ bool ScrollAnimatorMac::isRubberBandInProgress() const
 #endif
 }
 
+bool ScrollAnimatorMac::isScrollSnapInProgress() const
+{
+#if ENABLE(CSS_SCROLL_SNAP)
+    return m_scrollController.isScrollSnapInProgress();
+#else
+    return false;
+#endif
+}
+
 void ScrollAnimatorMac::immediateScrollToPointForScrollAnimation(const FloatPoint& newPosition)
 {
     ASSERT(m_scrollAnimationHelper);