Bug 145216 - Scroll-snap points needs to be updated during programmatic scrolls
Summary: Scroll-snap points needs to be updated during programmatic scrolls
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Brent Fulgham
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2015-05-20 11:38 PDT by Brent Fulgham
Modified: 2015-06-18 09:59 PDT (History)
4 users (show)

See Also:


Attachments
Patch (7.68 KB, patch)
2015-05-20 18:59 PDT, Brent Fulgham
dino: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brent Fulgham 2015-05-20 11:38:48 PDT
When content manually scrolls the view (e.g., 'Element.scrollIntoViewIfNeeded()', 'Window.scrollTo'), we need to make sure that we are actively updating our scroll snap state so that we do not try to snap back to a far off snap point once the programmatic scroll completes.
Comment 1 Radar WebKit Bug Importer 2015-05-20 18:48:07 PDT
<rdar://problem/21051035>
Comment 2 Radar WebKit Bug Importer 2015-05-20 18:48:31 PDT
<rdar://problem/21051039>
Comment 3 Brent Fulgham 2015-05-20 18:59:41 PDT
Created attachment 253492 [details]
Patch
Comment 4 Brent Fulgham 2015-05-20 19:06:09 PDT
Comment on attachment 253492 [details]
Patch

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

> Source/WebCore/platform/ScrollAnimator.cpp:172
> +    updateActiveScrollSnapIndexForOffset();

I'm worried this might get hit too frequently, but in my tests it only seems to get called from "notifyPositionChanged" coming from Async scrolling notifications, so I think it's okay.
Comment 5 Dean Jackson 2015-05-21 01:33:05 PDT
Comment on attachment 253492 [details]
Patch

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

> Source/WebCore/platform/cocoa/ScrollController.mm:730
> +    (void)closestSnapOffset<LayoutUnit, float>(snapState.m_snapOffsets, clampedOffset, 0, activeIndex);

For my own information, why do you need to cast here?
Comment 6 Brent Fulgham 2015-05-21 12:26:27 PDT
Comment on attachment 253492 [details]
Patch

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

>> Source/WebCore/platform/cocoa/ScrollController.mm:730
>> +    (void)closestSnapOffset<LayoutUnit, float>(snapState.m_snapOffsets, clampedOffset, 0, activeIndex);
> 
> For my own information, why do you need to cast here?

'closestSnapOffset' returns a LayoutUnit, which I don't care about. So instead of assigning it to a temporary, and saying (void)temporary (or UNUSED_ALWAYS(blah)) I just do the cast here to tell the compiler that I don't care about the return value.
Comment 7 Brent Fulgham 2015-05-21 12:37:57 PDT
Committed r184728: <http://trac.webkit.org/changeset/184728>