1715179 - allow double tap to zoom to work to zoom to content inside an oop iframe

Status: RESOLVED FIXED

(Core :: Panning and Zooming, enhancement)

Description

[Timothy Nikkel (:tnikkel)]

Follow up from bug 1713715.

Instead we zoom to the containing iframe.

Chrome and Safari do the same currently.

Comment 1

[Chris Peterson [:cpeterson]]

Doesn't need to block Fission MVP because Chrome and Safari have the same behavior.

Comment 2

[Timothy Nikkel (:tnikkel)]

As previously mentioned Chrome on macOS does not implement this, it just zooms to the boundary of any remote iframe. Surprisingly Chrome on Android does seem to implement this. Testing it is a little bit tricky though as only logged in sites get process separation by default on Chrome on Android. Based on

https://security.googleblog.com/2021/07/protecting-more-with-site-isolation.html

one can make all sites get their own process by flipping the flag at chrome://flags/#enable-site-per-process and then double check to make sure you get the process structure you expect by looking at chrome://process-internals

Comment 3

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1715179 - Add two test cases where `touch-action:manipulation` prevents double-tap-to-zoom. r?botond

Though there's no open bugs corresponding to these test cases, there were no
test cases to make sure touch-action:manipulation prevents double-tap-to-zoom
on touchscreen and doesn't prevent it on touchpad.

I've confirmed both Chrome and Safari have the same behavior on touchpad.

Comment 4

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1715179 - Add new IPC calls dedicated to double-tap-to-zoom. r?botond

Depends on D186321

Comment 5

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1715179 - Factor out ConvertRectInTopLevelCoords. r?botond

We will use the function for double-tap-to-zoom calculations on the main-thread.

Depends on D186322

Comment 6

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1715179 - Propagate VisualViewport rect and the root scrollable rect and use them on the main-thread. r?botond

Depends on D186323

Comment 7

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1715179 - Propagate transform matrix and relevant zoom values to convert CSSRect/CSSPoint in OOPIF to top level content document cooords. r?botond

Depends on D186324

Comment 8

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1715179 - Allow double-tap-to-zoom to zoom in contents inside OOP iframes. r?botond

Depends on D186325

Comment 9

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1715179 - Handle `overflow: visible` case in OOP iframes. r?botond

Depends on D186326

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b0c83c964ccf
Add two test cases where `touch-action:manipulation` prevents double-tap-to-zoom. r=botond
https://hg.mozilla.org/integration/autoland/rev/0acfcbbdf460
Propagate VisualViewport rect and the root scrollable rect and use them on the main-thread. r=botond
https://hg.mozilla.org/integration/autoland/rev/b851e0aff383
Propagate transform matrix and relevant zoom values to convert CSSRect/CSSPoint in OOPIF to top level content document cooords. r=botond
https://hg.mozilla.org/integration/autoland/rev/75c2c7f6230e
Allow double-tap-to-zoom to zoom in contents inside OOP iframes. r=botond
https://hg.mozilla.org/integration/autoland/rev/600afc2dc0ca
Handle `overflow: visible` case in OOP iframes. r=botond

Comment 11

[Serban Stanca [:SerbanS]]

Backed out for causing mochitests failures in test_group_double_tap_zoom-2.html on OSX.

• Backout link
• Push with failures
• Failure Log
• Failure line: TEST-UNEXPECTED-FAIL | gfx/layers/apz/test/mochitest/test_group_double_tap_zoom-2.html | helper_doubletap_zoom_oopif-2.html | offsetTop should be same on zoomed-out state - got -0.099609375, expected -1.3828125 epsilon: +/- 1

Comment 12

[Hiroyuki Ikezoe (:hiro)]

The failure is caused by our pixel snapping. Without ClampAndAlignWithPixels (bug 1774315) and SnapCoord (bug 1852884) the test is stable; here is a try run https://treeherder.mozilla.org/jobs?repo=try&revision=142f5a85b0fa6b20dbfbde5d014377e6dd5a5823

So I allowed 2px difference in the comparison of the visual viewport metrics here, but it still failed. Then I tried 3px difference, it still fails. I would say our pixel snapping is not what we wanted. :/

Comment 13

[Hiroyuki Ikezoe (:hiro)]

Oh wait, the failures with 2px or 3px difference are on zoomed out state. That makes sense.

Comment 14

[Hiroyuki Ikezoe (:hiro)]

This is tough. Layer pixel alignments makes visual viewport metrics diverge on different zoom levels. Here is a failure example;

TEST-UNEXPECTED-FAIL | gfx/layers/apz/test/mochitest/test_group_double_tap_zoom-2.html | helper_doubletap_zoom_oopif-2.html | offsetTop should be same on zoomed-in state - got 1808.2666015625, expected 1876.466796875 epsilon: +/- 2

The difference is over 70px! I did re-trigger the try run (which is without pixel snapping) to do double-check whether the failure is caused by pixel snapping or not. Indeed there's no failure.

I don't have any great idea to avoid the diverge here. It looks like only pageTop and offsetTop are affected, so I may ignore them in the test for now.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dc03bafe97e0
Add two test cases where `touch-action:manipulation` prevents double-tap-to-zoom. r=botond
https://hg.mozilla.org/integration/autoland/rev/8d88f8c16de6
Propagate VisualViewport rect and the root scrollable rect and use them on the main-thread. r=botond
https://hg.mozilla.org/integration/autoland/rev/14987c152160
Propagate transform matrix and relevant zoom values to convert CSSRect/CSSPoint in OOPIF to top level content document cooords. r=botond
https://hg.mozilla.org/integration/autoland/rev/2e506854844c
Allow double-tap-to-zoom to zoom in contents inside OOP iframes. r=botond
https://hg.mozilla.org/integration/autoland/rev/223106736a61
Handle `overflow: visible` case in OOP iframes. r=botond

Comment 16

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/dc03bafe97e0
https://hg.mozilla.org/mozilla-central/rev/8d88f8c16de6
https://hg.mozilla.org/mozilla-central/rev/14987c152160
https://hg.mozilla.org/mozilla-central/rev/2e506854844c
https://hg.mozilla.org/mozilla-central/rev/223106736a61