1666117 - PiP Player window video will not show correctly when video has no loaded metadata and using the context menu or shortcut to start Picture-in-Picture

Status: RESOLVED FIXED

(Toolkit :: Video/Audio Controls, defect, P3)

Description

[kernp25]

Attachment: 1BL7Y7Tvvd.mp4

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:80.0) Gecko/20100101 Firefox/80.0

Steps to reproduce:

1. Open https://www.w3.org/2010/05/video/mediaevents.html
2. Right-click on the video and choose Picture-in-Picture (PiP)

Actual results:

The player window is not in foreground (clicking on it will also not bring it to the foreground).

Comment 1

[kernp25]

Note: Only when the video starts playing, it will work correctly.

Comment 2

[:Gijs (he/him)]

Why did you needinfo me?

Comment 3

[kernp25]

(In reply to :Gijs (he/him) from comment #2)

Why did you needinfo me?

Should i needinfo Alex Cornestean?

So he can confirm this bug?

Comment 4

[:Gijs (he/him)]

(In reply to kernp25 from comment #3)

(In reply to :Gijs (he/him) from comment #2)

Why did you needinfo me?

Should i needinfo Alex Cornestean?

So he can confirm this bug?

Any defect/task (ie not enhancement) bug in this component is going to get triaged within a few days (we have triage meetings twice a week, the next one is today). In general, you do not need to needinfo anyone if all you're after is for someone to confirm the issue. Use the flag if you have concrete questions or if something is so urgent it can't wait a few days.

FWIW, I could reproduce but I don't have a clue off-hand what the root cause is, and having been away for a bit I'm too busy to do the digging myself right now - if this is a regression that information would help (but it can't be all that recent if it reproduces on release!).

Comment 5

[:Gijs (he/him)]

Root cause here is that https://searchfox.org/mozilla-central/rev/b58ca45005fe02077c92779483d1b60e9a49687c/toolkit/actors/PictureInPictureChild.jsm#1153-1154 assumes that there's a video width / height. In this case, these are both 0, because the video has the preload=none attribute.

We don't show the toggle before the video loads, but both the shortcut and context menu can start PiP in this case.

We should fall back on using the lazy/non-flushing bounding rect of the video, but if that returns height/width of 0, I think we should probably no-op instead of attempting to enter PiP? I also wonder if we should not offer the context menu item for videos in that state (though that doesn't "fix" the shortcut case).

Dropping this in the MSU backlog; I don't think almost any commercial video sites would use preload=none (as it adds delays to playing the video once the user hits play!) so the practical ramifications of this bug are probably limited - but I could imagine that on slower connections and with fast use of the shortcut, you could potentially trip this on realistic websites. Mike, does all that seem right?

Comment 6

[Mike Conley (:mconley) (:⚙️)]

Sorry for the delay. Yes, if the dimensions are 0x0, disabling the context menu (like we hide the toggle) makes sense to me.

Comment 7

[Micah [:mtigley] (she/her)]

This actually might be fixed by Bug 1626600. So once that resolved let's check on this bug again.

Comment 8

[Micah [:mtigley] (she/her)]

The issue looks like it's been fixed by Bug 1626600 in Nightly. Feel free to re-open if needed.