Bug 81436 - Enable scroll animation on layers
Summary: Enable scroll animation on layers
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Scott Byer
URL:
Keywords:
: 81434 (view as bug list)
Depends on: 81858
Blocks:
  Show dependency treegraph
 
Reported: 2012-03-16 17:50 PDT by Scott Byer
Modified: 2012-07-27 01:54 PDT (History)
8 users (show)

See Also:


Attachments
Patch (4.24 KB, patch)
2012-03-16 17:51 PDT, Scott Byer
no flags Details | Formatted Diff | Diff
compile fix attempt (4.23 KB, patch)
2012-03-19 15:31 PDT, Scott Byer
no flags Details | Formatted Diff | Diff
Patch (16.38 KB, patch)
2012-03-22 17:14 PDT, Scott Byer
no flags Details | Formatted Diff | Diff
Add in expectations (22.10 KB, patch)
2012-04-02 12:33 PDT, Scott Byer
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Scott Byer 2012-03-16 17:50:46 PDT
Enable scroll animation on layers
Comment 1 Scott Byer 2012-03-16 17:51:19 PDT
Created attachment 132428 [details]
Patch
Comment 2 Scott Byer 2012-03-16 17:53:04 PDT
Well, I did figure it out by EOD. Not ready for review - and definitely needs tests.
Comment 3 Alexey Proskuryakov 2012-03-16 22:12:21 PDT
*** Bug 81434 has been marked as a duplicate of this bug. ***
Comment 4 WebKit Review Bot 2012-03-17 13:25:51 PDT
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.
Comment 5 Build Bot 2012-03-17 13:46:11 PDT
Comment on attachment 132428 [details]
Patch

Attachment 132428 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/11968879
Comment 6 Scott Byer 2012-03-19 09:24:56 PDT
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.
Comment 7 James Robinson 2012-03-19 11:08:38 PDT
Comment on attachment 132428 [details]
Patch

I don't see why not!
Comment 8 Scott Byer 2012-03-19 15:31:08 PDT
Created attachment 132687 [details]
compile fix attempt

Just to see if I've got the right compile fix while I work on the tests
Comment 9 WebKit Review Bot 2012-03-19 15:33:23 PDT
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.
Comment 10 Build Bot 2012-03-19 15:57:25 PDT
Comment on attachment 132687 [details]
compile fix attempt

Attachment 132687 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/11984812
Comment 11 Scott Byer 2012-03-22 17:14:29 PDT
Created attachment 133392 [details]
Patch

Almort there; verifying compile fix, just need to figure out test expectations
Comment 12 Scott Byer 2012-04-02 12:33:06 PDT
Created attachment 135155 [details]
Add in expectations
Comment 13 Scott Byer 2012-04-03 11:16:33 PDT
James, do you have time to look at this today or tomorrow? At this point I think it's good to go.
Comment 14 James Robinson 2012-04-03 11:20:40 PDT
I'm curious what this is intended to be used on (what scroll type / platform)?
Comment 15 Scott Byer 2012-04-03 11:29:24 PDT
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).
Comment 16 Scott Byer 2012-04-06 14:32:22 PDT
James: ping?

Or, feel free to point me at another reviewer.
Comment 17 James Robinson 2012-04-06 14:38:27 PDT
(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.
Comment 18 Scott Byer 2012-04-06 15:03:36 PDT
(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.
Comment 19 Adam Barth 2012-07-27 01:54:22 PDT
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.