Bug 145140

Summary:

REGRESSION(142590): Scroll-snap points are improperly snapping to earlier index values

Product:

WebKit

Reporter:

Brent Fulgham <bfulgham>

Component:

Layout and Rendering

Assignee:

Brent Fulgham <bfulgham>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

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

Priority:

Keywords:

InRadar

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Bug Depends on:

Bug Blocks:

145175

Attachments:

Description	Flags
Patch	bdakin: review+

Description Brent Fulgham 2015-05-18 14:18:52 PDT

My fix in Bug 142590 for resizing events introduced a snapping logic bug that causes some scroll-snap gestures to trigger the "nearestActiveSnapPoint" logic, throwing the scroll position back to the previous snap point.

This must be fixed!

Comment 1 Brent Fulgham 2015-05-18 14:35:56 PDT

The new "nearestActiveSnapPoint" logic is firing while scroll snap animations are running. We need to add an "isScrollSnapInProgress" predicate, much like the existing "isRubberBandInProgress" to avoid certain "fix-up" logic that we don't want running while we are in the process of moving to a new position.

Comment 2 Brent Fulgham 2015-05-18 14:55:53 PDT

<rdar://problem/21006738>

Comment 3 Brent Fulgham 2015-05-18 14:56:33 PDT

Created attachment 253345 [details]
Patch

Comment 4 Beth Dakin 2015-05-18 14:58:37 PDT

Comment on attachment 253345 [details]
Patch

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

> Source/WebCore/platform/mac/ScrollAnimatorMac.h:135
> +    bool isRubberBandInProgress() const override;

Did you mean to remove the "virtual" from isRubberBandInProgress()?

Comment 5 Brent Fulgham 2015-05-18 14:59:52 PDT

(In reply to comment #4)
> Comment on attachment 253345 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=253345&action=review
> 
> > Source/WebCore/platform/mac/ScrollAnimatorMac.h:135
> > +    bool isRubberBandInProgress() const override;
> 
> Did you mean to remove the "virtual" from isRubberBandInProgress()?

I just switched to the new 'override' style guideline, where we don't also leave the 'virtual' declaration (it's implied by the 'override' keyword).

Comment 6 Brent Fulgham 2015-05-18 15:01:27 PDT

Committed r184513: <http://trac.webkit.org/changeset/184513>