GNOME Bugzilla – Bug 754096
space doesn't always toggle pause/play
Last modified: 2016-10-10 10:09:42 UTC
If next_button or prev_button is focused, pressing space will activate these buttons rather than pausing/playing music. Space bar is a shortcut for playing or pausing music in media players and pressing it should (IMO) consistently pause or play the music regardless of without activating what widget might be in focus. Steps to reproduce: * Play a song from an album * press Next * Press space
(note that I am not particular well knowledged on how accessibility and space bar works together).
Totem uses Ctrol + Space or P. The space bar is important for accessibility. See also: https://bugzilla.gnome.org/show_bug.cgi?id=758637
You really shouldn't be using unmodified shortcuts in a UI full of widgets. Totem gets away with it because that shortcut only works when the video view is visible. Eating Space unmodified will either not work properly, or break keyboard navigation (it's used to activate buttons, menus, etc.). Ctrl+Space (as rhythmbox does) or Ctrl+Return are fine alternatives. Also make sure that the "Play" keyboard key works.
Created attachment 324218 [details] [review] Use Ctrl+P instead of space as a play shortcut. From Bastien Nocera on Bugzilla: "You really shouldn't be using unmodified shortcuts in a UI full of widgets. Totem gets away with it because that shortcut only works when the video view is visible. Eating Space unmodified will either not work properly, or break keyboard navigation (it's used to activate buttons, menus, etc.)." Instead of space we're using the Ctrl+P combination.
Ctrl+Space is also a good shortcut.
Huh. I changed the patch to accept Ctrl+Space (as well as Ctrl+P) and it seems that both Ctrl+Space and Ctrl+Return are used to enter selection mode, depending on the active widget. I'm not really sure where that behavior is coming from though, any ideas?
Created attachment 324552 [details] [review] Use Ctrl+P and Ctrl+Space as play/pause shortcut. Use them instead of space. From Bastien Nocera on Bugzilla: "You really shouldn't be using unmodified shortcuts in a UI full of widgets. Totem gets away with it because that shortcut only works when the video view is visible. Eating Space unmodified will either not work properly, or break keyboard navigation (it's used to activate buttons, menus, etc.)."
Review of attachment 324552 [details] [review]: Sure.
Thanks for your patch. Attachment 324552 [details] pushed as 59d029d - Use Ctrl+P and Ctrl+Space as play/pause shortcut.
I missed the conflicts with selection-mode and the move-cursor signal for treeview, as pointed out to me by Gaurav Narula on IRC. So we should handle/discuss that before. I reverted the commit for now. b717c27
Created attachment 324582 [details] [review] Unbind Ctrl+space and Ctrl+P Builds on the patch by Evandro Giovanini. Unbinded Ctrl+Space and Ctrl+P on treeview and iconview. Also added a check to ensure currentTrack is not None before trying to toggle pause.
Review of attachment 324582 [details] [review]: Thanks for your patch, but I'm afraid we need to discuss this more further. Break the shortcut compatibility with other applications that have selection-mode is not desirable.
Would like to get this resolved before 3.22 .
@Gaurav Narula: Hi Gaurav. I would like to work on this bug.
@Yash: sure, feel free to work on this @Marinus, @Felipe: I'm helping Yash get started and thought this might be a good first bug. Could you guys suggest an approach that doesn't break compatibility with selection-mode?
The keyboard shortcuts popup window says Ctrl + Space (or ctrl + p) toggles play/pause, but currently, it's just Space (and ctrl + p) and Ctrl + Space does nothing related to play/pause.
The problem is we use some widgets that override space/ctrl+space and mess with our keybinds. It does work whenever you are not focused on any of these. Since ctrl+p is working as intended, maybe we should only advertise that for now. Until we solve the issues with ctrl+space.
(In reply to Marinus Schraal from comment #17) > Since ctrl+p is working as intended, maybe we should only advertise that for > now. Until we solve the issues with ctrl+space. I like that idea. In fact, that's what I did in the Ubuntu 16.10 Beta packaging.
(In reply to Jeremy Bicha from comment #18) > (In reply to Marinus Schraal from comment #17) > > Since ctrl+p is working as intended, maybe we should only advertise that for > > now. Until we solve the issues with ctrl+space. > > I like that idea. In fact, that's what I did in the Ubuntu 16.10 Beta > packaging. I removed the accelerator for now from the UI file, to avoid confusion. I would still like to have this tackled so we can bring it back.
It turns out we had some oldstyle css to unbind keys, fixing this resolved all issues afaics. So, I brought back the ctrl+space in the shortcuts window. Please test.
(In reply to Marinus Schraal from comment #20) > It turns out we had some oldstyle css to unbind keys, fixing this resolved > all issues afaics. So, I brought back the ctrl+space in the shortcuts window. > > Please test. Ctrl+space toggles play/pause now, but it also toggles selection mode for me at the same time
Created attachment 335366 [details] [review] shortcuts: remove Ctrl+P for Play/Pause
Review of attachment 335366 [details] [review]: lgtm
Comment on attachment 335366 [details] [review] shortcuts: remove Ctrl+P for Play/Pause After some discussion on irc decided to drop ctrl+p, as it was just a workaround for the difficulties with ctrl+space. Thanks for the quick patch.
(In reply to Bastian Ilsø from comment #21) > (In reply to Marinus Schraal from comment #20) > > It turns out we had some oldstyle css to unbind keys, fixing this resolved > > all issues afaics. So, I brought back the ctrl+space in the shortcuts window. > > > > Please test. > > Ctrl+space toggles play/pause now, but it also toggles selection mode for me > at the same time turns out i needed to force rebuild gnome-music, everything works as expected now. thanks!
Closing, available in 3.22.