1649440 - Make ZoomToFocusedInput fission-friendly

Status: RESOLVED FIXED

(Core :: Panning and Zooming, task, P3)

Description

[Kartikaya Gupta (email:[e-mail])]

nsDOMWindowUtils::ZoomToFocusedInput uses APZCCallbackHelper::GetRootContentDocumentPresShellForContent which in turn uses nsPresContext::GetToplevelContentDocumentPresContext which is not fission-friendly. This means that if you have an input element inside an OOP iframe and tap on it, the "zoom to focused input" behaviour will probably not take effect.

HOWEVER, this API is only used in geckoview, which AFAIK is not going to be getting fission in the initial release, so this is not that urgent for Fission.

There are different potential solutions. One involves detecting that ZoomToFocusedInput method was called in an OOP iframe, and sending a message to the process with the root content document so that it can do the zooming action.

The other is to modify the message that gets sent to APZ so that it doesn't need to take an explicit view id for the root content document's document element but instead just asks APZ to zoom the rootmost zoomable thing.

Comment 1

[Daniel Holbert [:dholbert]]

nsDOMWindowUtils::ZoomToFocusedInput also includes a call to GetCrossDocParentFrame, which is not fission-friendly.

Comment 2

[Daniel Holbert [:dholbert]]

I've got a testcase for this behavior at https://danielholbert.com/input-iframe.html (which includes an oop iframe served from dholbert.org).

STR:

1. View that^ URL in Firefox for Android, with fission enabled.
2. Tap the first textfield (the one inside of the iframe)

EXPECTED RESULTS: When the textfield gains focus, Firefox should zoom in to feature the textfield.
ACTUAL RESULTS: No zooming happens.

This works properly (i.e. I get expected results) for the second textfield (the one that's in the outer document); but it doesn't work for the textfield in the out-of-process iframe.

Comment 3

[Makoto Kato [:m_kato]]

There is some issues before kats' comment.

• https://searchfox.org/mozilla-central/rev/dc323d0d9a3b722ca8ff0d1b2b752e5bd00dab9b/mobile/android/modules/geckoview/GeckoViewContent.jsm#97-117 doens't send GeckoView:ZoomToInput to OOP iframe now.
• When calling ZoomToFocusedInput on content root process, since focused element is null (focus is into OOP iframe), ZoomToFocusedInput does nothing.

Also, if we call ZoomToFocusedInput in OOP iframe process, AsyncPanZoomController::ZoomToRect will hit the assertion of MOZ_ASSERT(Metrics().IsRootContent()). (The reason is that kat's comment?).

Comment 4

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1649440 - Send GeckoView:ZoomToInput messages only to the focused browsing context. r?#geckoview-reviewers

Comment 5

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1649440 - Defer the calculation of ZoomToFocusInput's target rect to APZ. r?botond

With specifying the flag for ZoomToFocusedInput works in OOP iframes.

Depends on D184439

Comment 6

[Hiroyuki Ikezoe (:hiro)]

With these changes, I think ZoomToFocusedInput works in OOP iframes. Though I just tested it on the example in comment 2. Thank you Daniel for keeping it working as expected!

I wasn't sure there's any automated test cases for ZoomToFocusedInput, but now I found test_group_zoomToFocusedInput.html. I will add an OOP iframe case there.

Comment 7

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1649440 - Rewrite helper_zoomToFocusedInput_iframe.html with SpecialPowers.spawn. r?botond

With SpecialPowers.spawn we can easily make the the test work in OOP iframes.

Depends on D184440

Comment 8

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1649440 - Add another variant of helper_zoomToFocusedInput_iframe.html to run the test case in an OOP iframe. r?botond

Depends on D184446

Comment 9

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1649440 - Move ContentTaskUtils into specialpowers to make SpecialPowers.spawn work in reftests. r?jmaher,jgraham

We'd like to use SpecialPowers.spawn to do something in cross-origin iframes in reftests.
Without moving the file calling SpecialPowers.spawn in reftests raises an exception
at here [1] in SpecialPowersChild.sys.mjs.

[1] https://searchfox.org/mozilla-central/rev/2bf90dc51ce7e8274ce208fbb9d68b3ff535185e/testing/specialpowers/content/SpecialPowersChild.sys.mjs#1637

Depends on D184439

Comment 10

[Hiroyuki Ikezoe (:hiro)]

Adding bug 1827330 into the dependency list since one of the changes here depends bug 1827330.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/677d0763ba27
Send GeckoView:ZoomToInput messages only to the focused browsing context. r=geckoview-reviewers,m_kato
https://hg.mozilla.org/integration/autoland/rev/9f29d5bbcaa7
Move ContentTaskUtils into specialpowers to make SpecialPowers.spawn work in reftests. r=jmaher
https://hg.mozilla.org/integration/autoland/rev/4c44cb1968ae
Defer the calculation of ZoomToFocusInput's target rect to APZ. r=botond
https://hg.mozilla.org/integration/autoland/rev/a4ee97519b88
Rewrite helper_zoomToFocusedInput_iframe.html with SpecialPowers.spawn. r=botond
https://hg.mozilla.org/integration/autoland/rev/4cca72b5cd90
Add another variant of helper_zoomToFocusedInput_iframe.html to run the test case in an OOP iframe. r=botond

Comment 12

[Cristian Tuns]

Backed out for causing reftest failures in zoom-to-focus-input-oopif.html

• Backout link
• Push with failures
• Failure Log
• Failure line: REFTEST TEST-UNEXPECTED-FAIL | gfx/layers/apz/test/reftest/zoom-to-focus-input-oopif.html == gfx/layers/apz/test/reftest/zoom-to-focus-input-oopif-ref.html | load failed: timed out waiting for reftest-wait to be removed

Comment 13

[Hiroyuki Ikezoe (:hiro)]

needs-focus was necessary for the reftest. I don't know why the test passed on other platforms without it.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9d276bd75cbb
Send GeckoView:ZoomToInput messages only to the focused browsing context. r=geckoview-reviewers,m_kato
https://hg.mozilla.org/integration/autoland/rev/5aa7f43e3cdc
Move ContentTaskUtils into specialpowers to make SpecialPowers.spawn work in reftests. r=jmaher
https://hg.mozilla.org/integration/autoland/rev/b5536df01db7
Defer the calculation of ZoomToFocusInput's target rect to APZ. r=botond
https://hg.mozilla.org/integration/autoland/rev/a56fcdc3931e
Rewrite helper_zoomToFocusedInput_iframe.html with SpecialPowers.spawn. r=botond
https://hg.mozilla.org/integration/autoland/rev/91f2928497ba
Add another variant of helper_zoomToFocusedInput_iframe.html to run the test case in an OOP iframe. r=botond

Comment 15

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/9d276bd75cbb
https://hg.mozilla.org/mozilla-central/rev/5aa7f43e3cdc
https://hg.mozilla.org/mozilla-central/rev/b5536df01db7
https://hg.mozilla.org/mozilla-central/rev/a56fcdc3931e
https://hg.mozilla.org/mozilla-central/rev/91f2928497ba