authorgravatar for saphid@gmail.comAlex Southwell <saphid@gmail.com> 2026-09-02 14:19:00+10:00
committergravatar for noreply@github.comGitHub <noreply@github.com> 2026-09-01 21:19:00-07:00
logb8262b41228e03b2e1e14b701be449034349c8ba
tree615055127f900902c7eacb6c81c287ee227c847f
parentcde12790d721ab2a90471750fd75d9f5bf13f57d
signature Signed by PGP key B5690EEEBB952194

fix(desktop): hold-to-quit no longer gets stuck (#9141)

4 files changed, 159 insertions(+), 8 deletions(-)

apps/desktop/src/window/DesktopWindow.test.ts+23
...@@ -105,6 +105,8 @@ function makeFakeBrowserWindow() {...@@ -105,6 +105,8 @@ function makeFakeBrowserWindow() {
105 restore: vi.fn(),105 restore: vi.fn(),
106 setBackgroundColor: vi.fn(),106 setBackgroundColor: vi.fn(),
107 setAutoHideCursor: vi.fn(),107 setAutoHideCursor: vi.fn(),
108 setFullScreen: vi.fn(),
109 setOpacity: vi.fn(),
108 setTitle: vi.fn(),110 setTitle: vi.fn(),
109 setTitleBarOverlay: vi.fn(),111 setTitleBarOverlay: vi.fn(),
110 show: vi.fn(),112 show: vi.fn(),
...@@ -127,6 +129,8 @@ function makeFakeBrowserWindow() {...@@ -127,6 +129,8 @@ function makeFakeBrowserWindow() {
127 setZoomLevel: webContents.setZoomLevel,129 setZoomLevel: webContents.setZoomLevel,
128 setBackgroundThrottling: webContents.setBackgroundThrottling,130 setBackgroundThrottling: webContents.setBackgroundThrottling,
129 setAutoHideCursor: window.setAutoHideCursor,131 setAutoHideCursor: window.setAutoHideCursor,
132 setFullScreen: window.setFullScreen,
133 setOpacity: window.setOpacity,
130 webContentsListeners,134 webContentsListeners,
131 windowListeners,135 windowListeners,
132 };136 };
...@@ -394,6 +398,25 @@ const makeSplashScenario = (createOutcomes: readonly (Electron.BrowserWindow | n...@@ -394,6 +398,25 @@ const makeSplashScenario = (createOutcomes: readonly (Electron.BrowserWindow | n
394 });398 });
395399
396describe("DesktopWindow", () => {400describe("DesktopWindow", () => {
401 it("leaves fullscreen before concealing a pending quit", () => {
402 const fakeWindow = makeFakeBrowserWindow();
403
404 DesktopWindow.concealPendingQuitWindow(fakeWindow.window);
405 assert.deepEqual(fakeWindow.setOpacity.mock.calls, [[0]]);
406
407 fakeWindow.setOpacity.mockClear();
408 fakeWindow.isFullScreen.mockReturnValue(true);
409 DesktopWindow.concealPendingQuitWindow(fakeWindow.window);
410 assert.deepEqual(fakeWindow.setFullScreen.mock.calls, [[false]]);
411 assert.deepEqual(fakeWindow.setOpacity.mock.calls, [[0]]);
412
413 fakeWindow.setOpacity.mockClear();
414 fakeWindow.isFullScreen.mockReturnValue(false);
415 fakeWindow.isDestroyed.mockReturnValue(true);
416 DesktopWindow.concealPendingQuitWindow(fakeWindow.window);
417 assert.equal(fakeWindow.setOpacity.mock.calls.length, 0);
418 });
419
397 it("restores bounds only when the window fits within a connected display", () => {420 it("restores bounds only when the window fits within a connected display", () => {
398 const persistedBounds = { x: 2040, y: 80, width: 1320, height: 880 };421 const persistedBounds = { x: 2040, y: 80, width: 1320, height: 880 };
399 const displays = [422 const displays = [
apps/desktop/src/window/DesktopWindow.ts+18
...@@ -207,6 +207,21 @@ export function isRetryableDevelopmentRendererLoadFailure(input: {...@@ -207,6 +207,21 @@ export function isRetryableDevelopmentRendererLoadFailure(input: {
207 );207 );
208}208}
209209
210export function concealPendingQuitWindow(
211 window: Pick<
212 Electron.BrowserWindow,
213 "isDestroyed" | "isFullScreen" | "setFullScreen" | "setOpacity"
214 >,
215): void {
216 if (window.isDestroyed()) return;
217 if (window.isFullScreen()) {
218 window.setFullScreen(false);
219 }
220 // Electron implements window opacity on macOS and Windows. Linux keeps the
221 // release-gated quit behavior but cannot make the pending window disappear.
222 window.setOpacity(0);
223}
224
210function getWindowTitleBarOptions(225function getWindowTitleBarOptions(
211 shouldUseDarkColors: boolean,226 shouldUseDarkColors: boolean,
212 platform: NodeJS.Platform,227 platform: NodeJS.Platform,
...@@ -570,6 +585,9 @@ export const make = Effect.gen(function* () {...@@ -570,6 +585,9 @@ export const make = Effect.gen(function* () {
570 window.webContents.send(QUIT_SHORTCUT_CHANNEL, hint);585 window.webContents.send(QUIT_SHORTCUT_CHANNEL, hint);
571 }586 }
572 },587 },
588 // Keep the transparent window focused until the physical shortcut is
589 // released so its remaining repeats cannot reach the next app.
590 concealWindow: () => concealPendingQuitWindow(window),
573 quit: () => {591 quit: () => {
574 void runPromise(electronApp.quit);592 void runPromise(electronApp.quit);
575 },593 },
apps/desktop/src/window/QuitHold.test.ts+83-2
...@@ -32,11 +32,13 @@ function makeHarness(options?: {...@@ -32,11 +32,13 @@ function makeHarness(options?: {
32 getMode?: () => Promise<QuitConfirmationMode>;32 getMode?: () => Promise<QuitConfirmationMode>;
33}) {33}) {
34 const notifications: Array<QuitShortcutHintEvent> = [];34 const notifications: Array<QuitShortcutHintEvent> = [];
35 const concealWindow = vi.fn();
35 const quit = vi.fn();36 const quit = vi.fn();
36 const handler = makeQuitShortcutHandler({37 const handler = makeQuitShortcutHandler({
37 platform: options?.platform ?? "darwin",38 platform: options?.platform ?? "darwin",
38 getMode: options?.getMode ?? (() => Promise.resolve(options?.mode ?? "hold")),39 getMode: options?.getMode ?? (() => Promise.resolve(options?.mode ?? "hold")),
39 notify: (event) => notifications.push(event),40 notify: (event) => notifications.push(event),
41 concealWindow,
40 quit,42 quit,
41 });43 });
42 const preventDefault = vi.fn();44 const preventDefault = vi.fn();
...@@ -57,7 +59,7 @@ function makeHarness(options?: {...@@ -57,7 +59,7 @@ function makeHarness(options?: {
57 await send(makeInput({ isAutoRepeat: true, ...repeatOverrides }));59 await send(makeInput({ isAutoRepeat: true, ...repeatOverrides }));
58 }60 }
59 };61 };
60 return { notifications, quit, preventDefault, send, holdFor };62 return { notifications, concealWindow, quit, preventDefault, send, holdFor };
61}63}
6264
63describe("makeQuitShortcutHandler", () => {65describe("makeQuitShortcutHandler", () => {
...@@ -82,10 +84,11 @@ describe("makeQuitShortcutHandler", () => {...@@ -82,10 +84,11 @@ describe("makeQuitShortcutHandler", () => {
82 expect(harness.notifications).toEqual([HOLD_DOWN, UP]);84 expect(harness.notifications).toEqual([HOLD_DOWN, UP]);
83 });85 });
8486
85 it("quits after a completed hold is released", async () => {87 it("conceals a completed hold, then quits after release", async () => {
86 const harness = makeHarness();88 const harness = makeHarness();
87 await harness.send(makeInput({}));89 await harness.send(makeInput({}));
88 await harness.holdFor(QUIT_HOLD_DURATION_MS + 200);90 await harness.holdFor(QUIT_HOLD_DURATION_MS + 200);
91 expect(harness.concealWindow).toHaveBeenCalledTimes(1);
89 expect(harness.quit).not.toHaveBeenCalled();92 expect(harness.quit).not.toHaveBeenCalled();
90 await harness.send(makeInput({ type: "keyUp", key: "Meta", meta: false }));93 await harness.send(makeInput({ type: "keyUp", key: "Meta", meta: false }));
91 expect(harness.quit).not.toHaveBeenCalled();94 expect(harness.quit).not.toHaveBeenCalled();
...@@ -94,6 +97,81 @@ describe("makeQuitShortcutHandler", () => {...@@ -94,6 +97,81 @@ describe("makeQuitShortcutHandler", () => {
94 expect(harness.notifications).toEqual([HOLD_DOWN, UP]);97 expect(harness.notifications).toEqual([HOLD_DOWN, UP]);
95 });98 });
9699
100 it("keeps a concealed hold committed when another key is pressed", async () => {
101 const harness = makeHarness();
102 await harness.send(makeInput({}));
103 await harness.holdFor(QUIT_HOLD_DURATION_MS);
104
105 await harness.send(makeInput({ key: "Shift", shift: true }));
106 expect(harness.concealWindow).toHaveBeenCalledTimes(1);
107 expect(harness.quit).not.toHaveBeenCalled();
108
109 vi.advanceTimersByTime(QUIT_HOLD_RELEASE_GRACE_MS);
110 expect(harness.quit).toHaveBeenCalledTimes(1);
111 });
112
113 it("keeps a concealed hold committed through a fresh Cmd+Q press", async () => {
114 const harness = makeHarness();
115 await harness.send(makeInput({}));
116 await harness.holdFor(QUIT_HOLD_DURATION_MS);
117
118 await harness.send(makeInput({}));
119 expect(harness.concealWindow).toHaveBeenCalledTimes(1);
120 expect(harness.quit).not.toHaveBeenCalled();
121
122 await harness.send(makeInput({ type: "keyUp" }));
123 expect(harness.quit).toHaveBeenCalledTimes(1);
124 });
125
126 it("quits when a completed hold goes quiet without release events", async () => {
127 const harness = makeHarness();
128 await harness.send(makeInput({}));
129 await harness.holdFor(QUIT_HOLD_DURATION_MS + QUIT_HOLD_RELEASE_GRACE_MS * 2);
130
131 // If neither keyUp reaches the handler, continued repeats must keep the
132 // app alive. Once they stop, the quiet period is the release signal.
133 expect(harness.quit).not.toHaveBeenCalled();
134 vi.advanceTimersByTime(QUIT_HOLD_RELEASE_GRACE_MS);
135
136 expect(harness.quit).toHaveBeenCalledTimes(1);
137 expect(harness.notifications).toEqual([HOLD_DOWN, UP]);
138 });
139
140 it("waits for slow repeats to stop before quitting", async () => {
141 const harness = makeHarness();
142 await harness.send(makeInput({}));
143
144 vi.advanceTimersByTime(300);
145 await harness.send(makeInput({ isAutoRepeat: true }));
146 vi.advanceTimersByTime(900);
147 await harness.send(makeInput({ isAutoRepeat: true }));
148
149 vi.advanceTimersByTime(QUIT_HOLD_RELEASE_GRACE_MS);
150 expect(harness.quit).not.toHaveBeenCalled();
151
152 vi.advanceTimersByTime(300);
153 await harness.send(makeInput({ isAutoRepeat: true }));
154 vi.advanceTimersByTime(1_799);
155 expect(harness.quit).not.toHaveBeenCalled();
156 vi.advanceTimersByTime(1);
157 expect(harness.quit).toHaveBeenCalledTimes(1);
158 });
159
160 it("uses the initial repeat delay when the first repeat completes the hold", async () => {
161 const harness = makeHarness();
162 await harness.send(makeInput({}));
163
164 vi.advanceTimersByTime(QUIT_HOLD_DURATION_MS + 100);
165 await harness.send(makeInput({ isAutoRepeat: true }));
166
167 vi.advanceTimersByTime(QUIT_HOLD_RELEASE_GRACE_MS);
168 expect(harness.quit).not.toHaveBeenCalled();
169 vi.advanceTimersByTime(1_999);
170 expect(harness.quit).not.toHaveBeenCalled();
171 vi.advanceTimersByTime(1);
172 expect(harness.quit).toHaveBeenCalledTimes(1);
173 });
174
97 it("waits for Q release when Cmd is released first", async () => {175 it("waits for Q release when Cmd is released first", async () => {
98 const harness = makeHarness();176 const harness = makeHarness();
99 await harness.send(makeInput({}));177 await harness.send(makeInput({}));
...@@ -115,6 +193,7 @@ describe("makeQuitShortcutHandler", () => {...@@ -115,6 +193,7 @@ describe("makeQuitShortcutHandler", () => {
115 await harness.send(makeInput({ type: "keyUp" }));193 await harness.send(makeInput({ type: "keyUp" }));
116 expect(harness.notifications).toEqual([HOLD_DOWN, UP]);194 expect(harness.notifications).toEqual([HOLD_DOWN, UP]);
117 vi.advanceTimersByTime((QUIT_HOLD_DURATION_MS + QUIT_HOLD_RELEASE_GRACE_MS) * 2);195 vi.advanceTimersByTime((QUIT_HOLD_DURATION_MS + QUIT_HOLD_RELEASE_GRACE_MS) * 2);
196 expect(harness.concealWindow).not.toHaveBeenCalled();
118 expect(harness.quit).not.toHaveBeenCalled();197 expect(harness.quit).not.toHaveBeenCalled();
119 });198 });
120199
...@@ -130,6 +209,7 @@ describe("makeQuitShortcutHandler", () => {...@@ -130,6 +209,7 @@ describe("makeQuitShortcutHandler", () => {
130 it("quits without showing a hint in direct mode", async () => {209 it("quits without showing a hint in direct mode", async () => {
131 const harness = makeHarness({ mode: "direct" });210 const harness = makeHarness({ mode: "direct" });
132 await harness.send(makeInput({}));211 await harness.send(makeInput({}));
212 expect(harness.concealWindow).not.toHaveBeenCalled();
133 expect(harness.quit).toHaveBeenCalledTimes(1);213 expect(harness.quit).toHaveBeenCalledTimes(1);
134 expect(harness.notifications).toEqual([]);214 expect(harness.notifications).toEqual([]);
135 });215 });
...@@ -223,6 +303,7 @@ describe("makeQuitShortcutHandler", () => {...@@ -223,6 +303,7 @@ describe("makeQuitShortcutHandler", () => {
223 await harness.send(makeInput({}));303 await harness.send(makeInput({}));
224 vi.advanceTimersByTime(QUIT_DOUBLE_PRESS_MS - 100);304 vi.advanceTimersByTime(QUIT_DOUBLE_PRESS_MS - 100);
225 await harness.send(makeInput({}));305 await harness.send(makeInput({}));
306 expect(harness.concealWindow).not.toHaveBeenCalled();
226 expect(harness.quit).toHaveBeenCalledTimes(1);307 expect(harness.quit).toHaveBeenCalledTimes(1);
227 expect(harness.notifications).toEqual([DOUBLE_CLICK_DOWN, UP]);308 expect(harness.notifications).toEqual([DOUBLE_CLICK_DOWN, UP]);
228 });309 });
apps/desktop/src/window/QuitHold.ts+35-6
...@@ -11,9 +11,12 @@ export const QUIT_DOUBLE_PRESS_MS = 500;...@@ -11,9 +11,12 @@ export const QUIT_DOUBLE_PRESS_MS = 500;
11// release: macOS suppresses a letter keyUp while the command key is down, so a11// release: macOS suppresses a letter keyUp while the command key is down, so a
12// tap release can go completely unseen and a release-based timer would quit12// tap release can go completely unseen and a release-based timer would quit
13// anyway. Once held, quitting waits for Q keyUp or a quiet grace period after13// anyway. Once held, quitting waits for Q keyUp or a quiet grace period after
14// modifier keyUp so repeats cannot reach the next app. Keyboards with14// repeats stop so they cannot reach the next app. Keyboards with
15// auto-repeat disabled fall back to the application menu Quit action.15// auto-repeat disabled fall back to the application menu Quit action.
16export const QUIT_HOLD_RELEASE_GRACE_MS = 600;16export const QUIT_HOLD_RELEASE_GRACE_MS = 600;
17// A slow repeat rate can exceed the fixed grace. Waiting for two observed
18// cadences keeps the timer behind the next repeat without slowing normal rates.
19const QUIT_HOLD_REPEAT_CADENCE_MULTIPLIER = 2;
1720
18export interface QuitHoldKeyInput {21export interface QuitHoldKeyInput {
19 readonly type: string;22 readonly type: string;
...@@ -29,6 +32,7 @@ export interface QuitShortcutOptions {...@@ -29,6 +32,7 @@ export interface QuitShortcutOptions {
29 readonly platform: NodeJS.Platform;32 readonly platform: NodeJS.Platform;
30 readonly getMode: () => Promise<QuitConfirmationMode>;33 readonly getMode: () => Promise<QuitConfirmationMode>;
31 readonly notify: (event: QuitShortcutHintEvent) => void;34 readonly notify: (event: QuitShortcutHintEvent) => void;
35 readonly concealWindow: () => void;
32 readonly quit: () => void;36 readonly quit: () => void;
33}37}
3438
...@@ -45,6 +49,8 @@ export function makeQuitShortcutHandler(...@@ -45,6 +49,8 @@ export function makeQuitShortcutHandler(
45 let quitOnRelease = false;49 let quitOnRelease = false;
46 let heldSince = 0;50 let heldSince = 0;
47 let lastPressAt = 0;51 let lastPressAt = 0;
52 let lastRepeatAt = 0;
53 let repeatCadenceMs = 0;
48 // Incremented when a press is superseded or explicitly cancelled. A plain54 // Incremented when a press is superseded or explicitly cancelled. A plain
49 // key release does not invalidate its pending mode read: direct mode and a55 // key release does not invalidate its pending mode read: direct mode and a
50 // completed second press must still be honored after that read settles.56 // completed second press must still be honored after that read settles.
...@@ -64,6 +70,8 @@ export function makeQuitShortcutHandler(...@@ -64,6 +70,8 @@ export function makeQuitShortcutHandler(
64 holding = false;70 holding = false;
65 armed = false;71 armed = false;
66 quitOnRelease = false;72 quitOnRelease = false;
73 lastRepeatAt = 0;
74 repeatCadenceMs = 0;
67 if (keepHint) return;75 if (keepHint) return;
6876
69 mode = undefined;77 mode = undefined;
...@@ -80,6 +88,15 @@ export function makeQuitShortcutHandler(...@@ -80,6 +88,15 @@ export function makeQuitShortcutHandler(
80 options.quit();88 options.quit();
81 };89 };
8290
91 const quitAfterQuietPeriod = () => {
92 clearWatchdog();
93 const quietPeriodMs = Math.max(
94 QUIT_HOLD_RELEASE_GRACE_MS,
95 repeatCadenceMs * QUIT_HOLD_REPEAT_CADENCE_MULTIPLIER,
96 );
97 watchdog = setTimeout(quitNow, quietPeriodMs);
98 };
99
83 return (event, input) => {100 return (event, input) => {
84 const key = input.key.toLowerCase();101 const key = input.key.toLowerCase();
85 if (input.type === "keyUp") {102 if (input.type === "keyUp") {
...@@ -91,20 +108,31 @@ export function makeQuitShortcutHandler(...@@ -91,20 +108,31 @@ export function makeQuitShortcutHandler(
91 if (!quitOnRelease) {108 if (!quitOnRelease) {
92 release(false, true);109 release(false, true);
93 } else {110 } else {
94 watchdog = setTimeout(quitNow, QUIT_HOLD_RELEASE_GRACE_MS);111 quitAfterQuietPeriod();
95 }112 }
96 }113 }
97 return;114 return;
98 }115 }
99 if (input.type !== "keyDown") return;116 if (input.type !== "keyDown") return;
100117
101 if (quitOnRelease && input.isAutoRepeat && key === "q") {118 const modifierDown = options.platform === "darwin" ? input.meta : input.control;
119 if (input.isAutoRepeat && modifierDown && key === "q") {
120 const now = Date.now();
121 repeatCadenceMs = now - (lastRepeatAt === 0 ? heldSince : lastRepeatAt);
122 lastRepeatAt = now;
123 }
124 if (quitOnRelease) {
102 event.preventDefault();125 event.preventDefault();
103 clearWatchdog();126 if (key === "q") {
127 if (modifierDown) {
128 quitAfterQuietPeriod();
129 } else {
130 clearWatchdog();
131 }
132 }
104 return;133 return;
105 }134 }
106135
107 const modifierDown = options.platform === "darwin" ? input.meta : input.control;
108 if (!modifierDown || input.alt || input.shift || key !== "q") {136 if (!modifierDown || input.alt || input.shift || key !== "q") {
109 // Re-pressing the platform modifier is the first half of a second full137 // Re-pressing the platform modifier is the first half of a second full
110 // quit shortcut, so it must not cancel an active double-press window.138 // quit shortcut, so it must not cancel an active double-press window.
...@@ -129,7 +157,8 @@ export function makeQuitShortcutHandler(...@@ -129,7 +157,8 @@ export function makeQuitShortcutHandler(
129 if (mode === "hold" && armed && Date.now() - heldSince >= QUIT_HOLD_DURATION_MS) {157 if (mode === "hold" && armed && Date.now() - heldSince >= QUIT_HOLD_DURATION_MS) {
130 armed = false;158 armed = false;
131 quitOnRelease = true;159 quitOnRelease = true;
132 clearWatchdog();160 options.concealWindow();
161 quitAfterQuietPeriod();
133 }162 }
134 return;163 return;
135 }164 }