Changeset 215075 in webkit


Ignore:
Timestamp:
Apr 6, 2017 5:38:42 PM (7 years ago)
Author:
Wenson Hsieh
Message:

Scroll offset jumps after a programmatic scroll in an overflow container with scroll snapping
https://bugs.webkit.org/show_bug.cgi?id=170560
<rdar://problem/31484693>

Reviewed by Tim Horton.

Source/WebCore:

Test: css3/scroll-snap/scroll-snap-programmatic-overflow-scroll.html

Logic for maintaining the scroll snap state in ScrollController was previously removed from iOS when refactoring
ScrollController. This was done because scroll snapping on iOS is driven not by the ScrollController (as it is
on Mac) but rather by sending scroll snap offsets to the UI process and hooking into UIScrollView delegates to
handle retargeted scrolling.

However, on iOS, this ScrollController state is still important for the purposes of keeping the last active
snap point index in sync with the UI process when the scroll offset changes outside of a user gesture (i.e.
programmatic scrolling). Since the UI process does not get a chance to update the active snap offset during a
programmatic scroll, our last active snap offset state was only being updated to the last snap position that the
user manually scrolled to, making programmatic scrolling jump to this offset.

To fix this, we need to update scroll snap state on iOS within ScrollController. Also adds a new Layout test
that exercises programmatic scrolling in an overflow scrolling container on all platforms.

  • platform/cocoa/ScrollController.mm:

(WebCore::otherScrollEventAxis):
(WebCore::ScrollController::updateScrollSnapState):
(WebCore::ScrollController::updateScrollSnapPoints):

LayoutTests:

Add a test verifying that programmatically changing the scroll offset of an overflow container does not cause the
scroll offset to jump back to the last active snap position. See WebCore ChangeLog for more details.

  • css3/scroll-snap/scroll-snap-programmatic-overflow-scroll-expected.txt: Added.
  • css3/scroll-snap/scroll-snap-programmatic-overflow-scroll.html: Added.
Location:
trunk
Files:
2 added
3 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r215070 r215075  
     12017-04-06  Wenson Hsieh  <wenson_hsieh@apple.com>
     2
     3        Scroll offset jumps after a programmatic scroll in an overflow container with scroll snapping
     4        https://bugs.webkit.org/show_bug.cgi?id=170560
     5        <rdar://problem/31484693>
     6
     7        Reviewed by Tim Horton.
     8
     9        Add a test verifying that programmatically changing the scroll offset of an overflow container does not cause the
     10        scroll offset to jump back to the last active snap position. See WebCore ChangeLog for more details.
     11
     12        * css3/scroll-snap/scroll-snap-programmatic-overflow-scroll-expected.txt: Added.
     13        * css3/scroll-snap/scroll-snap-programmatic-overflow-scroll.html: Added.
     14
    1152017-04-05  Simon Fraser  <simon.fraser@apple.com>
    216
  • trunk/Source/WebCore/ChangeLog

    r215070 r215075  
     12017-04-06  Wenson Hsieh  <wenson_hsieh@apple.com>
     2
     3        Scroll offset jumps after a programmatic scroll in an overflow container with scroll snapping
     4        https://bugs.webkit.org/show_bug.cgi?id=170560
     5        <rdar://problem/31484693>
     6
     7        Reviewed by Tim Horton.
     8
     9        Test: css3/scroll-snap/scroll-snap-programmatic-overflow-scroll.html
     10
     11        Logic for maintaining the scroll snap state in ScrollController was previously removed from iOS when refactoring
     12        ScrollController. This was done because scroll snapping on iOS is driven not by the ScrollController (as it is
     13        on Mac) but rather by sending scroll snap offsets to the UI process and hooking into UIScrollView delegates to
     14        handle retargeted scrolling.
     15
     16        However, on iOS, this ScrollController state is still important for the purposes of keeping the last active
     17        snap point index in sync with the UI process when the scroll offset changes outside of a user gesture (i.e.
     18        programmatic scrolling). Since the UI process does not get a chance to update the active snap offset during a
     19        programmatic scroll, our last active snap offset state was only being updated to the last snap position that the
     20        user manually scrolled to, making programmatic scrolling jump to this offset.
     21
     22        To fix this, we need to update scroll snap state on iOS within ScrollController. Also adds a new Layout test
     23        that exercises programmatic scrolling in an overflow scrolling container on all platforms.
     24
     25        * platform/cocoa/ScrollController.mm:
     26        (WebCore::otherScrollEventAxis):
     27        (WebCore::ScrollController::updateScrollSnapState):
     28        (WebCore::ScrollController::updateScrollSnapPoints):
     29
    1302017-04-05  Simon Fraser  <simon.fraser@apple.com>
    231
  • trunk/Source/WebCore/platform/cocoa/ScrollController.mm

    r212211 r215075  
    7676    return multiplier;
    7777}
     78#endif
    7879
    7980static ScrollEventAxis otherScrollEventAxis(ScrollEventAxis axis)
     
    8182    return axis == ScrollEventAxis::Horizontal ? ScrollEventAxis::Vertical : ScrollEventAxis::Horizontal;
    8283}
    83 #endif
    8484
    8585ScrollController::ScrollController(ScrollControllerClient& client)
     
    556556}
    557557
     558void ScrollController::startScrollSnapTimer()
     559{
     560    if (m_scrollSnapTimer.isActive())
     561        return;
     562
     563    startDeferringTestsDueToScrollSnapping();
     564    m_client.startScrollSnapTimer();
     565    m_scrollSnapTimer.startRepeating(1.0 / 60.0);
     566}
     567
     568void ScrollController::stopScrollSnapTimer()
     569{
     570    if (!m_scrollSnapTimer.isActive())
     571        return;
     572
     573    stopDeferringTestsDueToScrollSnapping();
     574    m_client.stopScrollSnapTimer();
     575    m_scrollSnapTimer.stop();
     576}
     577
     578void ScrollController::scrollSnapTimerFired()
     579{
     580    if (!m_scrollSnapState) {
     581        ASSERT_NOT_REACHED();
     582        return;
     583    }
     584
     585    bool isAnimationComplete;
     586    auto animationOffset = m_scrollSnapState->currentAnimatedScrollOffset(isAnimationComplete);
     587    auto currentOffset = m_client.scrollOffset();
     588    m_client.immediateScrollByWithoutContentEdgeConstraints(FloatSize(animationOffset.x() - currentOffset.x(), animationOffset.y() - currentOffset.y()));
     589    if (isAnimationComplete) {
     590        m_scrollSnapState->transitionToDestinationReachedState();
     591        stopScrollSnapTimer();
     592    }
     593}
     594#endif // PLATFORM(MAC)
     595
    558596void ScrollController::updateScrollSnapState(const ScrollableArea& scrollableArea)
    559597{
     
    590628}
    591629
    592 void ScrollController::startScrollSnapTimer()
    593 {
    594     if (m_scrollSnapTimer.isActive())
    595         return;
    596 
    597     startDeferringTestsDueToScrollSnapping();
    598     m_client.startScrollSnapTimer();
    599     m_scrollSnapTimer.startRepeating(1.0 / 60.0);
    600 }
    601 
    602 void ScrollController::stopScrollSnapTimer()
    603 {
    604     if (!m_scrollSnapTimer.isActive())
    605         return;
    606 
    607     stopDeferringTestsDueToScrollSnapping();
    608     m_client.stopScrollSnapTimer();
    609     m_scrollSnapTimer.stop();
    610 }
    611 
    612 void ScrollController::scrollSnapTimerFired()
    613 {
    614     if (!m_scrollSnapState) {
    615         ASSERT_NOT_REACHED();
    616         return;
    617     }
    618 
    619     bool isAnimationComplete;
    620     auto animationOffset = m_scrollSnapState->currentAnimatedScrollOffset(isAnimationComplete);
    621     auto currentOffset = m_client.scrollOffset();
    622     m_client.immediateScrollByWithoutContentEdgeConstraints(FloatSize(animationOffset.x() - currentOffset.x(), animationOffset.y() - currentOffset.y()));
    623     if (isAnimationComplete) {
    624         m_scrollSnapState->transitionToDestinationReachedState();
    625         stopScrollSnapTimer();
    626     }
    627 }
    628 #else
    629 void ScrollController::updateScrollSnapState(const ScrollableArea&)
    630 {
    631 }
    632 #endif // PLATFORM(MAC)
    633 
    634630unsigned ScrollController::activeScrollSnapIndexForAxis(ScrollEventAxis axis) const
    635631{
Note: See TracChangeset for help on using the changeset viewer.