Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: volume key globalShortcut registration #23782

Merged
merged 1 commit into from
May 28, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
29 changes: 18 additions & 11 deletions patches/chromium/command-ismediakey.patch
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,8 @@ From: Jeremy Apthorp <jeremya@chromium.org>
Date: Wed, 10 Oct 2018 15:07:34 -0700
Subject: command-ismediakey.patch

Override MediaKeysListener::IsMediaKeycode to also listen for Volume Up, Volume Down,
and Mute. We also need to patch out Chromium's usage of RemoteCommandCenterDelegate, as
it uses MPRemoteCommandCenter. MPRemoteCommandCenter makes it such that GlobalShortcuts
in Electron will not work as intended, because by design an app does not receive remote
control events until it begins playing audio. This means that a media shortcut would not kick
into effect until you, for example, began playing a YouTube video which sort of defeats the
purpose of GlobalShortcuts.

At the moment there is no upstream possibility for this; but perhaps Chromium may
consider some kind of switch, enabled by default, which would conditionally choose to avoid usage of
RemoteCommandCenterDelegate on macOS.
Override MediaKeysListener::IsMediaKeycode and associated functions to also listen for
Volume Up, Volume Down, and Mute.

Also apply electron/electron@0f67b1866a9f00b852370e721affa4efda623f3a
and electron/electron@d2368d2d3b3de9eec4cc32b6aaf035cc89921bf1 as
Expand Down Expand Up @@ -95,3 +86,19 @@ index 85378bb565de617b1bd611d28c8714361747a357..36de4c0b0353be2418dacd388e92d7c3
return event;
}

diff --git a/ui/base/accelerators/system_media_controls_media_keys_listener.cc b/ui/base/accelerators/system_media_controls_media_keys_listener.cc
index 9d6084ceaccfd071549e63e3015f55ef292312ec..3f6af8b1b49bf0f226e9336c222884b07bf69e55 100644
--- a/ui/base/accelerators/system_media_controls_media_keys_listener.cc
+++ b/ui/base/accelerators/system_media_controls_media_keys_listener.cc
@@ -65,6 +65,11 @@ bool SystemMediaControlsMediaKeysListener::StartWatchingMediaKey(
case VKEY_MEDIA_STOP:
service_->SetIsStopEnabled(true);
break;
+ case VKEY_VOLUME_DOWN:
+ case VKEY_VOLUME_UP:
+ case VKEY_VOLUME_MUTE:
+ // Do nothing.
+ break;
default:
NOTREACHED();
}
6 changes: 4 additions & 2 deletions shell/browser/api/electron_api_global_shortcut.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@ bool RegisteringMediaKeyForUntrustedClient(const ui::Accelerator& accelerator) {
if (base::mac::IsAtLeastOS10_14()) {
constexpr ui::KeyboardCode mediaKeys[] = {
ui::VKEY_MEDIA_PLAY_PAUSE, ui::VKEY_MEDIA_NEXT_TRACK,
ui::VKEY_MEDIA_PREV_TRACK, ui::VKEY_MEDIA_STOP};
ui::VKEY_MEDIA_PREV_TRACK, ui::VKEY_MEDIA_STOP,
ui::VKEY_VOLUME_UP, ui::VKEY_VOLUME_DOWN,
ui::VKEY_VOLUME_MUTE};

if (std::find(std::begin(mediaKeys), std::end(mediaKeys),
accelerator.key_code()) != std::end(mediaKeys)) {
Expand Down Expand Up @@ -60,7 +62,7 @@ GlobalShortcut::~GlobalShortcut() {
void GlobalShortcut::OnKeyPressed(const ui::Accelerator& accelerator) {
if (accelerator_callback_map_.find(accelerator) ==
accelerator_callback_map_.end()) {
// This should never occur, because if it does, GlobalGlobalShortcutListener
// This should never occur, because if it does, GlobalShortcutListener
// notifies us with wrong accelerator.
NOTREACHED();
return;
Expand Down
17 changes: 17 additions & 0 deletions spec-main/api-global-shortcut-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,21 @@ ifdescribe(process.platform !== 'win32')('globalShortcut module', () => {
expect(globalShortcut.isRegistered(accelerators[0])).to.be.false('first unregistered');
expect(globalShortcut.isRegistered(accelerators[1])).to.be.false('second unregistered');
});

it('does not crash when registering media keys as global shortcuts', () => {
const accelerators = [
'VolumeUp',
'VolumeDown',
'VolumeMute',
'MediaNextTrack',
'MediaPreviousTrack',
'MediaStop', 'MediaPlayPause'
];

expect(() => {
globalShortcut.registerAll(accelerators, () => {});
}).to.not.throw();

globalShortcut.unregisterAll();
});
});