Bug 24368 – DOM WheelEvent delta conversion inaccurate

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Peter Kasting]

The code in WebCore/dom/WheelEvent.cpp that calculates a wheel event's scroll delta isn't right.  It tries to convert the provided wheelDeltaX and wheelDeltaY by multiplying by 120 (the value of Windows' WHEEL_DELTA constant).  But the scroll delta is supposed to be the original value supplied by Windows, which isn't just (delta * WHEEL_DELTA), but more like (delta * WHEEL_DELTA / SPI_GETWHEELSCROLLLINES).

There are two ways to fix.  One is to write more complex code here that reverses everything done up in WebKit (where the native event was transformed into a WebCore one) to recalculate the original native value.  The other way, which is probably better, is to carry the original native delta amount along on the WebCore event, and supply it directly to this function.

I don't know how the glue code would set this value on non-Windows OSes, but I imagine it could make up something close to what happens now.

[Peter Kasting]

Created attachment 28282 [details]
Testcase to demonstrate IE vs. WebKit difference

Try running this testcase in IE 7 and Safari/Win or Chrome.  In IE, the result of wheel scrolling over the grey box should basically always be +/-120.  In Safari/Chrome, the result is scaled by your system "lines to scroll per tick" setting.

[Peter Kasting]

Created attachment 28324 [details]
patch v1

This bug turns out to be a regression since Safari 3.

[Darin Adler]

What do we need the old deltaX/Y for?

[Peter Kasting]

deltaX and deltaY and used by the ScrollView in order to decide how far to actually scroll the page.  One wheel tick should generally result in a DOM scroll amount of +/- 120, but may result in differing page scroll amounts based on what the platform convention is, whether this is a page/line scroll, etc.

[Dave Hyatt]

Comment on attachment 28324 [details]
patch v1

I'd like to see a patch that does not force any platform but Windows to write specific code.  Here would be my suggestions:

(1) Do the 120 multiplication in one place (probably in the DOM code).
(2) Instead of a "DOM" member in the wheel event, I'd have a wheel tick member or something, e.g., something that just represents the # of lines.

[Peter Kasting]

Created attachment 28488 [details]
patch v2

Addresses hyatt's review comments.

[Peter Kasting]

Created attachment 28542 [details]
patch v3

Updated after I landed the fix for bug 24502.  This now sends a shift + wheel event to the DOM as a shift + wheel event, using the correct native sign direction, as discussed with hyatt on IRC.

[Dave Hyatt]

Comment on attachment 28542 [details]
patch v3

r=me

[Peter Kasting]

Fixed in r41746.