Picture-in-Picture keyboard shortcut should open the video currently playing if there's multiple videos on a page
Categories
(Toolkit :: Video/Audio Controls, defect, P5)
Tracking
()
Tracking | Status | |
---|---|---|
firefox75 | --- | fixed |
People
(Reporter: sdk, Assigned: sapoliakaran, Mentored, NeedInfo)
References
(Blocks 1 open bug)
Details
(Keywords: good-first-bug, nightly-community, Whiteboard: [lang=js])
Attachments
(1 file)
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:72.0) Gecko/20100101 Firefox/72.0
Steps to reproduce:
- Open a page with multiple video on it (e.g https://mozilla.github.io/open-leadership-training-series/articles/introduction-to-open-leadership/getting-to-know-mozilla-and-the-leadership-network/)
- Play the second video
- Open Picture-in-Picture
Actual results:
Picture-in-Picture opens and shows the first video in it's window
Expected results:
Picture-in-Picture should open the video currently playing in it's window
Reporter | ||
Updated•4 years ago
|
Comment 1•4 years ago
|
||
I'm unable to reproduce this on Windows. The expected video moves to a PiP when clicking on the blue button that overlays the video. Are you entering PiP through a different method? Can you try with Firefox Nightly since this may have been already fixed ( http://nightly.mozilla.org/ )?
Reporter | ||
Comment 2•4 years ago
|
||
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #1)
I'm unable to reproduce this on Windows. The expected video moves to a PiP when clicking on the blue button that overlays the video. Are you entering PiP through a different method?
Aha. I tried with the toggle button and it works. So, it seems like this issue only happens if I use the keyboard shortcut.
Can you try with Firefox Nightly since this may have been already fixed ( http://nightly.mozilla.org/ )?
I'm always on Nightly ;)
Comment 3•4 years ago
|
||
(In reply to Danny Colin [:sdk] from comment #2)
Can you try with Firefox Nightly since this may have been already fixed ( http://nightly.mozilla.org/ )?
I'm always on Nightly ;)
Ah, I asked because comment #0 mentions Firefox 72 in the user agent string.
With the keyboard shortcut we can probably choose the first video that is playing at least.
Reporter | ||
Comment 4•4 years ago
|
||
Ah, I asked because comment #0 mentions Firefox 72 in the user agent string.
I wasn't on my main laptop when I filed this bugreport ;).
With the keyboard shortcut we can probably choose the first video that is playing at least.
Don't sure I'm following you. Do you mean that with the shortcut you can't choose the video
currently playing but only a specific <video> tag in the DOM (in this case the first one being the
default)?
Comment 5•4 years ago
|
||
What I'm trying to say is that instead of doing something roughly like document.querySelector("video")
to find the video that should be moved to the new window, we can do Array.from(document.querySelectorAll("video")).filter(v => !v.paused) || document.querySelector("video")
.
This way if the keyboard shortcut is used we will pop out the first non-paused video if one exists, otherwise just the first video we find. Of course there is more nuance to it, in that we should only pop out videos that meet the criteria to be moved to a PiP window. You can see the criteria here, https://searchfox.org/mozilla-central/rev/2e355fa82aaa87e8424a9927c8136be184eeb6c7/toolkit/content/widgets/videocontrols.js#126-161
Updated•4 years ago
|
Comment 6•4 years ago
|
||
The code for this is at https://searchfox.org/mozilla-central/rev/2e355fa82aaa87e8424a9927c8136be184eeb6c7/toolkit/actors/PictureInPictureChild.jsm#1397-1400.
Would you like to submit a patch?
Updated•4 years ago
|
Reporter | ||
Comment 7•4 years ago
|
||
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #6)
Would you like to submit a patch?
Sadly, I don't have enough free time to set a build environment and work on a patch currently.
Thanks for triaging it tho. Hopefully, with the good-first-bug this issue will gain more visibility ;).
Updated•4 years ago
|
Comment 8•4 years ago
|
||
Hi, I'd like to work on this, is that still open ?
Comment 9•4 years ago
|
||
Hi Julien! Yes, this bug is unassigned. Do you have a local build of Firefox set up to work on?
Assignee | ||
Comment 10•4 years ago
|
||
The code for this is at https://searchfox.org/mozilla-central/rev/2e355fa82aaa87e8424a9927c8136be184eeb6c7/toolkit/actors/PictureInPictureChild.jsm#1397-1400.
Would you like to submit a patch?
Hello Jared, I created the patch as suggested.
The suggested patch has one issue though.
If you pause the currently playing video in PiP mode, and then use the keyboard shortcut to close the PiP window, the first video in the page opens up in the PiP window instead of the PiP window closing. I think the user expects the window to close instead. Also, I think this happens because after using the shortcut to close the PiP window, the keyboard focus immediately goes to the body element and so our patch picks up the first video immediately, insted of closing the window. Let me know if we can address that within this patch, or should we create a separate bug for that. I have attached a gif of the case: https://gfycat.com/wetcleverkingfisher
Also, I think we should give keyboard focus the top priority. With the patch, when all videos are paused, only the first video in the page opens in PiP, even if someone has put keyboard focus on the second or third video in the page. For a person using keyboard as their primary input source, keyboard focus is the way of selecting things in the page. It might be unintuitive for them if the have used the tab key to put focus on a subsequent video, and on using the keyboard shortcut, the first video on the page opens up.
Assignee | ||
Comment 11•4 years ago
|
||
If a page has multiple videos, PiP keyboard shortcut opens the currently playing video.
Opens the first video, if no video is playing.
Updated•4 years ago
|
Assignee | ||
Comment 12•4 years ago
|
||
Kept this patch limited to opening first of current playing videos only, so as to keep the 1 bug per issue focus. Would like to discuss (and implement, if useful) keyboard focus priority with everyone's inputs :)
Comment 13•4 years ago
|
||
Someone can give me the link to download the local version of Firefox, please.
Comment 14•4 years ago
|
||
Pushed by jwein@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/0aa4f9852887 Picture-in-Picture keyboard shortcut should open currently playing video. r=jaws
Comment 15•4 years ago
|
||
bugherder |
Reporter | ||
Updated•4 years ago
|
Updated•4 years ago
|
Description
•