[webkit-reviews] review denied: [Bug 8707] event.clientX and event.clientY should be relative to the viewport, not the canvas : [Attachment 8970] Improved patch+testcase

[bugzilla-request-daemon at opendarwin.org]

Darin Adler <darin at apple.com> has denied Darin Adler <darin at apple.com>'s
request for review:
Bug 8707: event.clientX and event.clientY should be relative to the viewport,
not the canvas
http://bugzilla.opendarwin.org/show_bug.cgi?id=8707

Attachment 8970: Improved patch+testcase
http://bugzilla.opendarwin.org/attachment.cgi?id=8970&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
Almost there!

I still think that the local variables in EventTargetNode::dispatchMouseEvent
should not be named offsetX/Y. Instead I think we should have pageX/Y local
variables and do -= statements inside the if statement.

+    if (FrameView *view = document()->view()) {

Formatting wrong here -- * should be next to FrameView.

-						    document()->defaultView(),
e.globalX(), e.globalY(), pos.x(), pos.y(),
-						    e.ctrlKey(), e.altKey(),
e.shiftKey(), e.metaKey());
+						    document()->defaultView(),
e.globalX(), e.globalY(),
+						    pos.x(), pos.y(),
e.ctrlKey(), e.altKey(), e.shiftKey(), e.metaKey());

Should roll this change out -- it's just reformatting.

What guarantees that neither frame() or view() will be 0 in the WheelEvent
constructor?

Between these small issues, I think I still give this a review-, but it's about
ready to land. The only one I'm really concerned about is the WheelEvent
constructor nil-check issue.

Also, the layout test does not test the wheel event code.