Closed
Bug 1333008
Opened 7 years ago
Closed 7 years ago
Pressing the space key do nothing after clicking Play/Pause button from the media control panel
Categories
(Toolkit :: Video/Audio Controls, defect)
Tracking
()
VERIFIED
FIXED
mozilla54
People
(Reporter: hyacoub, Assigned: ralin)
References
Details
Attachments
(1 file)
59 bytes,
text/x-review-board-request
|
jaws
:
review+
gchang
:
approval-mozilla-aurora+
|
Details |
[Affected versions]: Nightly 53.0a1 [Affected platforms]: All platforms: Windows 10 x 64, Ubuntu 16.04 [Steps to reproduce]: 1. Launch Nightly 53.0a1 with a new profile and then open this link https://people-mozilla.org/~ralin/vtt/. 2. Click on the play button from the control panel. 3. Press space key. [Expected result]: The video should play or pause depend on the previous behavior. [Actual result]: Pressing the space key do nothing. [Note] It's not reproducible on Mac OS X 10.11
Assignee | ||
Comment 1•7 years ago
|
||
I suspect that on Windows & Ubuntu, the <video> element isn't focused after clicking(on play button). Sorry that I don't get a Windows environment on hand right now, I'll check on it tomorrow. Thanks.
Flags: needinfo?(ralin)
This was mentioned by SV as partially blocking "Video Play Visual Refresh" feature. Tracked for 53+
tracking-firefox53:
--- → +
Assignee | ||
Comment 3•7 years ago
|
||
Video actually play->pause->play or pause->play->pause when pressing space in a flash. After a quick check, I got the result: Mac OS : only <video> is focused. Press space, then play/pause depends on current state [0] Windows: <video>, <videocontrols>, <button>(play button), are all focused. Similar to Mac OS, but pressing space trigger 'click' on play button as well. On the one hand, <video> handler play the video, on the other hand, <button> pause the video right away. (vice versa) Ubuntu : haven't verified yet. Should be same as Windows Two thoughts: 1. disable the focusability of all controls (except CC and Fullsceen Button, see Bug 1329555). Apparently, it make more sense as that is how videocontrols works in Mac OS. 2. prevent `keypress` event from default behavior. [0] https://dxr.mozilla.org/mozilla-central/rev/3cedab21a7e65e6a1c4c2294ecfb5502575a46e3/toolkit/content/widgets/videocontrols.xml#1320
Assignee | ||
Comment 4•7 years ago
|
||
Hi Jared, I initially thought we could use `-moz-user-focus: ignore` to avoid focusing on the controls, and after a while trying, I found another bug 379939 addressing `-moz-user-focus` issue on HTML elements. Not sure is it a good way to call "event.target.blur()" right away while controls being focused or "preventDefault" to "keypress"? Could you give me some suggestions? Thanks.
Flags: needinfo?(ralin)
Updated•7 years ago
|
Flags: needinfo?(jaws)
Comment 5•7 years ago
|
||
I believe the reason you can't reproduce on OSX is because by default OSX has Full Keyboard Access for All Controls disabled (see https://www.macobserver.com/tmo/article/mac_os_x_the_power_of_full_keyboard_access for how to enable it). I don't think we should be removing keyboard focus from these buttons. I would prefer instead if we made it easier to use the keyboard with video controls, though that is probably more work for another bug. The "space" key triggers whichever button has focus. It also is being used to play/pause the video. In videocontrols.xml's keyHandler function, inside of the switch case for "space", we should check if the event.originalTarget's localName is "button" and if the button is not disabled. If the originalTarget is an enabled button, then we should break out of the switch statement and not call togglePause().
Flags: needinfo?(jaws)
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → ralin
Status: NEW → ASSIGNED
Comment hidden (mozreview-request) |
Comment 7•7 years ago
|
||
mozreview-review |
Comment on attachment 8838024 [details] Bug 1333008 - Don't toggle play or pause when button is focused. https://reviewboard.mozilla.org/r/113026/#review114532 r=me with the below issue fixed. ::: toolkit/content/widgets/videocontrols.xml:1358 (Diff revision 1) > try { > switch (keystroke) { > case "space": /* Play */ > + let target = event.originalTarget; > + if (target.localName === "button" && !target.disabled) { > + return; This should be `break;` instead of `return;`. Note that at the bottom of this function we call `event.preventDefault();` to prevent page scrolling.
Attachment #8838024 -
Flags: review?(jaws) → review+
Comment 8•7 years ago
|
||
Also, it would be good to include a test here.
Comment hidden (mozreview-request) |
Assignee | ||
Comment 10•7 years ago
|
||
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #8) > Also, it would be good to include a test here. Thank you Jared :) issues are fixed
Keywords: checkin-needed
Comment 11•7 years ago
|
||
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/autoland/rev/75c3c966cf29 Don't toggle play or pause when button is focused. r=jaws
Keywords: checkin-needed
Comment 12•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/75c3c966cf29
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox54:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Assignee | ||
Comment 13•7 years ago
|
||
Comment on attachment 8838024 [details] Bug 1333008 - Don't toggle play or pause when button is focused. Approval Request Comment [Feature/Bug causing the regression]: Bug 1271765 [User impact if declined]: cannot toggle video play/pause by press space on play button [Is this code covered by automated tests?]: yes [Has the fix been verified in Nightly?]: no [Needs manual test from QE? If yes, steps to reproduce]: no [List of other uplifts needed for the feature/fix]: none [Is the change risky?]: low [Why is the change risky/not risky?]: only add a specified condition for early return [String changes made/needed]: none Thanks :)
Attachment #8838024 -
Flags: approval-mozilla-aurora?
Comment 14•7 years ago
|
||
Hi Brindusa, could you help find someone to verify if this issue was fixed as expected on a latest Nightly build? Thanks!
Flags: needinfo?(brindusa.tot)
Reporter | ||
Comment 15•7 years ago
|
||
Build ID: 20170221030205 User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:54.0) Gecko/20100101 Firefox/54.0 Verified as fixed on Windows 10 x 64, Mac OS X 10.10 and Ubuntu 16.04 x64 on Firefox Nightly 54.0a1.
Comment 16•7 years ago
|
||
Comment on attachment 8838024 [details] Bug 1333008 - Don't toggle play or pause when button is focused. Fix an issue related to video play/pause control by pressing space key and was verified. Aurora53+.
Attachment #8838024 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 17•7 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-aurora/rev/27470afc80e8
Comment 18•7 years ago
|
||
Setting qe-verify- based on Ray's assessment on manual testing needs (see Comment 13) and the fact that this fix has automated coverage. However, this is a good candidate for the community. Flagging it accordingly.
QA Whiteboard: [good first verify]
Flags: qe-verify-
You need to log in
before you can comment on or make changes to this bug.
Description
•