Bug 185034 - getBoundingClientRect() x and y are divided by CSS zoom
Summary: getBoundingClientRect() x and y are divided by CSS zoom
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-04-26 10:51 PDT by Juan Pablo Ugarte
Modified: 2018-05-04 14:10 PDT (History)
10 users (show)

See Also:


Attachments
Small test case (597 bytes, text/html)
2018-04-26 10:51 PDT, Juan Pablo Ugarte
no flags Details
Proposed patch (1.01 KB, patch)
2018-04-27 11:49 PDT, Juan Pablo Ugarte
no flags Details | Formatted Diff | Diff
Proposed patch v2 (3.45 KB, patch)
2018-05-03 11:12 PDT, Juan Pablo Ugarte
no flags Details | Formatted Diff | Diff
Proposed patch v3 (3.47 KB, patch)
2018-05-03 12:46 PDT, Juan Pablo Ugarte
ews: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews102 for mac-sierra (3.00 MB, application/zip)
2018-05-03 13:31 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews106 for mac-sierra-wk2 (3.94 MB, application/zip)
2018-05-03 14:09 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews126 for ios-simulator-wk2 (3.67 MB, application/zip)
2018-05-03 14:25 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews112 for mac-sierra (3.78 MB, application/zip)
2018-05-03 15:35 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews202 for win-future (12.87 MB, application/zip)
2018-05-03 18:53 PDT, Build Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Juan Pablo Ugarte 2018-04-26 10:51:53 PDT
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.
Comment 1 Juan Pablo Ugarte 2018-04-27 11:49:39 PDT
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.
Comment 2 Simon Fraser (smfr) 2018-04-29 11:37:28 PDT
I think we have tests that cover this already. Any patch should come with new tests.
Comment 3 Juan Pablo Ugarte 2018-05-03 11:12:32 PDT
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
Comment 4 Juan Pablo Ugarte 2018-05-03 12:42:00 PDT
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)
Comment 5 Juan Pablo Ugarte 2018-05-03 12:46:43 PDT
Created attachment 339444 [details]
Proposed patch v3

Ignore unused parameter
Comment 6 Build Bot 2018-05-03 13:31:36 PDT
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
Comment 7 Build Bot 2018-05-03 13:31:37 PDT
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
Comment 8 Build Bot 2018-05-03 14:09:15 PDT
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
Comment 9 Build Bot 2018-05-03 14:09:17 PDT
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
Comment 10 Build Bot 2018-05-03 14:25:36 PDT
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
Comment 11 Build Bot 2018-05-03 14:25:38 PDT
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
Comment 12 Build Bot 2018-05-03 15:35:44 PDT
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
Comment 13 Build Bot 2018-05-03 15:35:45 PDT
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
Comment 14 Build Bot 2018-05-03 18:53:42 PDT
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
Comment 15 Build Bot 2018-05-03 18:53:54 PDT
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
Comment 16 Konstantin Tokarev 2018-05-04 06:12:57 PDT
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
Comment 17 Juan Pablo Ugarte 2018-05-04 07:21:23 PDT
(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?