[webkit-reviews] review granted: [Bug 24736] event.offsetX and offsetY are wrong with full page zoom : [Attachment 28838] Patch, changelog

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Simon Fraser (smfr)
<simon.fraser at apple.com>'s request for review:
Bug 24736: event.offsetX and offsetY are wrong with full page zoom
https://bugs.webkit.org/show_bug.cgi?id=24736

Attachment 28838: Patch, changelog
https://bugs.webkit.org/attachment.cgi?id=28838&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
> +	   * dom/Document.cpp:
> +	   (WebCore::Document::elementFromPoint):
> +	   Document::elementFromPoint() needs to take full-page zoom into
account.

How can the caller tell that the x and y here are supposed to be "non-zoomed"
coordinates? Is there a naming scheme for such things?

> +    float zoomFactor = frame() ? frame()->pageZoomFactor() : 1.0f;

It's becoming more and more clear that the zoom factor needs to be stored in
the Document or the FrameView, not the Frame.

Also, elementFromPoint seems like it should be a simple forwarder to a function
in FrameView or RenderView. We may be obliged to put some of these operations
in the DOM because of the public DOM API, but internally we should try to make
a bit of model/view separation.

Where are the regression tests for these changes?

I am really tempted to say review- because of the lack of regression tests, but
I'm going to say review+ counting on you adding them.