Closed Bug 1566183 Opened 11 months ago Closed 9 months ago

Picture-in-Picture player window can't be accessed with keyboard only

Categories

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

68 Branch
defect

Tracking

()

VERIFIED FIXED
mozilla71
Tracking Status
firefox71 --- verified

People

(Reporter: mconley, Assigned: mconley)

References

Details

(Keywords: access)

Attachments

(1 file)

Steps to reproduce:

  1. Launch Nightly
  2. Go to Youtube https://youtube.com
  3. Select any video to play
  4. Use only the keyboard to enable the Picture-in-Picture window using the context menu or the toggle.

Expected Result:
The PiP should be accessible with keyboard only.

Actual Result:
The PiP can't be enabled using keyboard only.

I have a patch up here that adds a keyboard shortcut, Ctrl-Alt-P / Cmd-Alt-P to enter Picture-in-Picture for the first found video in the focused frame of the selected tab. I'm in the process of reaching out to UX to find out whether or not this keyboard shortcut needs to be run by anyone first - UX, localization, etc. Once I have that information, I'll request review.

Hi Asa, you were suggested as a potential source of feedback for the keyboard shortcut here. Does my selection of Ctrl/Cmd-Alt-P make sense, or should we go for something else?

Flags: needinfo?(asa)

Though this shortcut would overlap with the shortcut for presentation mode for PDFs in Firefox, practically I don't think that would be a significant problem.

I've looked around (macOS and Chrome PiP modes) and don't see any existing shortcuts to emulate for PiP.

I've also checked and this shortcut doesn't seem to be used very often in other apps. It does show up some, for example in Word's switch to print layout view and OneNote for play audio recordings, but I don't think that should cause any significant confusion.

Shorlander had some good questions:

  • Does it conflict with another popular or expected shortcut? No.
    -- Related: Is there already a commonly accepted shortcut for this action? No.
  • Does it work across operating systems? Yes.
  • Is it somewhat easy to activate? (i.e. how much do you have to contort your hand to do it?) Requires two hands so not the most convenient but that's true of many of our current shortcuts.

I think we can go with this.

Flags: needinfo?(asa)

Hey Asa,

According to Neil in https://phabricator.services.mozilla.com/D40082#1215968, our original keyboard shortcut proposal (Ctrl+Alt+P) may not be workable because Ctrl+Alt+P maps to AltGr+P on some keyboards, which is a printable character.

Is Ctrl+Shift+V acceptable instead?

Flags: needinfo?(asa)

Ctrl+Shift+V is an existing Firefox shortcut for paste without formatting. Unless I'm missing something obvious, a reasonable assumption :) I can imagine scenarios where a page had a video and a text form that could make re-using this shortcut problematic.

Flags: needinfo?(asa)

(In reply to Asa Dotzler [:asa] from comment #7)

Ctrl+Shift+V is an existing Firefox shortcut for paste without formatting.

Shoot, good point. :( Alright let me see if I can find another...

Ctrl+Shift+} doesn't appear taken. Can I use that? (I'm basically picking characters at random though, unless anybody has a better suggestion.)

Flags: needinfo?(asa)

(In reply to Mike Conley (:mconley) (:⚙️) (Catching up from PTO) from comment #9)

Ctrl+Shift+} doesn't appear taken. Can I use that? (I'm basically picking characters at random though, unless anybody has a better suggestion.)

ctrl+shift+] is "bring to front" for some popular design applications so it's kind of on point. I'm looking into a possible conflict with JAWS, the popular screen reader. Will report back soon.

Flags: needinfo?(asa)
Flags: needinfo?(enndeakin)

Hi Asa, any updates on whether ctrl+shift+] makes sense as the keyboard shortcut?

Flags: needinfo?(asa)

(In reply to Mike Conley (:mconley) (:⚙️) from comment #11)

Hi Asa, any updates on whether ctrl+shift+] makes sense as the keyboard shortcut?

I think this makes sense. Note, for some people using the JAWS screen reader, the keyboard shortcut will be usurped for screen reader functionality.

Flags: needinfo?(asa)
Pushed by mconley@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/bb08767934a8
Add a keyboard shortcut to enter Picture-in-Picture for first video of focused window. r=NeilDeakin,JSON_voorhees,flod

Backed out for causing perma fails on browser_duplicateIDs.js

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedJob=265182262&resultStatus=testfailed%2Cbusted%2Cexception&revision=bb08767934a86eabb6127ce3759e70d3fa083771

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=265182262&repo=autoland&lineNumber=2158

[task 2019-09-05T17:01:20.607Z] 17:01:20 INFO - TEST-START | browser/base/content/test/general/browser_duplicateIDs.js
[task 2019-09-05T17:01:20.745Z] 17:01:20 INFO - TEST-PASS | browser/base/content/test/general/browser_duplicateIDs.js | key_toggleReaderMode should be unique -
[task 2019-09-05T17:01:20.745Z] 17:01:20 INFO - TEST-PASS | browser/base/content/test/general/browser_duplicateIDs.js | key_togglePictureInPicture should be unique -
[task 2019-09-05T17:01:20.745Z] 17:01:20 INFO - Buffered messages finished
[task 2019-09-05T17:01:20.746Z] 17:01:20 INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/general/browser_duplicateIDs.js | key_togglePictureInPicture should be unique -
[task 2019-09-05T17:01:20.746Z] 17:01:20 INFO - Stack trace:
[task 2019-09-05T17:01:20.746Z] 17:01:20 INFO - chrome://mochikit/content/browser-test.js:test_ok:1580
[task 2019-09-05T17:01:20.746Z] 17:01:20 INFO - chrome://mochitests/content/browser/browser/base/content/test/general/browser_duplicateIDs.js:test/<:7
[task 2019-09-05T17:01:20.746Z] 17:01:20 INFO - chrome://mochitests/content/browser/browser/base/content/test/general/browser_duplicateIDs.js:test:3
[task 2019-09-05T17:01:20.746Z] 17:01:20 INFO - chrome://mochikit/content/browser-test.js:Tester_execTest:1387
[task 2019-09-05T17:01:20.746Z] 17:01:20 INFO - chrome://mochikit/content/browser-test.js:nextTest/<:1213
[task 2019-09-05T17:01:20.746Z] 17:01:20 INFO - chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<:805
[task 2019-09-05T17:01:20.747Z] 17:01:20 INFO - TEST-PASS | browser/base/content/test/general/browser_duplicateIDs.js | key_reload should be unique -

Backout: https://hg.mozilla.org/integration/autoland/rev/40265561197b8f035773576eed117dc2d1321ae6

Flags: needinfo?(mconley)
Pushed by mconley@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/dbcd1724fe42
Add a keyboard shortcut to enter Picture-in-Picture for first video of focused window. r=NeilDeakin,JSON_voorhees,flod
Flags: needinfo?(mconley)
Status: NEW → RESOLVED
Closed: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla71
Assignee: nobody → mconley

Build ID 20190906094324
User Agent Mozilla/5.0 (Windows NT 10.0; rv:71.0) Gecko/20100101 Firefox/71.0

Verified as fixed on the latest Nightly build.

Status: RESOLVED → VERIFIED
See Also: → 1599376
You need to log in before you can comment on or make changes to this bug.