Bug 108647 - ScrollAnimatorMac should adopt contentAreaScrolledInDirection
Summary: ScrollAnimatorMac should adopt contentAreaScrolledInDirection
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified OS X 10.8
: P2 Normal
Assignee: Beth Dakin
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2013-02-01 09:16 PST by Beth Dakin
Modified: 2013-02-01 12:15 PST (History)
4 users (show)

See Also:

Attachments
Patch (12.79 KB, patch)
2013-02-01 09:27 PST, Beth Dakin 		no flags Details | Formatted Diff | Diff
Patch (13.00 KB, patch)
2013-02-01 10:55 PST, Beth Dakin 		andersca: review+
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Beth Dakin 2013-02-01 09:16:05 PST 
ScrollAnimatorMac should adopt contentAreaScrolledInDirection.

<rdar://problem/12434779>
Comment 1 Beth Dakin 2013-02-01 09:27:40 PST 
Created attachment 186061 [details]
Patch
Comment 2 Anders Carlsson 2013-02-01 10:28:03 PST 
Comment on attachment 186061 [details]
Patch

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

> Source/WebCore/platform/ScrollAnimator.h:97
> +    virtual void notifyContentAreaScrolled(const FloatPoint& /*delta*/) { }

I don't like the /*delta*/ - why just not name the parameter?

Also, shouldn't the delta be a FloatSize?

Also, I know that Dan hates using the term delta for this, why not offset?
Comment 3 Beth Dakin 2013-02-01 10:55:08 PST 
Created attachment 186079 [details]
Patch
Comment 4 Beth Dakin 2013-02-01 11:00:54 PST 
Thank you!

http://trac.webkit.org/changeset/141610
Comment 5 Florin Malita 2013-02-01 11:48:19 PST 
(In reply to comment #4)
> Thank you!
> 
> http://trac.webkit.org/changeset/141610

This appears to have busted the Win builds (both CR & Apple):

http://build.webkit.org/builders/Chromium%20Win%20Release/builds/53692/steps/compile-webkit/logs/stdio

Please take a look.
Comment 6 Beth Dakin 2013-02-01 11:56:33 PST 
Committed a speculative fix: http://trac.webkit.org/changeset/141620
Comment 7 Beth Dakin 2013-02-01 12:15:29 PST 
Tried again with http://trac.webkit.org/changeset/141625