http://code.google.com/p/chromium/issues/detail?id=29080
Created attachment 44092 [details] fix
style-queue ran check-webkit-style on attachment 44092 [details] without any errors.
[I am not a reviewer] This looks good to me, but perhaps it would benefit from a comment on the counterintuitive notion that WebKit expect +X to mean to the left.
Comment on attachment 44092 [details] fix r=me, the windows version has this comment: // Windows is <- -/+ ->, WebKit <- +/- ->.
Comment on attachment 44092 [details] fix I'm not sure why WebKit is reversed that way. Probably due to: wkGetWheelEventDeltas(event, &m_deltaX, &m_deltaY, &continuous); http://trac.webkit.org/browser/trunk/WebCore/platform/mac/WheelEventMac.mm#L46 I'll CC Hyatt, as he wrote that code.
Comment on attachment 44092 [details] fix Clearing flags on attachment: 44092 Committed r51614: <http://trac.webkit.org/changeset/51614>
All reviewed patches have been landed. Closing bug.
It is not sufficient to point to a website for a manual test. Manual tests should be in WebCore/manual-tests So the website version of the test, should be rewritten to test exactly what this is fixing, and then be landed in the tree. Further there is no reason that this should not be tested automatically. The absence of existing automatic tests is not justification to not write an automated test, if necessary your patch should include additions to DRT to allow wheel event generation (i'm really surprised it doesn't already). This is how DRT came to provide the functionality it does: the first time anyone encounters a untested area DRT is extended to make such testing possible.
With respect to testing this automatically and DRT: DRT (or for chromium, Test Shell/EventSendingController) generates its own WebInputEvent objects, it does not start with a platform object (for gtk, GdkEvent*) and use this code to convert it, hence layout tests do not and can not cover this code without major structural changes. On the other hand, infrastructure to test event conversion code is largely in place in the Chromium tree in the form of the Automation/UIControls infrastructure, which is used in our automated test suite (interactive tests, browser tests, UI tests, etc.). When I said "manual test", I simply meant "this is how I know the patch is correct", I didn't intend that this should be added to WebCore/manual-tests. In light of the fact that it can be automatically tested (within Chromium), I think it should not be a manual test.
I think the distinction being lost here is that this code appears only to affect the conversion between OS event objects and WebCore event objects. It does not affect event objects which are synthesized wholly within WebCore, and thus untestable. At some point we should either c++ unit test these sort of platform -> webcore event conversion routines or add layout tests for them. Unfortunately we don't yet have a working c++ unit testing framework. Bug 21010.
yes, the c++ unit testing framework would be ideal
This seems to be an old Chromium bug.