1757031 - Fenix issue when coming from PIP: Video is halved.

Status: RESOLVED FIXED

(GeckoView :: Media, defect, P1)

Description

[Petru-Mugurel Lingurar [:petru]]

Attachment: GvVideoFraming.mp4

Seen on all Release 97, Beta 98, Nightly 99.

STRs:

• access a youtube video, in my case https://m.youtube.com/watch?v=Aaehn1aY8Ig
• put the video in fullscreen
• press device's home button to enter in Picture in Picture mode
• exit PiP mode, enter the app

Expected result:

• Video is not in fullscreen anymore but do extend to the available GV layout bounds in the app.

Actual result:

• Video is halved (seems like it's width in landscape still occupys what would be screen width while in portrait) but GV does occupy the full width of the screen.
• Also saw another case in which the video is not halved but just smaller - the black bars are larger, maybe such that it would entirely fit the screen, while otherwise it would have to extend to a bit below the visible screen.
• A scroll in the page will have the video immediately take the expected dimensions.

Device used in tests:

• Oneplus 7T Pro with Android 11

Comment 1

[Petru-Mugurel Lingurar [:petru]]

Attachment: GvVideoFraming.png

Comment 2

[Hiroyuki Ikezoe (:hiro)]

(In reply to Petru-Mugurel Lingurar [:petru] from comment #0)

Created attachment 9265370 [details]
GvVideoFraming.mp4

Seen on all Release 97, Beta 98, Nightly 99.

STRs:

• access a youtube video, in my case https://m.youtube.com/watch?v=Aaehn1aY8Ig
• put the video in fullscreen
• press device's home button to enter in Picture in Picture mode
• exit PiP mode, enter the app

Expected result:

• Video is not in fullscreen anymore but do extend to the available GV layout bounds in the app.

Actual result:

• Video is halved (seems like it's width in landscape still occupys what would be screen width while in portrait) but GV does occupy the full width of the screen.
• Also saw another case in which the video is not halved but just smaller - the black bars are larger, maybe such that it would entirely fit the screen, while otherwise it would have to extend to a bit below the visible screen.
• A scroll in the page will have the video immediately take the expected dimensions.

Though I don't fully understand what the problem is, this last comment makes me suspect YouTube does something, I mean it's a fault on Youtube, isn't it?

Petru, can you provide a simplified test case? Thanks!

Comment 3

[Petru-Mugurel Lingurar [:petru]]

Attachment: ThinnerBigBuckBunny

Comment 4

[Petru-Mugurel Lingurar [:petru]]

Comment on attachment 9265861 [details]
ThinnerBigBuckBunny

>Don't think the issue is specific to youtube as this can easily be reproduced on other sites also like with the big bubk bunny video.
>
>Also seeing here that while the device was always in portrait mode upon returning from PIP
>- GeckoView is shown in landscape
>- content is drawn seemingly in the bounds available for portrait
>- the layout used is the one for landscape.

Comment 5

[Petru-Mugurel Lingurar [:petru]]

Attachment: BigBuckBunnyCutOff.mp4

Don't think the issue is specific to youtube as this can easily be reproduced on other sites also like with the big bubk bunny video.

Also seeing here that while the device was always in portrait mode upon returning from PIP

• GeckoView is shown in landscape
• content is drawn seemingly in the bounds available for portrait
• the layout used is the one for landscape.

Comment 6

[Petru-Mugurel Lingurar [:petru]]

Attachment: BigBuckBunnyPortraitLandscapePortrait.mp4

A simpler scenario in which to see the issue.
With the device always held in portrait, just entering fullscreen and then exiting will have the video have smaller width.
Tried in GeckoViewExample also but there fullscreen won't change the orientation so the issue doesn't reproduces.

Since I saw some issues with the Fenix/AC functionality related to fullscreen/setting orientation I've also opened a ticket on Fenix - https://github.com/mozilla-mobile/fenix/issues/24039 for investigations on this side but it seems like there is a layout/video player issue irrespective of Fenix set orientation.

Comment 7

[Kevin Brosnan [Ex-Mozilla]]

I wonder if this is related to bug 1731980

Comment 8

[Hiroyuki Ikezoe (:hiro)]

I tried the STR in comment 0 on my pixel 3 Android 12. I can't see the issue. Rather when the video gets fullscreened, even if I disabled Android's auto rotation setting, the video gets rotated to the landscape mode. And when the video becomes PiP mode, the auto rotation happens again. Isn't it something related to bug 1754802 maybe?

I'd start with GeckoView components.

Comment 9

[Makoto Kato [:m_kato]]

Actually, no way to set orientation from web content since orientation lock is delegated to a-c (See https://github.com/mozilla-mobile/fenix/issues/23415). I guess this is a-c's media feature.

Comment 10

[Petru-Mugurel Lingurar [:petru]]

There might be an issue in how AC sets the orientation depending on the fullscreen media status and it might be of essence for this bug - repeated / fast orientation changes. I opened https://github.com/mozilla-mobile/fenix/issues/24039 for more investigation on this side also.
But I also see a layout / media playback issue.
In https://bugzilla.mozilla.org/show_bug.cgi?id=1757031#c5 at the 10 second mark although the device is in landscape and GeckoView (View layout bounds that the app sets) actually extends to the full width of the screen the page layout is cut off.
And then when the orientation is set to portrait we still see the landscape layout - different that how the page looked in portrait at second 01.
This does to me sound like #1754802. Though I can reproduce this on the Oneplus 7T and a Pixel 3 both with Android 11. Can test again once that lands.

Comment 11

[Emily Toop (:fluffyemily)]

We need to determine whether this is a GeckoView issue or an AC one. Could you take a look and make a decision @m_kato?

Comment 12

[Makoto Kato [:m_kato]]

I guess that we have to figure out why window size isn't changed after exiting full screen. I might have to refresh window size when GV becomes active.

Comment 13

[Makoto Kato [:m_kato]]

After landing bug 1754802, I will look this.

Comment 14

[Makoto Kato [:m_kato]]

On debug build, Window size seems not to be updated after display information is changed (GeckoScreenOrientation log is outputted when display size information will be changed.)

03-08 12:43:18.652  8561  8587 D GeckoViewProgress: receiveMessage: MozAfterPaint
03-08 12:43:18.699  8561  8587 I Gecko   : nsWindow[0x7299b0eb9700]::Resize [291.000000 961.000000 1794.000000 1017.000000] (repaint 0)
03-08 12:43:18.699  8561  8587 I Gecko   : nsWindow: 0x7299b0eb9700 OnSizeChanged [1794 1017]
03-08 12:43:18.707  8561  8587 I Gecko   : nsWindow[0x7299b0eb9700]::Resize [0.000000 63.000000 1794.000000 1017.000000] (repaint 0)
03-08 12:43:18.733  8561  8561 D GeckoScreenOrientation: updating to new orientation LANDSCAPE_PRIMARY
03-08 12:43:18.738  8561  8587 D GeckoViewModule: dispatch GeckoViewContent:ExitFullScreen, data=null
03-08 12:43:18.738  8561  8587 D GeckoViewContent: onEvent: event=GeckoViewContent:ExitFullScreen, data=null
03-08 12:43:18.791  8561  8561 D GeckoRuntime: Lifecycle: onResume
03-08 12:43:18.791  8561  8561 D GeckoNetworkManager: Incoming event start for state OffWithListeners -> OnWithListeners
03-08 12:43:18.841  8561  8587 D GeckoViewContent: handleEvent: MozDOMFullscreen:Exited
03-08 12:43:18.861  8633  8691 D GeckoViewProgressDelegate[C]: handleEvent: MozAfterPaint
03-08 12:43:18.870  8561  8587 D GeckoViewProgress: receiveMessage: MozAfterPaint
03-08 12:43:18.874  8561  8561 D GeckoNetworkManager: Incoming event receivedUpdate for state OnWithListeners -> OnWithListeners
03-08 12:43:18.875  8561  8561 D GeckoNetworkManager: New network state: UP, CELLULAR, CELL_4G
03-08 12:43:18.918  8633  8691 D GeckoViewContent[C]: receiveMessage: GeckoView:DOMFullscreenExited

Comment 15

[Makoto Kato [:m_kato]]

window.outer* are updated, but window.inner* aren't updated...

Comment 16

[Makoto Kato [:m_kato]]

nsDOMWindowUtils::ExitFullscreen restores old window size unfortunately.

Comment 17

[Chris Peterson [:cpeterson]]

Tracking this bug for m100 because Makoto is investigating it.

Comment 18

[Amedyne Moya [:amoya]]

This can be another instance of this bug: https://github.com/mozilla-mobile/focus-android/issues/6804

Comment 19

[Chris Peterson [:cpeterson]]

Tracking for 101

Comment 20

[Makoto Kato [:m_kato]]

Attachment: Bug 1757031 - Don't restore previous viewport when window size isn't same. r=emilio!

Fenix's Picture-In-Picture uses full screen mode. So it isn't same as Firefox
Desktop. When exiting full screen, Fenix's window may not same as before
entering full screen.

When this occurs,

1. When device is portrait orientation, user changes to full screen by content
   script.
2. Fenix enters to full screen then orientation is changed to landscape.
3. User changes that device is changed to landscape by hand.
4. Fenix enters PiP mode. Android (Home screen) change to portrait orientation.
   Then PiP window keeps landscape orientation.
5. When exiting PiP, full screen will be exited.
6. Device is landscape (by Step 3.), so Fenix window is landscape. But Gecko
   restores 1.'s viewport by nsIDOMWindowUtils.exitFullScreen. So viewport
   becomes portrait orientation size even if window is landscape orientation.

For PiP mode, although nsIDOMWindowUtils.exitFullScreen is used, it doesn't
consider this situation. So I would like to add non-restore option for it.

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9bdeb89e58fe
Don't restore previous viewport when window size isn't same. r=emilio

Comment 22

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/9bdeb89e58fe

Comment 23

[BugBot [:suhaib / :marco/ :calixte]]

Since the status are different for nightly and release, what's the status for beta?
For more information, please visit auto_nag documentation.

Comment 24

[Chris Peterson [:cpeterson]]

Petru, does PIP video work for you now that Makoto's fix has landed? Do you think we should uplift Makoto's fix to Beta 101?

Comment 25

[Petru-Mugurel Lingurar [:petru]]

Yes, the issues seems fixed for me.
Tested now with beta in parallel where this is reproducing 5/5 times.
While on Nightly the issue dissapeared.

Given how easy it was to reproduce I'd say it would be nice to get this uplifted to beta.

Comment 26

[Chris Peterson [:cpeterson]]

Makoto, can you please request uplift for Beta 101? It would be nice for users to get your bug fix in 101 instead of waiting for 102.

Comment 27

[Makoto Kato [:m_kato]]

Comment on attachment 9272819 [details]
Bug 1757031 - Don't restore previous viewport when window size isn't same. r=emilio!

Beta/Release Uplift Approval Request

• User impact if declined: When using PiP video mode of new feature of Android 8, exiting PiP mode causes that viewport size of window becores incorrect when screen orientation is changed.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: See https://bugzilla.mozilla.org/show_bug.cgi?id=1757031#c0
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Low. This change is GeckoView only. We don't restore viewport information when orientation is different.
• String changes made/needed: Nothing
• Is Android affected?: Yes

Comment 28

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9272819 [details]
Bug 1757031 - Don't restore previous viewport when window size isn't same. r=emilio!

Approved for Fenix 101.0.0-beta.3.

Comment 29

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/4a33b6d1c174

Comment 30

[Laurentiu Apahidean]

I tested the issue on Firefox Beta 101.0.0-beta.3 using a OnePlus 6T (Android 9) and the issue no longer occurs, after returning from Picture in Picture the video is no longer halved.

Comment 31

[Chris Peterson [:cpeterson]]

Moving some media bugs to the new GeckoView::Media component.