1696717 - Bad interaction between fullscreen and viewport scaling (viewport initial scale or desktop viewport).

Status: RESOLVED FIXED

(Core :: Panning and Zooming, defect)

Description

[Emilio Cobos Álvarez (:emilio)]

Attachment: Testcase

On Firefox for android, going fullscreen on a page that has a desktop viewport can cause issues with percentages etc if the desktop page has overflow. See the attached test-case, which doesn't show the bottom green stripe because the page is sized incorrectly.

Comment 1

[Emilio Cobos Álvarez (:emilio)]

This is the cause for at least some of the issues in https://github.com/mozilla-mobile/fenix/issues/8252

Hiro, are you familiar with how we deal with fullscreen on android? And if not, do you know who would? I can probably take a look though I'm a bit swamped with other stuff atm so if we could prioritize this it'd be great.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Poking a bit, it seems we don't do anything particularly special other than AdjustViewportSizeForFixedPosition. That expands to the layout viewport if needed. Except that is too big in this case... ni?ing to not lose track of this.

Comment 3

[Emilio Cobos Álvarez (:emilio)]

So chrome seems to ignore the meta viewport stuff and resolution in fullscreen, if I'm testing this right... This might allow us to undo bug 1659761, I believe...

Comment 4

[Emilio Cobos Álvarez (:emilio)]

tentatively taking.

Comment 5

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1696717 - Constify the dom fullscreen code a bit. r=botond

Comment 6

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1696717 - Extract a NeedsToShrinkToDisplaySize helper in MobileViewportManager. r=botond

This is just drive-by and shouldn't change behavior but I wrote it while
I was looking at the code so if you think it's fine I think we could
just land it.

Depends on D107362

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1696717 - Mark GeckoMVMContext final. r=botond

Again, just drive-by cleanup.

Depends on D107363

Comment 8

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1696717 - Ignore viewport meta in fullscreen. r=botond

This seems to match Chrome, and makes our fullscreen implementation more
consistent between desktop and mobile.

Do you know how to best test this? This repros in RDM, but all
fullscreen tests seem disabled on Android...

Depends on D107364

Comment 10

[Emilio Cobos Álvarez (:emilio)]

https://treeherder.mozilla.org/jobs?repo=try&revision=017c1d6b357b630dc31eba539df7534469d0ed5a

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8364fcab9a7f
Constify the dom fullscreen code a bit. r=hiro
https://hg.mozilla.org/integration/autoland/rev/3e627b123676
Mark GeckoMVMContext final. r=hiro

Comment 12

[Hiroyuki Ikezoe (:hiro)]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #1)

This is the cause for at least some of the issues in https://github.com/mozilla-mobile/fenix/issues/8252

Hiro, are you familiar with how we deal with fullscreen on android? And if not, do you know who would? I can probably take a look though I'm a bit swamped with other stuff atm so if we could prioritize this it'd be great.

I am not familiar with the stuff, but I could recall. GeckoView has onFullScreen function which is a notification when entering fullscreen and leaving from fullscreen. And each browser reacts to the notification, for example Fenix hides/shows their toolbars.

Anyways, if chrome ignores meta viewport, doing the same thing sounds quite reasonable to me.

Comment 13

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/8364fcab9a7f
https://hg.mozilla.org/mozilla-central/rev/3e627b123676

Comment 14

[Hiroyuki Ikezoe (:hiro)]

For references, note that I've also confirmed that Chrome doesn't applying viewport size changes by meta viewport. What I checked is the returns value of document.documentElement.getBoundingClientRect(), the width is 980px on non fullscreen state, 393px on fullscreen state on my pixel 3.

Also note that the explainer about viewport by David Bokan mentions;

The fixed viewport is sized to the minimum scale size. This means that a position: fixed element that has width: 100%; height: 100% will exactly fill the viewport when the page is fully zoomed out.

So if Chrome didn't ignore the viewport stuff on fullscreen state (I'd expect Chrome implements fullscreen stuff by applying position:fixed), the fullscreen-ed element width should be 980px. So I am mostly sure Chrome ignores viewport stuff on fullscreen stuff.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2fa7b66f8438
Ignore viewport meta in fullscreen. r=botond

Comment 16

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/2fa7b66f8438

Comment 17

[Alastor Wu [:alwu]]

I think this change deserves being added in the release note.

Release Note Request (optional, but appreciated)
[Why is this notable]: This fixed the issue where video playing in fullscreen and picture-in-picture would not display correctly. Espeically in the picture-in-picture mode, the video would be cropped seriously that is almost not able to be watched.
[Affects Firefox for Android]: Yes, this only affects Firefox Android.
[Suggested wording]:
[Links (documentation, blog post, etc)]:

Comment 18

[Ryan VanderMeulen [:RyanVM]]

Added to the Fx88 beta relnotes.