Closed Bug 1474574 Opened 2 years ago Closed 2 years ago
Focus the player immediately when opening media files
59 bytes, text/x-review-board-request
User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:61.0) Gecko/20100101 Firefox/61.0 Build ID: 20180703192103 Steps to reproduce: Open a media file in a new tab that is supported by Firefox (e.g. mp3). The Firefox media player is shown, the file starts playing. For example take https://ondemand-mp3.dradio.de/file/dradio/2018/06/29/zeitenwende_ende_des_internet_wie_wir_es_kennen_drk_20180629_0720_b130116b.mp3 Actual results: Since Firefox 61 nothing happens if you now press the space key (to pause), the cursor keys left or right (to jump in the file) or up and down (to adjust volume). The reason is that when opening a media file the HTML tag has the keyboard focus. You have to press the TAB key first to shift the focus to the player. Expected results: Prior to Firefox 61 when opening a media file in a new tab the player was focussed at once. You could act immediately with the player via keyboard. It would be great if this behaviour could be restored. (By the way: Does somebody know what key I can press to get a focussed video fullscreen?)
Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:63.0) Gecko/20100101 Firefox/63.0 20180710100040 https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=556811240710a775a0a1ec982cad4d27b02ff7f6&tochange=99fc41ec7ce98a3617b8dc4e5198fff23565cf6a
Thanks for filing. (In reply to Gerhard Großmann from comment #0) > (By the way: Does somebody know what key I can press to get a focussed video > fullscreen?) There isn't a hotkey for that. You might want to file another issue as a feature request.
(In reply to Tim Guan-tin Chien [:timdream] (please needinfo) from comment #2) > (In reply to Gerhard Großmann from comment #0) > > (By the way: Does somebody know what key I can press to get a focussed video > > fullscreen?) > > There isn't a hotkey for that. You might want to file another issue as a > feature request. That's incorrect; the hotkey is F11 and it is handled in TopLebelVideoDocument.js https://searchfox.org/mozilla-central/rev/a80651653faa78fa4dfbd238d099c2aad1cec304/toolkit/content/TopLevelVideoDocument.js The same script is responsible for regenerating the key events, my patch breaks it by ignoring these untrusted events.
Rev 2 fixes the regression (and make videocontrols re-accepts untrusted events), while rev 3 offers a better fix IMHO. These documents are accessible by the same-origin web content, including the |videoElement| variable... I've filed bug 1474832 for that.
Comment on attachment 8991202 [details] Bug 1474574 - Ensure <video> is the only focusable element in TopLevelVideoDocument https://reviewboard.mozilla.org/r/256168/#review263054 ::: toolkit/content/TopLevelVideoDocument.js:12 (Diff revision 3) > // <video> is used for top-level audio documents as well > let videoElement = document.getElementsByTagName("video"); > > -// 1. Handle fullscreen mode; > -// 2. Send keystrokes to the video element if the body element is focused, > -// to be received by the event listener in videocontrols.xml. > +// Redirect focus to the video element whenever the document receives > +// focus. > +document.addEventListener('focus', () => videoElement.focus(), true); As per lint, this should use double quotes.
Attachment #8991202 - Flags: review?(gijskruitbosch+bugs) → review+
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/2e214762b823 Ensure <video> is the only focusable element in TopLevelVideoDocument r=Gijs
Will request for uplift
See Also: → 1478484
Comment on attachment 8991202 [details] Bug 1474574 - Ensure <video> is the only focusable element in TopLevelVideoDocument Approval Request Comment [Feature/Bug causing the regression]: regression of bug 1444489 [User impact if declined]: The user will not be able to use keyboard to control media docuemnts. [Is this code covered by automated tests?]: No. [Has the fix been verified in Nightly?]: Yes. [Needs manual test from QE? If yes, steps to reproduce]: See comment 0 [List of other uplifts needed for the feature/fix]: bug 1478484 is a follow up that need to be uplifted altogether. [Is the change risky?]: no [Why is the change risky/not risky?]: minor behavior fix, unlikely to break other things. [String changes made/needed]: no.
Attachment #8991202 - Flags: approval-mozilla-beta?
Comment on attachment 8991202 [details] Bug 1474574 - Ensure <video> is the only focusable element in TopLevelVideoDocument Fix for regression from 61, looks safe, let's uplift for beta 16.
Attachment #8991202 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Hi, I've tested this issue on Nightly 63.0a1(20180809220138) and Beta 62.0b16(20180809104529) and is still reproducing intermittently, the player is not always focused immediately after opening the link from comment 0 and audio file links from here: http://media.ca7.uscourts.gov/sound/external/ Tested on: Win10 x64, Ubuntu 16.04 and MacOS 10.13.
Upon testing the current Nightly again I've realized there are two STRs and the fix (together with bug 1478484 only fixes one of them.) The fix here helps when the focus is given to the document, e.g. when the user switched to the tab with audio/video. If the page is navigated to, it is still broken. I am filing a follow-up bug to deal with this.
It looks like TopLevelDocument.js does not receive the focus event sometimes. This cannot be reliably produced so I can't work on it yet...
Moving follow-up work to bug 1483408.
Verified as fixed on Windows 10 x64, Ubuntu 16.04 x64 and macOS 10.12.6 using the 62.0b20 taskcluster build from 2018-08-23 and latest Nightly 63.0a1.
Sorry for occupying your mailbox … I just wanted to say how thankful I am! I just updated to Firefox 62 and now audio files could be easily paused again with the space key. Thank you very much for your work, all the best!
You need to log in before you can comment on or make changes to this bug.