Fix fast/events/continuous-platform-wheelevent-in-scrolling-div.html

• Component: DOM
• Status: RESOLVED FIXED

[Peter Kasting]

Bug 29601 contains a lot of discussion about how wheel events should be handled.  Because that bug is ultimately supposed to be about addressing issues scrolling Yahoo! Mail (and similar sites), I'm splitting this off to focus on a subset of the issues: making the [XXX]wheelevent-in-scrolling-div LayoutTests pass on both Chrome and Safari.  Unfortunately there is a lot of background to cover to do that, so I apologize for the enormous block of text below.  Summary at bottom if your eyes glaze over.

LAYOUT TEST HISTORY

After http://trac.webkit.org/changeset/60974 backed out the Safari 5 scroll event behavior, http://trac.webkit.org/changeset/60975 was landed to make fast/events/continuous-platform-wheelevent-in-scrolling-div.html not appear to be failing -- but rather than rewrite the test to match the new behavior, the expected results were simply changed to say "FAIL".

Later http://trac.webkit.org/changeset/61092 and http://trac.webkit.org/changeset/61093 fixed part of the test, but the first portion still reported FAIL.

In http://trac.webkit.org/changeset/66666 I implemented functions in Chrome's DumpRenderTree to allow it to run this test.  Because the event plumbing is different on Chrome, this implementation basically gets to do whatever it wants, so I implemented what would make the test PASS.  Of course, given the FAIL results that are checked in, this means that Chrome is now "failing" this test.  I think the right thing to do is to make Chrome and Safari agree on the behavior this test covers, rewrite the test if needed, check in a result set that is all PASSes, and enable the test for both platforms.

WHEEL EVENT BEHAVIOR

To properly fix the test, I need to know how Chrome and Safari report wheel events.  From inspection, I believe the current behavior on a Mac is:

Chrome:
For continuous devices (e.g. Mighty Mouse wheel, trackpad): Accelerated wheel tick value * 120 reported to DOM events; accelerated wheel tick value used to scroll scrollable areas
For discrete devices (e.g. Logitech mouse wheel): Non-accelerated wheel tick value * 120 reported to DOM events; accelerated wheel tick value * 4 px/tick used to scroll scrollable areas

Safari w/WebKit trunk:
For continuous devices: Accelerated wheel tick value * 3 reported to DOM events; accelerated wheel tick value used to scroll scrollable areas
For discrete devices: Accelerated wheel tick value * 120 (?) reported to DOM events; accelerated wheel tick value * 4 px/tick used to scroll scrollable areas

The Safari behaviors above are from observing testcases.  They shouldn't actually be possible given my reading of the code -- as far as I can tell, the discrete DOM value should be 3 * (value used to scroll scrollable areas) [see WebCore/platform/mac/WheelEventMac.mm lines 52-55; deltaX/Y are the values to scroll the scrollable area, while wheelDeltaX/Y are multiplied by 120 and reported to the DOM, and pixelsPerLineStep() is 40].  What I actually saw was a scroll of 4 px on one tick, with a DOM value ranging from 120 - 196 (the value would be constant until you did a bunch of accelerated scrolls up and down, then it would be different).  So I don't know exactly how the discrete case works :(

PLAN OF ACTION

Based on the above and the background on bug 29601 etc., I believe Chrome should change its continuous events to report a smaller value to the DOM (to match Safari).  It's not clear to me which side should change to match the other on discrete events; Safari's behavior is closer to its continuous device behavior, while Chrome's behavior matches other platforms (which don't report accelerated wheelDelta values regardless of how scrolling onscreen is accelerated).  So I intend to do nothing on that front for now.

Because I'm keeping the current Safari behavior, I think the test should be rewritten to expect that behavior.  So I'll change the test and its expectations.  This will also mean I need to change the Chrome DRT implementation I wrote yesterday to match this.

SUMMARY

This bug is about changing Chrome/Mac's continuous event wheelDelta calculation to match Safari/Mac; rewriting continuous-platform-wheelevent-in-scrolling-div.html to test for the current Safari behavior; changing the expected output to match; and changing Chrome's DRT implementation of continuousMouseScrollBy() to match the actual Chrome behavior.  After doing this, Chrome will be passing more layout tests and will have one fewer difference from Safari.  Unfortunately it will mean Chrome becomes subject to bug 29601, which we need to fix either by evangelism or by DOM event coalescing.

[Peter Kasting]

Created attachment 66549 [details]
patch v1.0

[Peter Kasting]

Created attachment 66552 [details]
patch v1.1

This also removes the now-unnecessary (and wrong) Chromium-specific result.

[Adam Barth]

Comment on attachment 66552 [details]
patch v1.1

Thanks for the clear explanation.  It's too bad that we'll be introducing that compat bug into Chrome, but align behavior across WebKit ports is an important step towards resolving these sorts of bugs in the future.

If I were writing this patch, I would have put some of the explanation from the bug into the ChangeLog where it will be easier for future code archeologists to find.

[WebKit Commit Bot]

Comment on attachment 66552 [details]
patch v1.1

Clearing flags on attachment: 66552

Committed r66812: <http://trac.webkit.org/changeset/66812>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[WebKit Review Bot]

http://trac.webkit.org/changeset/66812 might have broken Qt Linux Release

[Adam Barth]

Updated Qt result in http://trac.webkit.org/changeset/66814