Closed Bug 1757031 Opened 2 years ago Closed 2 years ago

Fenix issue when coming from PIP: Video is halved.

Categories

(GeckoView :: Media, defect, P1)

Firefox 97
ARM64
Android

Tracking

(firefox-esr91 wontfix, firefox98 wontfix, firefox99 wontfix, firefox100 wontfix, firefox101 fixed, firefox102 fixed)

RESOLVED FIXED
102 Branch
Tracking Status
firefox-esr91 --- wontfix
firefox98 --- wontfix
firefox99 --- wontfix
firefox100 --- wontfix
firefox101 --- fixed
firefox102 --- fixed

People

(Reporter: petru, Assigned: m_kato)

References

Details

(Whiteboard: [geckoview:m100] [geckoview:m101] [geckoview:m102])

Attachments

(5 files, 1 obsolete file)

Attached video GvVideoFraming.mp4

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

STRs:

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
Attached image GvVideoFraming.png

(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:

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!

Flags: needinfo?(petru.lingurar)
Attached video 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.

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.

I wonder if this is related to bug 1731980

See Also: → 1731980

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.

Component: Layout → General
Product: Core → GeckoView
See Also: → 1754802

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.

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.

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?

Flags: needinfo?(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.

Flags: needinfo?(m_kato)

After landing bug 1754802, I will look this.

Assignee: nobody → 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

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

nsDOMWindowUtils::ExitFullscreen restores old window size unfortunately.

Tracking this bug for m100 because Makoto is investigating it.

Severity: -- → S2
Priority: -- → P1
Whiteboard: [geckoview:m100]

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

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.

Attachment #9272819 - Attachment description: Bug 1757031 - Don't restore previous viewport when window size isn't same. r=emilio!,#geckoview-reviewers → Bug 1757031 - Don't restore previous viewport when window size isn't same. r=emilio!
Whiteboard: [geckoview:m100] [geckoview:m101] → [geckoview:m100] [geckoview:m101] [geckoview:m102]
Pushed by m_kato@ga2.so-net.ne.jp:
https://hg.mozilla.org/integration/autoland/rev/9bdeb89e58fe
Don't restore previous viewport when window size isn't same. r=emilio
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 102 Branch

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

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?

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.

Flags: needinfo?(petru.lingurar)

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.

Flags: needinfo?(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
Flags: needinfo?(m_kato)
Attachment #9272819 - Flags: approval-mozilla-beta?
Flags: qe-verify+

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.

Attachment #9272819 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
QA Whiteboard: [qa-triaged]

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.

Flags: qe-verify+

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

Component: General → Media
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: