Bug 145216

Summary: Scroll-snap points needs to be updated during programmatic scrolls
Product: WebKit Reporter: Brent Fulgham <bfulgham>
Component: Layout and RenderingAssignee: Brent Fulgham <bfulgham>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, dino, simon.fraser, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
See Also: https://bugs.webkit.org/show_bug.cgi?id=145318
Attachments:
Description Flags
Patch dino: review+

Brent Fulgham
Reported 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.
Attachments
Patch (7.68 KB, patch)
2015-05-20 18:59 PDT, Brent Fulgham 		dino: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2015-05-20 18:48:07 PDT
<rdar://problem/21051035>
Radar WebKit Bug Importer
Comment 2 2015-05-20 18:48:31 PDT
<rdar://problem/21051039>
Brent Fulgham
Comment 3 2015-05-20 18:59:41 PDT
Created attachment 253492 [details] Patch
Brent Fulgham
Comment 4 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.
Dean Jackson
Comment 5 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?
Brent Fulgham
Comment 6 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.
Brent Fulgham
Comment 7 2015-05-21 12:37:57 PDT
Committed r184728: <http://trac.webkit.org/changeset/184728>
Note You need to log in before you can comment on or make changes to this bug.