[webkit-reviews] review granted: [Bug 172019] elementFromPoint() should consider x and y to be in client (layout viewport) coordinates : [Attachment 314984] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has granted Ali Juma
<ajuma at chromium.org>'s request for review:
Bug 172019: elementFromPoint() should consider x and y to be in client (layout
viewport) coordinates
https://bugs.webkit.org/show_bug.cgi?id=172019

Attachment 314984: Patch

https://bugs.webkit.org/attachment.cgi?id=314984&action=review




--- Comment #13 from Simon Fraser (smfr) <simon.fraser at apple.com> ---
Comment on attachment 314984
  --> https://bugs.webkit.org/attachment.cgi?id=314984
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=314984&action=review

> Source/WebCore/dom/TreeScope.cpp:309
> +	   FloatRect layoutViewportBounds(FloatPoint(),
view->layoutViewportRect().size());

FloatPoint() can be { }

> Source/WebCore/page/FrameView.cpp:4939
> +    p.scale(frame().frameScaleFactor());
> +    return p;

This can use p.scaled()

> Source/WebCore/page/FrameView.cpp:4946
> +    p.scale(frame().pageZoomFactor());
> +    return p;

This can use p.scaled()

> Source/WebCore/page/FrameView.h:455
> +    //    Relative to the layout viewport rect, affected by page zoom but
not by page scale. Affected by scroll origin.

I would say "Similar to client coordinates, but affected by page zoom (but not
page scale)." I think the "Affected by scroll origin" adds confusion, so remove
it.

> Source/WebCore/rendering/RenderLayer.cpp:4931
> +	       const FrameView& frameView = renderer().view().frameView();

auto& frameView =

> Source/WebCore/rendering/RenderLayer.cpp:4932
> +	       LayoutRect layoutViewportBounds(LayoutPoint(),
frameView.layoutViewportRect().size());

LayoutPoint() -> { }