Skip to content
Permalink
Browse files

fix: MediaKey globalShortcuts not working on macOS (#21442)

  • Loading branch information
codebytere authored and zcbenz committed Dec 11, 2019
1 parent 5cecc23 commit cbe1e3a1d091e210ccceae5c4c8b13877e1f3ab4
Showing with 53 additions and 39 deletions.
  1. +1 −0 buildflags/BUILD.gn
  2. +2 −0 buildflags/buildflags.gni
  3. +50 −39 patches/chromium/command-ismediakey.patch
@@ -21,6 +21,7 @@ buildflag_header("buildflags") {
"ENABLE_ELECTRON_EXTENSIONS=$enable_electron_extensions",
"ENABLE_BUILTIN_SPELLCHECKER=$enable_builtin_spellchecker",
"ENABLE_PICTURE_IN_PICTURE=$enable_picture_in_picture",
"ENABLE_MEDIA_KEY_OVERRIDES=$enable_media_key_overrides",
"OVERRIDE_LOCATION_PROVIDER=$enable_fake_location_provider",
]
}
@@ -22,6 +22,8 @@ declare_args() {

enable_picture_in_picture = true

enable_media_key_overrides = true

# Provide a fake location provider for mocking
# the geolocation responses. Disable it if you
# need to test with chromium's location provider.
@@ -3,48 +3,22 @@ From: Jeremy Apthorp <jeremya@chromium.org>
Date: Wed, 10 Oct 2018 15:07:34 -0700
Subject: command-ismediakey.patch

define Command::IsMediaKey on mac; copied from //chrome/common/extensions/command.cc,
which also defines a bunch of other stuff that depends on extensions.
since we only need IsMediaKey, and we don't want the extensions stuff
(and aren't compiling command.cc), it's safe to duplicate the
definition. A candidate for upstreaming would be to move the IsMediaKey
function into //ui.
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.

Also apply electron/electron@0f67b1866a9f00b852370e721affa4efda623f3a
and electron/electron@d2368d2d3b3de9eec4cc32b6aaf035cc89921bf1 as
patches.

diff --git a/chrome/browser/extensions/global_shortcut_listener_mac.mm b/chrome/browser/extensions/global_shortcut_listener_mac.mm
index befe726af9c10b1563a7fc0bb77cc55f65943d5c..46c6fe08bab8471007f78d3ef227e5195bfdf0e1 100644
--- a/chrome/browser/extensions/global_shortcut_listener_mac.mm
+++ b/chrome/browser/extensions/global_shortcut_listener_mac.mm
@@ -21,6 +21,26 @@

namespace extensions {

+// NOTE: this is defined in command.cc, but command.cc is full of
+// chrome-extensions-specific logic that we don't want to depend on.
+// Since we don't build command.cc in Electron, it's safe to re-define this
+// function here. Ideally, though, `IsMediaKey` would be the responsibility of
+// `ui::Accelerator`, rather than `extensions::Command`.
+
+// static
+bool Command::IsMediaKey(const ui::Accelerator& accelerator) {
+ if (accelerator.modifiers() != 0)
+ return false;
+
+ return (accelerator.key_code() == ui::VKEY_MEDIA_NEXT_TRACK ||
+ accelerator.key_code() == ui::VKEY_MEDIA_PREV_TRACK ||
+ accelerator.key_code() == ui::VKEY_MEDIA_PLAY_PAUSE ||
+ accelerator.key_code() == ui::VKEY_MEDIA_STOP ||
+ accelerator.key_code() == ui::VKEY_VOLUME_UP ||
+ accelerator.key_code() == ui::VKEY_VOLUME_DOWN ||
+ accelerator.key_code() == ui::VKEY_VOLUME_MUTE);
+}
+
// static
GlobalShortcutListener* GlobalShortcutListener::GetInstance() {
CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
diff --git a/chrome/browser/extensions/global_shortcut_listener_win.cc b/chrome/browser/extensions/global_shortcut_listener_win.cc
index c5125495b4d178ffb18be4d2d9670f7556412cbd..cddb321abb938c667a4a2089f87eab999510e9b1 100644
--- a/chrome/browser/extensions/global_shortcut_listener_win.cc
@@ -87,11 +61,33 @@ index 392cf3d58c64c088596e8d321a2ce37b0ec60b6e..43e30f47240dc10a3a9b950255d4e487

ui::Accelerator accelerator(
ui::KeyboardCodeFromXKeyEvent(x_event), modifiers);
diff --git a/ui/base/accelerators/media_keys_listener.cc b/ui/base/accelerators/media_keys_listener.cc
index 1145e1f3d79482b5bb468c3128431ac674310e5f..e9f595045e0c076e0735f27dfc38bfbc7951d372 100644
--- a/ui/base/accelerators/media_keys_listener.cc
+++ b/ui/base/accelerators/media_keys_listener.cc
@@ -13,7 +13,8 @@ MediaKeysListener::~MediaKeysListener() = default;
// static
bool MediaKeysListener::IsMediaKeycode(KeyboardCode key_code) {
return key_code == VKEY_MEDIA_PLAY_PAUSE || key_code == VKEY_MEDIA_STOP ||
- key_code == VKEY_MEDIA_PREV_TRACK || key_code == VKEY_MEDIA_NEXT_TRACK;
+ key_code == VKEY_MEDIA_PREV_TRACK || key_code == VKEY_MEDIA_NEXT_TRACK ||
+ key_code == VKEY_VOLUME_UP || key_code == VKEY_VOLUME_DOWN || key_code == VKEY_VOLUME_MUTE;
}

} // namespace ui
diff --git a/ui/base/accelerators/media_keys_listener_mac.mm b/ui/base/accelerators/media_keys_listener_mac.mm
index 85378bb565de617b1bd611d28c8714361747a357..36de4c0b0353be2418dacd388e92d7c38a7ee139 100644
index f4e3126a4efd66f05c4f13e40ba23db10b8cca96..9b4d1895731f758c09e5cb8350d1f225ab0f8a71 100644
--- a/ui/base/accelerators/media_keys_listener_mac.mm
+++ b/ui/base/accelerators/media_keys_listener_mac.mm
@@ -32,6 +32,12 @@ KeyboardCode MediaKeyCodeToKeyboardCode(int key_code) {
@@ -11,6 +11,7 @@
#include <IOKit/hidsystem/ev_keymap.h>

#include "base/containers/flat_set.h"
+#include "electron/buildflags/buildflags.h"
#include "ui/base/accelerators/accelerator.h"
#include "ui/base/accelerators/system_media_controls_media_keys_listener.h"

@@ -33,6 +33,12 @@ KeyboardCode MediaKeyCodeToKeyboardCode(int key_code) {
case NX_KEYTYPE_NEXT:
case NX_KEYTYPE_FAST:
return VKEY_MEDIA_NEXT_TRACK;
@@ -116,3 +112,18 @@ index 85378bb565de617b1bd611d28c8714361747a357..36de4c0b0353be2418dacd388e92d7c3
return event;
}

@@ -223,12 +223,14 @@ static CGEventRef EventTapCallback(CGEventTapProxy proxy,
// For Mac OS 10.12.2 or later, we want to use MPRemoteCommandCenter for
// getting media keys globally if there is a RemoteCommandCenterDelegate
// available.
+#if !BUILDFLAG(ENABLE_MEDIA_KEY_OVERRIDES)
if (scope == Scope::kGlobal) {
auto listener =
std::make_unique<SystemMediaControlsMediaKeysListener>(delegate);
if (listener->Initialize())
return listener;
}
+#endif

return std::make_unique<MediaKeysListenerImpl>(delegate, scope);
}

0 comments on commit cbe1e3a

Please sign in to comment.
You can’t perform that action at this time.