Bug 146696

Summary: Scroll snapping doesn't kick in when dragging scrollbars
Product: WebKit Reporter: Wenson Hsieh <wenson_hsieh>
Component: WebCore Misc.Assignee: Martin Robinson <mrobinson>
Status: RESOLVED FIXED    
Severity: Normal CC: clopez, mrobinson, simon.fraser, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: Unspecified   
Bug Depends on: 222111    
Bug Blocks: 218115    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch none

Description Wenson Hsieh 2015-07-07 15:47:54 PDT
Scroll snapping doesn't trigger when scrolling a scroll snap container by dragging the scrollbar.
Comment 1 Martin Robinson 2021-02-12 03:00:46 PST
*** Bug 203968 has been marked as a duplicate of this bug. ***
Comment 2 Martin Robinson 2021-02-22 06:51:19 PST
Created attachment 421186 [details]
Patch
Comment 3 Martin Robinson 2021-02-23 03:29:40 PST
Created attachment 421297 [details]
Patch
Comment 4 Simon Fraser (smfr) 2021-02-26 08:38:05 PST
Comment on attachment 421297 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=421297&action=review

> Source/WebCore/platform/Scrollbar.cpp:374
> +    if (previouslyPressedPart == ThumbPart)
> +        m_scrollableArea.doPostThumbDragSnapping(m_orientation);

It's also possible to make scrollbars scroll by clicking in the track area. Doesn't that need to snap too? This feels like a too-specific place for this code.
Comment 5 Martin Robinson 2021-03-01 00:55:59 PST
(In reply to Simon Fraser (smfr) from comment #4)
> Comment on attachment 421297 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=421297&action=review
> 
> > Source/WebCore/platform/Scrollbar.cpp:374
> > +    if (previouslyPressedPart == ThumbPart)
> > +        m_scrollableArea.doPostThumbDragSnapping(m_orientation);
> 
> It's also possible to make scrollbars scroll by clicking in the track area.
> Doesn't that need to snap too? This feels like a too-specific place for this
> code.

It looks like snapping is happening properly when clicking on the gutter. There are two things that can happen when the scrollbar gutter is clicked:

 1. The theme specifies that the event should cause the thumb to center on the clicked location. In this case, there is a call to setPressedPart(ThumbPart). During the mouseUp event, this will result in snapping due to the new code this patch adds, since it triggers when the pressed part is the thumb.

 2. If the theme does not specify thumb centering, this works like a paging operation. In this case Scrollbar::autoscrollPressedPart is called. This method does paging via ScrollableArea::scroll(...), which already properly handles scroll snapping.

I've done two things in the new version of the patch:

1. I have renamed ScrollableArea::doPostDragSnapping to ScrollableArea::doPostThumbMoveSnapping since it doesn't just trigger during dragging.

2. I have added a test to verify that snapping happens during a typical gutter click.
Comment 6 Martin Robinson 2021-03-01 01:18:41 PST
Created attachment 421798 [details]
Patch
Comment 7 Martin Robinson 2021-03-01 04:21:12 PST
Created attachment 421809 [details]
Patch
Comment 8 EWS 2021-03-01 13:32:26 PST
Committed r273690: <https://commits.webkit.org/r273690>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 421809 [details].
Comment 9 Radar WebKit Bug Importer 2021-03-01 13:33:14 PST
<rdar://problem/74888135>