Summary: | DOM WheelEvent delta conversion inaccurate | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Peter Kasting <pkasting> | ||||||||||
Component: | DOM | Assignee: | Peter Kasting <pkasting> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | erik.arvidsson | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | PC | ||||||||||||
OS: | All | ||||||||||||
Attachments: |
|
Description
Peter Kasting
2009-03-04 13:54:39 PST
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.
Created attachment 28324 [details]
patch v1
This bug turns out to be a regression since Safari 3.
What do we need the old deltaX/Y for? 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. 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.
Created attachment 28488 [details]
patch v2
Addresses hyatt's review comments.
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. Comment on attachment 28542 [details]
patch v3
r=me
|