Bug 81436 – Enable scroll animation on layers

• Component: New Bugs
• Status: NEW

[Scott Byer]

Enable scroll animation on layers

[Scott Byer]

Created attachment 132428 [details]
Patch

[Scott Byer]

Well, I did figure it out by EOD. Not ready for review - and definitely needs tests.

[Alexey Proskuryakov]

*** Bug 81434 has been marked as a duplicate of this bug. ***

[WebKit Review Bot]

Attachment 132428 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 1 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Build Bot]

Comment on attachment 132428 [details]
Patch

Attachment 132428 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/11968879

[Scott Byer]

James, thoughts on this? Cleaning up the compile is easy, tests is a bit harder, but I'd like to take a couple of days to see if I can put some together.

[James Robinson]

Comment on attachment 132428 [details]
Patch

I don't see why not!

[Scott Byer]

Created attachment 132687 [details]
compile fix attempt

Just to see if I've got the right compile fix while I work on the tests

[WebKit Review Bot]

Attachment 132687 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 1 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Build Bot]

Comment on attachment 132687 [details]
compile fix attempt

Attachment 132687 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/11984812

[Scott Byer]

Created attachment 133392 [details]
Patch

Almort there; verifying compile fix, just need to figure out test expectations

[Scott Byer]

Created attachment 135155 [details]
Add in expectations

[Scott Byer]

James, do you have time to look at this today or tomorrow? At this point I think it's good to go.

[James Robinson]

I'm curious what this is intended to be used on (what scroll type / platform)?

[Scott Byer]

Windows and Linux, enables smooth scrolling on inner divs (e.g., GMail subject list) when the page doesn't override the default scroll action (wheel, page up/down).

[Scott Byer]

James: ping?

Or, feel free to point me at another reviewer.

[James Robinson]

(In reply to comment #15)
> Windows and Linux, enables smooth scrolling on inner divs (e.g., GMail subject list) when the page doesn't override the default scroll action (wheel, page up/down).

But smooth scrolling is disabled by default on windows and linux (right?) and we don't have any plans to turn it on in the short term (right?).  I'm OK with having this code, but I'm always nervous about coding something up that we aren't running and we don't plan to run in the near future - it's taking a risk (churn) for something that doesn't have a corresponding benefit for users.

If I'm mistaken about where this is used or if this is useful for other ports who want smooth scrolling (using this code) on other paths then I think it'd be more worthwhile to pursue this.

[Scott Byer]

(In reply to comment #17)
> (In reply to comment #15)
> > Windows and Linux, enables smooth scrolling on inner divs (e.g., GMail subject list) when the page doesn't override the default scroll action (wheel, page up/down).
> 
> But smooth scrolling is disabled by default on windows and linux (right?) and we don't have any plans to turn it on in the short term (right?).

Not on by default now, but I think the intent is to get it to the point were it would be the default. It's also useful for the GTK port, which uses ScrollAnimatorNone as well.

I think enough people turn on the flag that this code would end up being exercised.

[Adam Barth]

Comment on attachment 135155 [details]
Add in expectations

This patch has been up for review for a while.  I'm clearing the review flag, but please feel free to re-nominate it for review if it's still relevant.