Bug 209947 - Twitter feed contents overlap when changing the zoom level (caused by ResizeObserver)
Summary: Twitter feed contents overlap when changing the zoom level (caused by ResizeO...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: Safari Technology Preview
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: cathiechen
URL:
Keywords: InRadar
: 210432 (view as bug list)
Depends on:
Blocks:
 
Reported: 2020-04-02 19:57 PDT by Simon Fraser (smfr)
Modified: 2020-04-13 09:32 PDT (History)
7 users (show)

See Also:


Attachments
Screenshot (648.29 KB, image/png)
2020-04-02 20:00 PDT, Simon Fraser (smfr)
no flags Details
Patch (5.16 KB, patch)
2020-04-04 10:45 PDT, cathiechen
no flags Details | Formatted Diff | Diff
Patch (6.58 KB, patch)
2020-04-04 23:42 PDT, cathiechen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2020-04-02 19:57:48 PDT
Load https://twitter.com/explore. Zoom out with Command--. Note how elements on the timeline now overlap.

Does not happen in Chrome.
Comment 1 Simon Fraser (smfr) 2020-04-02 20:00:11 PDT
The feed is a series of absolutely positioned boxes, offset with translateY() transforms. Those offsets are computing using ResizeObserver.

ResizeObserver doesn't seem to fire callbacks when the zoom level changes, so those translateY() don't change. They do in Chrome.
Comment 2 Simon Fraser (smfr) 2020-04-02 20:00:22 PDT
Created attachment 395339 [details]
Screenshot
Comment 3 Simon Fraser (smfr) 2020-04-03 19:54:02 PDT
It's also possible that the sizes being reported by ResizeObserver fail to take zoom into account.
Comment 4 Simon Fraser (smfr) 2020-04-03 19:55:04 PDT
That might be similar to bug 209264.
Comment 5 cathiechen 2020-04-04 10:45:25 PDT
Created attachment 395457 [details]
Patch
Comment 6 Simon Fraser (smfr) 2020-04-04 11:29:13 PDT
Comment on attachment 395457 [details]
Patch

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

> Source/WebCore/ChangeLog:8
> +        Zoom in/out shouldn't affect ResizeObserver. Use adjustLayoutUnitForAbsoluteZoom instead.

Is the bug that its was triggering resizeObserver? Does Command-+ zooming affect getBoundingClientRect results?

> LayoutTests/imported/w3c/ChangeLog:9
> +        * web-platform-tests/resize-observer/resize-observer-with-zoom-expected.txt: Added.
> +        * web-platform-tests/resize-observer/resize-observer-with-zoom.html: Added.

New WPT tests should go into http/wpt so they get upstreamed.

> LayoutTests/imported/w3c/web-platform-tests/resize-observer/resize-observer-with-zoom.html:35
> +                    window.internals.setPageZoomFactor(2);

I don't think a WPT test can use window.internals.setPageZoomFactor
Comment 7 cathiechen 2020-04-04 23:41:16 PDT
Comment on attachment 395457 [details]
Patch

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

Hi Simon,
Thanks for the review:)

>> Source/WebCore/ChangeLog:8
>> +        Zoom in/out shouldn't affect ResizeObserver. Use adjustLayoutUnitForAbsoluteZoom instead.
> 
> Is the bug that its was triggering resizeObserver? Does Command-+ zooming affect getBoundingClientRect results?

Ah, it should be "don't affect ResizeObserver size" here. "ResizeObserver size" matters, not triggering notification. Command-+ doesn't affect getBoundingClientRect, and I would add a svg test case for it.

>> LayoutTests/imported/w3c/ChangeLog:9
>> +        * web-platform-tests/resize-observer/resize-observer-with-zoom.html: Added.
> 
> New WPT tests should go into http/wpt so they get upstreamed.

I see, I usually upload the test by github, will try this next time.

>> LayoutTests/imported/w3c/web-platform-tests/resize-observer/resize-observer-with-zoom.html:35
>> +                    window.internals.setPageZoomFactor(2);
> 
> I don't think a WPT test can use window.internals.setPageZoomFactor

Hmm, I can't find a similar interface in WPT, maybe we can move the test to /LayoutTests/resize-observer/resize-observer-with-zoom.html instead?
Comment 8 cathiechen 2020-04-04 23:42:42 PDT
Created attachment 395489 [details]
Patch
Comment 9 EWS 2020-04-06 10:47:43 PDT
Committed r259578: <https://trac.webkit.org/changeset/259578>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 395489 [details].
Comment 10 Radar WebKit Bug Importer 2020-04-06 10:48:15 PDT
<rdar://problem/61349043>
Comment 11 Simon Fraser (smfr) 2020-04-06 11:02:15 PDT
<rdar://problem/59944646>
Comment 12 Simon Fraser (smfr) 2020-04-13 09:32:05 PDT
*** Bug 210432 has been marked as a duplicate of this bug. ***