1617115 - [Ubuntu] PiP gets smaller if you navigate back and forth between videos with different formats

Status: REOPENED

(Core :: Widget: Gtk, defect, P2)

Description

[Oana Botisan, Desktop Release QA]

Attachment: Peek 2020-02-21 10-36.gif

Note

• This bug happens when the videos have different formats.
• Please look at the attached gif.

Affected versions

• Firefox 74.0b6
• Firefox 75.0a1

Affected platforms

• Ubuntu 18 x64

Steps to reproduce

1. Go to a video on youtube (e.g: https://www.youtube.com/watch?v=R7A1mIdiheE)
2. Click on the Picture-in-Picture button.
3. From the video controls click on the next song.
4. From the browser controls click on the "Go back page" button.

Expected result

• The video has the same dimensions as in step 2.

Actual result

• The video gets smaller.

Regression range

• If it's a regression I will try to find it as soon as possible.

Comment 1

[Mike Conley (:mconley) (:⚙️) (PTO: May 24 - June 2nd)]

Hey Mark,

I seem to recall us considering this scenario when you were working on the window resizing bits for PiP... was this case not covered somehow?

Comment 2

[Mark Striemer [:mstriemer]]

So I poked around at this on linux a little, added some dump calls with the expected size and position combinations and the math seems to add up correctly, but sometimes the window will end up moving/shrinking. Seems like the size/position isn't ending up as what PictureInPicture.jsm is requesting, I tried moving the window away from the edge of the screen to see if it was moving off the screen and being recalculated but that didn't seem to fix it. This still happens intermittently.

We have tests for this, but they were originally disabled on linux [1] since the resizing wasn't implemented, they weren't enabled again when the resizing patch landed. I tried running the tests on linux but they all fail since it can't decode the video file for some reason.

We should look at enabling that test file on linux, I'm guessing it would have intermittent failures right now though.

[1] https://searchfox.org/mozilla-central/rev/96f1457323cc598a36f5701f8e67aedaf97acfcf/toolkit/components/pictureinpicture/tests/browser.ini#35-36

Comment 3

[Mark Striemer [:mstriemer]]

Poked a little more and it looks like the win.moveTo(left, top) call [1] is basically being ignored. We're calculating the new expected window size and position then calling win.resizeTo(width, height); immediately followed by win.moveTo(left, top). I wrapped the win.moveTo() call in win.setTimeout(() => win.moveTo(...), 100) and it started working. Adding 500ms timeouts to the tests made them pass.

@stransky is there some other way we should be resizing and moving this window at the same time or do you have any idea why the win.moveTo() call is being ignored?

[1] https://searchfox.org/mozilla-central/rev/96f1457323cc598a36f5701f8e67aedaf97acfcf/toolkit/components/pictureinpicture/PictureInPicture.jsm#505-507

Comment 4

[Martin Stránský [:stransky] (ni? me)]

Yes, Linux/Gtk uses async resize. I'll look at it.

Comment 5

[Release mgmt bot [:sylvestre / :calixte / :marco for bugbug]]

The product::component has been changed since the backlog priority was decided, so we're resetting it.
For more information, please visit auto_nag documentation.

Comment 6

[Release mgmt bot [:sylvestre / :calixte / :marco for bugbug]]

Bugbug thinks this bug is a regression, but please revert this change in case of error.

Comment 7

[Ethan Tseng [:ethan]]

Martin, could you please set the priority flag? Is it still P3?

Comment 8

[Martin Stránský [:stransky] (ni? me)]

I can reproduce that on Fedora 32/X11 too. Works ok on Wayland.

Comment 9

[Martin Stránský [:stransky] (ni? me)]

Attachment: Bug 1617115 [GNOME] Use CSD mode in Gnome, r?jhorak

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0a6d64975cfa
[GNOME] Use CSD mode in Gnome, r=jhorak

Comment 11

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/0a6d64975cfa

Comment 12

[Maria Berlinger [:mberlinger]]

Hello,

This issue is still reproducible on Firefox 76.0a1 (2020-04-02).
Here is a screenrecording with the issue: https://drive.google.com/file/d/1gh8Zugbs89vWbPV-kfhoYKrb5OGJ1N3c/view?usp=sharing

Comment 13

[Martin Stránský [:stransky] (ni? me)]

(In reply to Maria Berlinger [:maria_berlinger], Release Desktop QA from comment #12)

Hello,

This issue is still reproducible on Firefox 76.0a1 (2020-04-02).
Here is a screenrecording with the issue: https://drive.google.com/file/d/1gh8Zugbs89vWbPV-kfhoYKrb5OGJ1N3c/view?usp=sharing

And which distro/DE do you run?

Comment 14

[Maria Berlinger [:mberlinger]]

Sorry that I forgot to mention, Ubuntu 18.04x64.

Comment 15

[Martin Stránský [:stransky] (ni? me)]

(In reply to Maria Berlinger [:maria_berlinger], Release Desktop QA from comment #14)

Sorry that I forgot to mention, Ubuntu 18.04x64.

Can you set "widget.use-aspect-ratio" to false and retest please?
Thanks.

Comment 16

[Maria Berlinger [:mberlinger]]

By creating the "widget.use-aspect-ratio" and set it to false, the issue is no more reproducible.

Comment 17

[Maria Berlinger [:mberlinger]]

Hello

Martin, do you think is worth filling another bug or to reopen this one, because the “widget.use-aspect-ratio" pref is not set by default on false and the issue still reproduces.

Comment 18

[Martin Stránský [:stransky] (ni? me)]

(In reply to Maria Berlinger [:maria_berlinger], Release Desktop QA from comment #17)

Hello

Martin, do you think is worth filling another bug or to reopen this one, because the “widget.use-aspect-ratio" pref is not set by default on false and the issue still reproduces.

That's really up to you, I don't have a strong opinion here.

When widget.use-aspect-ratio is set, keeping window aspect radio is disabled so the window is more difficult to resize by mouse cursor - you need to keep the aspect manually.

But it also fixes this bug and we can revert Bug 1624921 (PIP mode is switched back to system decorations) which fixes Bug 1622203.

Comment 19

[Maria Berlinger [:mberlinger]]

Based on the fact that "widget.use-aspect-ratio" = false pref isn't by default in Firefox (you should add it manually) this issue is still reproducible and the user will be hit by this problem, so in my humble opinion we should reopen this bug based on this assessment.

Comment 20

[Martin Stránský [:stransky] (ni? me)]

I wonder if a patch from Bug 1683075 can help here.