Focus the player immediately when opening media files

VERIFIED FIXED in Firefox 62

Status

()

defect
P2
normal
VERIFIED FIXED
10 months ago
8 months ago

People

(Reporter: gerhard.grossmann, Assigned: timdream)

Tracking

({regression})

61 Branch
mozilla63
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox-esr60 unaffected, firefox61 wontfix, firefox62 verified, firefox63 verified)

Details

Attachments

(1 attachment)

(Reporter)

Description

10 months ago
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?)

Comment 1

10 months ago
regression-window
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
Blocks: 1444489
Status: UNCONFIRMED → NEW
Has Regression Range: --- → yes
Component: Untriaged → XUL Widgets
Ever confirmed: true
Flags: needinfo?(timdream)
Keywords: regression
OS: Unspecified → All
Product: Firefox → Toolkit
Hardware: Unspecified → All
Assignee: nobody → timdream
Status: NEW → ASSIGNED
Flags: needinfo?(timdream)
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.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
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 hidden (obsolete)

Comment 9

10 months ago
mozreview-review
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")[0];
>  
> -// 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+
Comment hidden (mozreview-request)

Comment 11

10 months ago
Pushed by timdream@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/2e214762b823
Ensure <video> is the only focusable element in TopLevelVideoDocument r=Gijs

Updated

10 months ago
Priority: -- → P2

Comment 12

10 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/2e214762b823
Status: ASSIGNED → RESOLVED
Last Resolved: 10 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Will request for uplift
Flags: needinfo?(timdream)
See Also: → 1478484
Depends on: 1478484
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.
Flags: needinfo?(timdream)
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+
Flags: qe-verify+

Comment 17

9 months ago
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.
Flags: needinfo?(timdream)
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.
Flags: needinfo?(timdream)
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...
Flags: needinfo?(timdream)
Moving follow-up work to bug 1483408.
Flags: needinfo?(timdream)

Updated

8 months ago
Duplicate of this bug: 1483411
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.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
(Reporter)

Comment 23

8 months ago
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.