[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
bugzilla-request-daemon at opendarwin.org
Fri Jun 23 21:26:34 PDT 2006
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.
More information about the webkit-reviews
mailing list