Bug 185034 – getBoundingClientRect() x and y are divided by CSS zoom

• Status: NEW
• Component: DOM

[Juan Pablo Ugarte]

Created attachment 338891 [details]
Small test case

Setting CSS zoom property breaks getBoundingClientRect()

Both X and Y are divided by zoom instead of width and height which are unchanged.

[Juan Pablo Ugarte]

Created attachment 339011 [details]
Proposed patch

I think the problem is in Document::convertAbsoluteToClientRect()
If I remove style.effectiveZoom() from frameView.absoluteToDocumentRect() call everything seems to be right, the position is in CSS pixels and widgth/height are adjusted by zoom.

[Simon Fraser (smfr)]

I think we have tests that cover this already. Any patch should come with new tests.

[Juan Pablo Ugarte]

Created attachment 339432 [details]
Proposed patch v2

I could not test the new patch because I am having trouble building master, lots of weird errors like

Source/WTF/wtf/Expected.h:576:101: error: binding reference of type ‘std::experimental::fundamentals_v3::unexpected_t&’ to ‘const std::experimental::fundamentals_v3::unexpected_t’ discards qualifiers
 __EXPECTED_INLINE_VARIABLE constexpr std::experimental::unexpected_t& unexpect = std::experimental::unexpect;


In any case I wanted to show it to you to make sure LayoutTests/fast/zooming/client-rects-with-css-and-page-zoom.html was the test case you had in mind

[Juan Pablo Ugarte]

BTW it fails in some platforms because of an unused parameter

./dom/Document.cpp:6837:80: error: unused parameter 'style' [-Werror,-Wunused-parameter]
void Document::convertAbsoluteToClientRect(FloatRect& rect, const RenderStyle& style)

[Juan Pablo Ugarte]

Created attachment 339444 [details]
Proposed patch v3

Ignore unused parameter

[EWS Watchlist]

Comment on attachment 339444 [details]
Proposed patch v3

Attachment 339444 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/7554310

New failing tests:
fast/transforms/bounding-rect-zoom.html
svg/zoom/page/zoom-zoom-coords.xhtml
fast/sub-pixel/layout-boxes-with-zoom.html
fast/block/float/floats-with-margin-should-not-wrap.html
media/video-controls-zoomed.html
fast/sub-pixel/table-rows-have-stable-height.html
fast/zooming/client-rects-with-css-and-page-zoom.html
css3/zoom-coords.xhtml

[EWS Watchlist]

Created attachment 339451 [details]
Archive of layout-test-results from ews102 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-sierra  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 339444 [details]
Proposed patch v3

Attachment 339444 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/7554722

New failing tests:
fast/transforms/bounding-rect-zoom.html
svg/zoom/page/zoom-zoom-coords.xhtml
fast/sub-pixel/layout-boxes-with-zoom.html
fast/mediastream/delayed-permission-allowed.html
fast/block/float/floats-with-margin-should-not-wrap.html
media/video-controls-zoomed.html
fast/sub-pixel/table-rows-have-stable-height.html
fast/zooming/client-rects-with-css-and-page-zoom.html
css3/zoom-coords.xhtml

[EWS Watchlist]

Created attachment 339464 [details]
Archive of layout-test-results from ews106 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 339444 [details]
Proposed patch v3

Attachment 339444 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/7554697

New failing tests:
fast/transforms/bounding-rect-zoom.html
fast/block/float/floats-with-margin-should-not-wrap.html
fast/zooming/client-rects-with-css-and-page-zoom.html
fast/sub-pixel/table-rows-have-stable-height.html
fast/sub-pixel/layout-boxes-with-zoom.html

[EWS Watchlist]

Created attachment 339466 [details]
Archive of layout-test-results from ews126 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews126  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.4

[EWS Watchlist]

Comment on attachment 339444 [details]
Proposed patch v3

Attachment 339444 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/7556081

New failing tests:
fast/transforms/bounding-rect-zoom.html
svg/zoom/page/zoom-zoom-coords.xhtml
fast/sub-pixel/layout-boxes-with-zoom.html
fast/block/float/floats-with-margin-should-not-wrap.html
media/video-controls-zoomed.html
fast/sub-pixel/table-rows-have-stable-height.html
fast/zooming/client-rects-with-css-and-page-zoom.html
css3/zoom-coords.xhtml

[EWS Watchlist]

Created attachment 339476 [details]
Archive of layout-test-results from ews112 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112  Port: mac-sierra  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 339444 [details]
Proposed patch v3

Attachment 339444 [details] did not pass win-ews (win):
Output: http://webkit-queues.webkit.org/results/7558777

New failing tests:
fast/block/float/floats-with-margin-should-not-wrap.html
svg/zoom/page/zoom-zoom-coords.xhtml
fast/zooming/client-rects-with-css-and-page-zoom.html
css3/zoom-coords.xhtml

[EWS Watchlist]

Created attachment 339502 [details]
Archive of layout-test-results from ews202 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews202  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

[Konstantin Tokarev]

Comment on attachment 339444 [details]
Proposed patch v3

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

> Source/WebCore/dom/Document.cpp:6843
> +    (void)style;

You should remove style argument, or at least use UNUSED_PARAM() macro

[Juan Pablo Ugarte]

(In reply to Konstantin Tokarev from comment #16)
> Comment on attachment 339444 [details]
> Proposed patch v3
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=339444&action=review
> 
> > Source/WebCore/dom/Document.cpp:6843
> > +    (void)style;
> 
> You should remove style argument, or at least use UNUSED_PARAM() macro

Thanks for the tip, anyways this patch is not the proper way to fix this issue since it also breaks other tests which means it has to be fixed in other place if there is an issue in the first place.

zoom property is not standard so who knows how it should behave regarding getBoundingClientRect()

IMHO at least left and top property should not be affected by it but 
https://trac.webkit.org/browser/webkit/trunk/LayoutTests/css3/zoom-coords.xhtml#L80 seems to differ

so are those test cases wrong?

Given that the CSS zoom property is now standard (https://github.com/w3c/csswg-drafts/pull/9699), can this issue be looked at once more, it is still present in Safari 

Thank you!

[Radar WebKit Bug Importer]

<rdar://problem/139289244>