1481400 - [Youtube] Play/Pause video using spacebar is not functional after clicking on Play/Pause button

Status: VERIFIED FIXED

(Core :: DOM: UI Events & Focus Handling, defect, P2)

Description

[Timea Cernea [:tbabos][inactive]]

[Affected versions]:
Latest Nightly 63.0a1 (2018-08-06)
Latest Firefox 62.0b14 (64-bit)
Latest Firefox Release 61.0.1 (64-bit)

[Affected Platforms]:
Windows 10 x64
Windows 7 x64
Ubuntu 16.04 x64

[Steps to reproduce]: 
1. Launch Firefox and go to any video on Youtube
2. After the video started click on "Pause" button
3. Press spacebar to resume the video

[Expected result]:
The video playback should be resumed.

[Actual result]:
The video is not resumed and the pause button animation is displayed on the video.

[Notes]:
1. The issue does not occur on Mac OS.
2. Pausing the video with spacebar after the "Play" button is clicked has the same behavior.
3. As long as the Play/Pause button is highlighted the spacebar will not work.

Comment 1

[Dão Gottwald [:dao]]

Oops, Youtube doesn't use default video controls.

Comment 2

[Henri Sivonen (:hsivonen) (temporarily away from Bugzilla)]

Not sure if this should be P3 or an evangelism bug instead, but marking P2 in order to get a better look, since YouTube is a major site.

It seems to me that the problem is that clicking the pause button leaves focus on that button and them pressing space bar both activates the button as if by mouse and the keyboard event is also interpreted by the page, so the keyboard action and the as-if mouse action cancel each other.

Comment 3

[Timea Zsoldos [:zstimi/tzsoldos], Desktop QA]

I reproduced this issue with the following steps: 
1. Launch Firefox and access: https://www.quirksmode.org/html5/videos/big_buck_bunny.webm
2. After the video started click on "Full screen" button.
3. Press space bar to pause the video.

[Expected result]:
The video playback should be paused.

[Actual result]:
The video is not paused, seems that the focus remain on "Full screen" button.

Comment 4

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Hi Mike, even after bug 1522089 was fixed, this issue is still valid. I wonder if this has anything to do with bug 1491256.

Comment 5

[Mike Taylor [:miketaylr]]

(In reply to Timea Zsoldos [:zstimi/tzsoldos], Desktop Release QA from comment #3)

I reproduced this issue with the following steps:

1. Launch Firefox and access:
   https://www.quirksmode.org/html5/videos/big_buck_bunny.webm
2. After the video started click on "Full screen" button.
3. Press space bar to pause the video.

[Expected result]:
The video playback should be paused.

[Actual result]:
The video is not paused, seems that the focus remain on "Full screen" button.

This sounds like a different bug, as it doesn't relate to the controls that YouTube uses (who knows, it might be related). Probably worth filing a new bug for this though.

(In reply to Hsin-Yi Tsai [:hsinyi] from comment #4)

Hi Mike, even after bug 1522089 was fixed, this issue is still valid. I wonder if this has anything to do with bug 1491256.

Tom, can you please look into this bug (and see also Bug 149125)?

Comment 6

[Thomas Wisniewski [:twisniewski]]

It looks as though when I double-click the video to fullscreen it, subsequent keypress events target the video element. But if I click the fullscreen icon, they target the body element instead. Indeed, if I add this to chrome://global/content/TopLevelVideoDocument.js, then the spacebar seems to work fine after I click the fullscreen icon:

  document.addEventListener("fullscreenchange", e => {
    console.log(e, e.target);
    videoElement.focus();
  });

So this does seem to be a bug in the default player.

Comment 7

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to Thomas Wisniewski [:twisniewski] from comment #6)

It looks as though when I double-click the video to fullscreen it, subsequent keypress events target the video element. But if I click the fullscreen icon, they target the body element instead. Indeed, if I add this to chrome://global/content/TopLevelVideoDocument.js, then the spacebar seems to work fine after I click the fullscreen icon:

  document.addEventListener("fullscreenchange", e => {
    console.log(e, e.target);
    videoElement.focus();
  });

So this does seem to be a bug in the default player.

Hi Thomas, thanks for the investigation. So sounds like we should report this back to youtube, am I right? Are you the right person to take the action?

Comment 8

[Thomas Wisniewski [:twisniewski]]

:hsinyi, actually, I was testing with our own player using the STR in comment 3, so it's our own player that has the bug there.

Unfortunately I cannot reproduce the issue on YouTube, so either that has been fixed or we still have another bug I cannot diagnose (similar to bug 1491256, which I also cannot reproduce).

Comment 9

[Mike Taylor [:miketaylr]]

I also can't reproduce this anymore -- it's likely YouTube fixed this. Let's go ahead and close as WORKSFORME.

Comment 10

[v1tesse]

I don't know if this issue has reappeared or it just hasn't gone away, but this still happens (Win 10 x64, FF 76.0.1). I hadn't noticed it since I almost never click the play/pause button when I can just click the video itself. It's probably down to the Youtube team constantly tinkering with things like what happened with bug 1628947.

Open a video, click on the play/pause button and press space. On the initial click, the video pauses, but then, when pressing space, it triggers space as play/pause + the button action, so play/pause again, thus doing nothing.

This works identically to bug 1491256, which I can still reproduce, but instead of the mute button being previously clicked = mute + play/pause - which has the same issue for me, it's the play button that was previously clicked = play/pause + play/pause.

Comment 11

[Timea Cernea [:tbabos][inactive]]

I can also reproduce this again on youtube using the same STR as in the bug description. Not sure if it was ever fixed or used to work at some point.
Reproduced on latest Nightly 78.0a1, Beta 77.0b9 and Release 76.0.1.

Comment 12

[Asa Dotzler [:asa]]

I can reproduce what I think is this bug with the latest 84 Nightly. Steps to reproduce are to focus the YouTube play/pause button and then hit the space bar to play or pause. Space seems to trigger twice causing the video to briefly play or pause and then return to its previous state.

Comment 13

[Asa Dotzler [:asa]]

Using K to play/pause works as expected, it's only the space bar that's affected.

Comment 14

[Chris Peterson [:cpeterson]]

Related webcompat bug: https://github.com/webcompat/web-bugs/issues/61530

Comment 15

[Dennis Schubert [:denschub]]

We have reached out to them in .. May 2019, and have not seen much movement. Google is tracking this as b/132981735, and I've just pinged a contact to see if we can get an update here.

Comment 16

[Dennis Schubert [:denschub]]

The update here is that they can reproduce, but are having a hard time figuring out why that even happens. It could be worthwhile for us to check if we can build a simple testcase with a video player and a custom pause/unpause button and see if we can get this to behave similarly and if there is any difference between browsers there.

Comment 18

[Emilio Cobos Álvarez (:emilio)]

Attachment: Reduced test-case.

So I dug a bit with rr and figured out what's going on.

The main issue is that preventDefault on keydown doesn't seem to prevent the button activation. On keyup it does though.

Do you know what the right behavior is here Masayuki?

Comment 19

[Emilio Cobos Álvarez (:emilio)]

Relevant Blink code: https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/html/html_element.cc;l=1612;drc=f28f8bf74a55624bef40febdfc89673a57e0eb6f

Comment 20

[Emilio Cobos Álvarez (:emilio)]

Inherited from WebKit: https://github.com/WebKit/WebKit/blob/15b4c9cf573a083fc838248277b19fa527a7ad6a/Source/WebCore/html/HTMLButtonElement.cpp#L172-L177

Comment 21

[Emilio Cobos Álvarez (:emilio)]

IE 11 behaves like WebKit, so I guess that's it since it's sorta legacy behavior. IE11-compatible test-case: https://crisal.io/tmp/player-play-pause-kb-reduced.html

Comment 22

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #18)

The main issue is that preventDefault on keydown doesn't seem to prevent the button activation. On keyup it does though.

Do you know what the right behavior is here Masayuki?

It's out of scope of any standards. Nothing defines what is default action(s) of keydown event, and how to move focus with events.

We do it at keyup event of space char input:
https://searchfox.org/mozilla-central/rev/1ba472c91bbde4b127f6e614ae72888ab59611cf/dom/html/HTMLButtonElement.cpp#239-242
https://searchfox.org/mozilla-central/rev/1ba472c91bbde4b127f6e614ae72888ab59611cf/dom/html/HTMLInputElement.cpp#3772-3773,3793-3794
but we don't check the active state of the element, and it seems that we only set active state only when mousedown.
https://searchfox.org/mozilla-central/search?q=symbol:_ZN7mozilla17EventStateManager16SetActiveManagerEPS0_P10nsIContent&redirect=false
So, it seems that we have different "active" state handling, and it's required for keyup handling of the space key.

Comment 23

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1481400 - Minor HTMLButtonElement cleanups. r=masayuki

Comment 24

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1481400 - Centralize keyboard activation code. r=masayuki

Depends on D116583

Comment 25

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1481400 - Don't dispatch a simulated click on keyup for default-prevented keydown. r=masayuki

I decided to go with a separate flag instead of reusing :active because
the :active handling WebKit uses for buttons is extremely weird: They
apply :active only to the button, not to the whole element chain like
mouse activeness does, and something like this keeps the :active state
in the button ~forever:

<!doctype html>
<style>
:active { outline: 2px solid red; }
</style>
<button onkeyup="return false">ABC</button>

Depends on D116584

Comment 26

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5175b724b484
Minor HTMLButtonElement cleanups. r=masayuki
https://hg.mozilla.org/integration/autoland/rev/75c26420524c
Centralize keyboard activation code. r=masayuki
https://hg.mozilla.org/integration/autoland/rev/0fdb0a9e8942
Don't dispatch a simulated click on keyup for default-prevented keydown. r=masayuki

Comment 27

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/52451918a4a2
More explicitly activate checkbox in browser_protocol_ask_dialog_permission.js.

Comment 28

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/699b45c4963a
focus-management-expectations.html is passing now.

Comment 29

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/5175b724b484
https://hg.mozilla.org/mozilla-central/rev/75c26420524c
https://hg.mozilla.org/mozilla-central/rev/0fdb0a9e8942
https://hg.mozilla.org/mozilla-central/rev/52451918a4a2
https://hg.mozilla.org/mozilla-central/rev/699b45c4963a

Comment 30

[Chris Peterson [:cpeterson]]

Verified fixed in Nightly 91.0a1 (2021-06-04)

Comment 31

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/29249 for changes under testing/web-platform/tests

Comment 32

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 33

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:emilio, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Comment 34

[Emilio Cobos Álvarez (:emilio)]

Fairly longstanding bug, so probably would benefit from Nightly bake-time.

Comment 35

[Olli Pettay [:smaug][[e-mail]]]

Am I missing something, or should https://searchfox.org/mozilla-central/rev/a1a27f5aa092d7a0b045d2258130532872d94810/dom/html/HTMLBRElement.h#17 have been updated when HTML_ELEMENT_FLAG_BIT was added?

Comment 36

[Emilio Cobos Álvarez (:emilio)]

Nope, you're right!