Bug 145216

Summary:

Scroll-snap points needs to be updated during programmatic scrolls

Product:

WebKit

Reporter:

Brent Fulgham <bfulgham>

Component:

Layout and Rendering

Assignee:

Brent Fulgham <bfulgham>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

bfulgham, dino, simon.fraser, webkit-bug-importer

Priority:

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+

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>