Bug 146696 - Scroll snapping doesn't kick in when dragging scrollbars
Summary: Scroll snapping doesn't kick in when dragging scrollbars
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac Unspecified
: P2 Normal
Assignee: Martin Robinson
URL:
Keywords: InRadar
: 203968 (view as bug list)
Depends on: 222111
Blocks: 218115
  Show dependency treegraph
 
Reported: 2015-07-07 15:47 PDT by Wenson Hsieh
Modified: 2021-03-01 13:33 PST (History)
5 users (show)

See Also:


Attachments
Patch (15.25 KB, patch)
2021-02-22 06:51 PST, Martin Robinson
no flags Details | Formatted Diff | Diff
Patch (16.14 KB, patch)
2021-02-23 03:29 PST, Martin Robinson
no flags Details | Formatted Diff | Diff
Patch (20.81 KB, patch)
2021-03-01 01:18 PST, Martin Robinson
no flags Details | Formatted Diff | Diff
Patch (21.66 KB, patch)
2021-03-01 04:21 PST, Martin Robinson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>