Bug 180033 - Pressing the space bar while watching a fullscreen video doesn't play or pause
Summary: Pressing the space bar while watching a fullscreen video doesn't play or pause
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: Safari Technology Preview
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Antoine Quint
URL:
Keywords: InRadar
Depends on: 180092
Blocks:
  Show dependency treegraph
 
Reported: 2017-11-27 06:11 PST by Antoine Quint
Modified: 2017-11-29 05:31 PST (History)
8 users (show)

See Also:


Attachments
Patch (8.39 KB, patch)
2017-11-27 06:33 PST, Antoine Quint
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews103 for mac-elcapitan (2.27 MB, application/zip)
2017-11-27 07:12 PST, EWS Watchlist
no flags Details
Patch (8.37 KB, patch)
2017-11-27 07:22 PST, Antoine Quint
ap: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Antoine Quint 2017-11-27 06:11:51 PST
Pressing the space bar while in fullscreen should toggle the media's playback state.
Comment 1 Antoine Quint 2017-11-27 06:12:01 PST
<rdar://problem/33610443>
Comment 2 Antoine Quint 2017-11-27 06:33:40 PST
Created attachment 327632 [details]
Patch
Comment 3 EWS Watchlist 2017-11-27 07:12:32 PST
Comment on attachment 327632 [details]
Patch

Attachment 327632 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/5374406

New failing tests:
media/modern-media-controls/media-controller/media-controller-space-bar-toggle-playback.html
Comment 4 EWS Watchlist 2017-11-27 07:12:33 PST
Created attachment 327633 [details]
Archive of layout-test-results from ews103 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 5 Antoine Quint 2017-11-27 07:22:03 PST
Created attachment 327634 [details]
Patch
Comment 6 Antoine Quint 2017-11-27 08:54:07 PST
Committed r225173: <https://trac.webkit.org/changeset/225173>
Comment 7 Matt Lewis 2017-11-27 14:28:59 PST
This caused a consistent timeout on macOS Sierra and High Sierra for the tests:

media/fullscreen-api-enabled-media-with-presentation-mode.html
media/fullscreen-video-going-into-pip.html

https://build.webkit.org/results/Apple%20Sierra%20Debug%20WK2%20(Tests)/r225179%20(4178)/results.html
https://build.webkit.org/builders/Apple%20Sierra%20Debug%20WK2%20(Tests)/builds/4178
Comment 8 Matt Lewis 2017-11-27 15:30:52 PST
Reverted r225173 for reason:

This caused multiple tests to timeout consistently.

Committed r225200: <https://trac.webkit.org/changeset/225200>
Comment 9 WebKit Commit Bot 2017-11-27 15:51:25 PST
Comment on attachment 327634 [details]
Patch

Clearing flags on attachment: 327634

Committed r225201: <https://trac.webkit.org/changeset/225201>
Comment 10 WebKit Commit Bot 2017-11-27 15:51:26 PST
All reviewed patches have been landed.  Closing bug.
Comment 11 Matt Lewis 2017-11-27 16:21:52 PST
It looks like the commit queue decided it really didn't want to let this patch go. Reverting again.
Comment 12 Matt Lewis 2017-11-27 16:22:56 PST
Reverted r225201 for reason:

This caused multiple tests to timeout consistently.

Committed r225204: <https://trac.webkit.org/changeset/225204>
Comment 13 Antoine Quint 2017-11-28 04:31:28 PST
Committed r225209: <https://trac.webkit.org/changeset/225209>
Comment 14 Ryan Haddad 2017-11-28 08:43:59 PST
(In reply to Antoine Quint from comment #13)
> Committed r225209: <https://trac.webkit.org/changeset/225209>
The two fullscreen tests that started timing out with the original commit are still timing out with this change:

https://build.webkit.org/builders/Apple%20Sierra%20Release%20WK2%20%28Tests%29/builds/5977
Comment 15 WebKit Commit Bot 2017-11-28 10:26:19 PST
Re-opened since this is blocked by bug 180092
Comment 16 Alexey Proskuryakov 2017-11-28 10:28:59 PST
Rolled back in r225222: <https://trac.webkit.org/changeset/225222>
Comment 17 Alexey Proskuryakov 2017-11-28 11:50:46 PST
Comment on attachment 327634 [details]
Patch

MArking r- as this breaks tests. Please don't re-land until you fix the problem, skipping them is not OK.
Comment 18 Antoine Quint 2017-11-29 05:28:55 PST
The tests that timed out were generating a keydown event with the key set to the space bar, which would cause media playback to be toggled when changing presentation mode. The fix is to change the key used in this scenario since it can be mostly any key.
Comment 19 Antoine Quint 2017-11-29 05:31:43 PST
Committed r225265: <https://trac.webkit.org/changeset/225265>