Skip to content

Commit f313955

Browse files
lijy91lijy91
committed
Fix macOS global shortcuts never firing
The macOS backend pumped Carbon events from a private background thread via ReceiveNextEvent(). That call drains the *calling* thread's event queue, and hotkey events are only ever posted to the main thread's queue, so the poller drained an always-empty queue and no shortcut ever fired. Registration still succeeded and IsSupported() still returned true, so the failure was completely silent: examples/shortcut_example could not respond to a keypress at all. Rework the backend along the lines of soffes/HotKey: one process-wide Carbon handler, one RegisterEventHotKey per shortcut, and an EventHotKeyID signature so the handler passes hotkeys owned by the host application through instead of swallowing them. The background thread is gone. InstallEventHandler/RegisterEventHotKey target GetApplicationEventTarget() rather than the reference's GetEventDispatcherTarget(), because the dispatcher target is per-thread while Register() is documented as callable from any thread. The main thread's dispatcher propagates to the application target, so delivery works either way. Something still has to pump the main-thread queue. A Cocoa app gets that from [NSApp run]; PlatformRunMainThreadLoopFor() now performs the same routing for programs without one, and continues to drain the GCD main queue that RunOnMainThread()/EmitAsync() depend on. Also widen the accelerator vocabulary to match the reference's Key enum. Punctuation, the keypad, and the Control/Option/Command modifier spellings were understood by every platform parser but rejected by IsValidAccelerator(), which gates Register() -- so Cmd+Comma and similar were unreachable regardless of backend support. Windows and Linux gain the same token tables so one accelerator string means the same key everywhere, and their F-key paths no longer hand unvalidated input to std::stoi. Covered by the new shortcut_accelerator_test.
1 parent 5f5e643 commit f313955

7 files changed

Lines changed: 517 additions & 294 deletions

File tree

‎src/platform/linux/shortcut_manager_linux.cpp‎

Lines changed: 69 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -53,9 +53,9 @@ bool ParseAcceleratorTokens(const std::string& accelerator,
5353

5454
for (auto& part : parts) {
5555
std::string token = ToLower(part);
56-
if (token == "ctrl" || token == "control" || token == "alt" || token == "shift" ||
57-
token == "cmd" || token == "command" || token == "super" || token == "meta" ||
58-
token == "cmdorctrl") {
56+
if (token == "ctrl" || token == "control" || token == "alt" || token == "option" ||
57+
token == "shift" || token == "cmd" || token == "command" || token == "super" ||
58+
token == "meta" || token == "cmdorctrl" || token == "commandorcontrol") {
5959
modifiers.push_back(token);
6060
} else {
6161
if (!key_token.empty()) {
@@ -68,7 +68,64 @@ bool ParseAcceleratorTokens(const std::string& accelerator,
6868
return !key_token.empty();
6969
}
7070

71+
// Token -> X11 KeySym.
72+
//
73+
// Mirrors the token set in src/shortcut_manager.cpp's validator and the tables
74+
// in the macOS/Windows backends, so the same accelerator string means the same
75+
// key on every platform.
7176
KeySym KeySymFromToken(const std::string& token) {
77+
static const std::unordered_map<std::string, KeySym> kKeySyms = {
78+
// Whitespace and editing.
79+
{"space", XK_space},
80+
{"tab", XK_Tab},
81+
{"enter", XK_Return},
82+
{"return", XK_Return},
83+
{"escape", XK_Escape},
84+
{"esc", XK_Escape},
85+
{"backspace", XK_BackSpace},
86+
{"delete", XK_Delete},
87+
{"forwarddelete", XK_Delete},
88+
{"insert", XK_Insert},
89+
{"help", XK_Help},
90+
91+
// Navigation.
92+
{"home", XK_Home},
93+
{"end", XK_End},
94+
{"pageup", XK_Page_Up},
95+
{"pagedown", XK_Page_Down},
96+
{"up", XK_Up},
97+
{"down", XK_Down},
98+
{"left", XK_Left},
99+
{"right", XK_Right},
100+
101+
// Punctuation, by name and by literal character.
102+
{"plus", XK_plus},
103+
{"equal", XK_equal}, {"=", XK_equal},
104+
{"minus", XK_minus}, {"-", XK_minus},
105+
{"comma", XK_comma}, {",", XK_comma},
106+
{"period", XK_period}, {".", XK_period},
107+
{"slash", XK_slash}, {"/", XK_slash},
108+
{"backslash", XK_backslash}, {"\\", XK_backslash},
109+
{"semicolon", XK_semicolon}, {";", XK_semicolon},
110+
{"quote", XK_apostrophe}, {"'", XK_apostrophe},
111+
{"leftbracket", XK_bracketleft}, {"[", XK_bracketleft},
112+
{"rightbracket", XK_bracketright}, {"]", XK_bracketright},
113+
{"grave", XK_grave}, {"backquote", XK_grave}, {"`", XK_grave},
114+
115+
// Keypad.
116+
{"num0", XK_KP_0}, {"num1", XK_KP_1}, {"num2", XK_KP_2},
117+
{"num3", XK_KP_3}, {"num4", XK_KP_4}, {"num5", XK_KP_5},
118+
{"num6", XK_KP_6}, {"num7", XK_KP_7}, {"num8", XK_KP_8},
119+
{"num9", XK_KP_9},
120+
{"numdec", XK_KP_Decimal},
121+
{"numadd", XK_KP_Add},
122+
{"numsub", XK_KP_Subtract},
123+
{"nummult", XK_KP_Multiply},
124+
{"numdiv", XK_KP_Divide},
125+
{"numenter", XK_KP_Enter},
126+
};
127+
128+
// Letters and digits resolve through Xlib's own name table.
72129
if (token.size() == 1) {
73130
char ch = token[0];
74131
if (std::isalpha(static_cast<unsigned char>(ch))) {
@@ -79,33 +136,18 @@ KeySym KeySymFromToken(const std::string& token) {
79136
}
80137
}
81138

82-
if (token.rfind("f", 0) == 0) {
139+
// Function keys. XK_F1..XK_F24 are contiguous.
140+
if (token.size() > 1 && token[0] == 'f' &&
141+
token.find_first_not_of("0123456789", 1) == std::string::npos) {
83142
int fnum = std::stoi(token.substr(1));
84143
if (fnum >= 1 && fnum <= 24) {
85144
return XK_F1 + (fnum - 1);
86145
}
146+
return NoSymbol;
87147
}
88148

89-
if (token == "space") return XK_space;
90-
if (token == "tab") return XK_Tab;
91-
if (token == "enter" || token == "return") return XK_Return;
92-
if (token == "escape" || token == "esc") return XK_Escape;
93-
if (token == "backspace") return XK_BackSpace;
94-
if (token == "delete") return XK_Delete;
95-
if (token == "insert") return XK_Insert;
96-
if (token == "home") return XK_Home;
97-
if (token == "end") return XK_End;
98-
if (token == "pageup") return XK_Page_Up;
99-
if (token == "pagedown") return XK_Page_Down;
100-
if (token == "up") return XK_Up;
101-
if (token == "down") return XK_Down;
102-
if (token == "left") return XK_Left;
103-
if (token == "right") return XK_Right;
104-
if (token == "plus") return XK_plus;
105-
if (token == "equal") return XK_equal;
106-
if (token == "minus") return XK_minus;
107-
108-
return NoSymbol;
149+
auto it = kKeySyms.find(token);
150+
return it == kKeySyms.end() ? NoSymbol : it->second;
109151
}
110152

111153
bool ParseAcceleratorLinux(const std::string& accelerator,
@@ -122,9 +164,10 @@ bool ParseAcceleratorLinux(const std::string& accelerator,
122164
}
123165

124166
for (const auto& token : modifier_tokens) {
125-
if (token == "ctrl" || token == "control" || token == "cmdorctrl") {
167+
if (token == "ctrl" || token == "control" || token == "cmdorctrl" ||
168+
token == "commandorcontrol") {
126169
modifiers |= ControlMask;
127-
} else if (token == "alt") {
170+
} else if (token == "alt" || token == "option") {
128171
modifiers |= Mod1Mask;
129172
} else if (token == "shift") {
130173
modifiers |= ShiftMask;

‎src/platform/macos/dispatcher_macos.mm‎

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
#include "../../foundation/dispatcher_platform.h"
22

3+
#import <Carbon/Carbon.h>
34
#import <CoreFoundation/CoreFoundation.h>
45
#import <Foundation/Foundation.h>
56
#include <dispatch/dispatch.h>
@@ -37,9 +38,34 @@ bool PlatformRunOnMainThread(std::function<void()> fn) {
3738
}
3839

3940
bool PlatformRunMainThreadLoopFor(int timeout_ms) {
40-
// Servicing the GCD main queue means running the main run loop; there is no
41-
// way to drain that queue without it.
42-
CFRunLoopRunInMode(kCFRunLoopDefaultMode, timeout_ms / 1000.0, false);
41+
// Two queues have to be serviced here, and only one call does both.
42+
//
43+
// The GCD main queue carries RunOnMainThread() work, and draining it means
44+
// running the main run loop. The Carbon event queue carries global hotkeys
45+
// (see shortcut_manager_macos.mm) and other OS events; a bare
46+
// CFRunLoopRunInMode() does *not* dispatch those, which is why a program
47+
// without a Cocoa run loop would register a shortcut successfully and then
48+
// never see it fire.
49+
//
50+
// ReceiveNextEvent() runs the main run loop internally — so it drains the
51+
// GCD main queue too — and additionally hands back the next OS event, which
52+
// we forward to the Carbon dispatcher. That is the same routing
53+
// `[NSApp sendEvent:]` performs in a Cocoa app; an app that has one keeps
54+
// using it and never calls this function.
55+
//
56+
// Must be called on the main thread: ReceiveNextEvent() drains the calling
57+
// thread's event queue, and OS events are only ever posted to the main one.
58+
if (!PlatformIsMainThread()) {
59+
return false;
60+
}
61+
62+
EventRef event = nullptr;
63+
const EventTimeout timeout = timeout_ms / 1000.0 * kEventDurationSecond;
64+
OSStatus status = ReceiveNextEvent(0, nullptr, timeout, true, &event);
65+
if (status == noErr && event) {
66+
SendEventToEventTarget(event, GetEventDispatcherTarget());
67+
ReleaseEvent(event);
68+
}
4369
return true;
4470
}
4571

0 commit comments

Comments
 (0)