authorgravatar for 62337003+Bil0000@users.noreply.github.comBilal Bakr <62337003+Bil0000@users.noreply.github.com> 2026-08-18 20:41:07+03:00
committergravatar for noreply@github.comGitHub <noreply@github.com> 2026-08-18 13:41:07-04:00
logf3cb7f509595d304c6e2ccc7afe05c95ec45a7d8
tree404ea3ae1827fff593be8fe92aa306c9291749be
parenta87f691bd40c9e3c87715e3ce99ab39a584bdd1e
signature Signed by PGP key B5690EEEBB952194

fix(desktop): prevent quit shortcut spillover (#7397)

2 files changed, 57 insertions(+), 19 deletions(-)

apps/desktop/src/window/QuitHold.test.ts+23-7
...@@ -77,17 +77,32 @@ describe("makeQuitHoldHandler", () => {...@@ -77,17 +77,32 @@ describe("makeQuitHoldHandler", () => {
77 expect(harness.notifications).toEqual(["down", "up"]);77 expect(harness.notifications).toEqual(["down", "up"]);
78 });78 });
7979
80 it("quits once the shortcut auto-repeats past the hold duration", async () => {80 it("quits after a completed hold is released", async () => {
81 const harness = makeHarness();81 const harness = makeHarness();
82 await harness.send(makeInput({}));82 await harness.send(makeInput({}));
83 await harness.holdFor(QUIT_HOLD_DURATION_MS - 200);83 await harness.holdFor(QUIT_HOLD_DURATION_MS + 200);
84 expect(harness.quit).not.toHaveBeenCalled();
85 await harness.send(makeInput({ type: "keyUp", key: "Meta", meta: false }));
84 expect(harness.quit).not.toHaveBeenCalled();86 expect(harness.quit).not.toHaveBeenCalled();
85 await harness.holdFor(400);87 vi.advanceTimersByTime(QUIT_HOLD_RELEASE_GRACE_MS);
86 expect(harness.quit).toHaveBeenCalledTimes(1);88 expect(harness.quit).toHaveBeenCalledTimes(1);
87 // Exactly one hint cycle for the whole hold.
88 expect(harness.notifications).toEqual(["down", "up"]);89 expect(harness.notifications).toEqual(["down", "up"]);
89 });90 });
9091
92 it("waits for Q release when Cmd is released first", async () => {
93 const harness = makeHarness();
94 await harness.send(makeInput({}));
95 await harness.holdFor(QUIT_HOLD_DURATION_MS + 200);
96 await harness.send(makeInput({ type: "keyUp", key: "Meta", meta: false }));
97 harness.preventDefault.mockClear();
98 await harness.send(makeInput({ meta: false, isAutoRepeat: true }));
99 expect(harness.preventDefault).toHaveBeenCalledTimes(1);
100 vi.advanceTimersByTime(QUIT_HOLD_RELEASE_GRACE_MS * 2);
101 expect(harness.quit).not.toHaveBeenCalled();
102 await harness.send(makeInput({ type: "keyUp", meta: false }));
103 expect(harness.quit).toHaveBeenCalledTimes(1);
104 });
105
91 it("does not quit when the hold stops before the duration", async () => {106 it("does not quit when the hold stops before the duration", async () => {
92 const harness = makeHarness();107 const harness = makeHarness();
93 await harness.send(makeInput({}));108 await harness.send(makeInput({}));
...@@ -107,12 +122,11 @@ describe("makeQuitHoldHandler", () => {...@@ -107,12 +122,11 @@ describe("makeQuitHoldHandler", () => {
107 expect(harness.quit).not.toHaveBeenCalled();122 expect(harness.quit).not.toHaveBeenCalled();
108 });123 });
109124
110 it("quits immediately on a single press when disabled", async () => {125 it("quits without showing a hint when hold-to-quit is disabled", async () => {
111 const harness = makeHarness({ enabled: false });126 const harness = makeHarness({ enabled: false });
112 await harness.send(makeInput({}));127 await harness.send(makeInput({}));
113 expect(harness.quit).toHaveBeenCalledTimes(1);128 expect(harness.quit).toHaveBeenCalledTimes(1);
114 // The hint is dismissed in case the quit gets cancelled downstream.129 expect(harness.notifications).toEqual([]);
115 expect(harness.notifications).toEqual(["down", "up"]);
116 });130 });
117131
118 it("discards a stale isEnabled resolution from a superseded press", async () => {132 it("discards a stale isEnabled resolution from a superseded press", async () => {
...@@ -138,6 +152,7 @@ describe("makeQuitHoldHandler", () => {...@@ -138,6 +152,7 @@ describe("makeQuitHoldHandler", () => {
138 // Press #2 resolves enabled and completes a full hold.152 // Press #2 resolves enabled and completes a full hold.
139 resolvers[1]?.(true);153 resolvers[1]?.(true);
140 await harness.holdFor(QUIT_HOLD_DURATION_MS + 200);154 await harness.holdFor(QUIT_HOLD_DURATION_MS + 200);
155 await harness.send(makeInput({ type: "keyUp" }));
141 expect(harness.quit).toHaveBeenCalledTimes(1);156 expect(harness.quit).toHaveBeenCalledTimes(1);
142 });157 });
143158
...@@ -196,6 +211,7 @@ describe("makeQuitHoldHandler", () => {...@@ -196,6 +211,7 @@ describe("makeQuitHoldHandler", () => {
196 await harness.send(makeInput({ meta: false, control: true }));211 await harness.send(makeInput({ meta: false, control: true }));
197 expect(harness.preventDefault).toHaveBeenCalledTimes(1);212 expect(harness.preventDefault).toHaveBeenCalledTimes(1);
198 await harness.holdFor(QUIT_HOLD_DURATION_MS + 200, { meta: false, control: true });213 await harness.holdFor(QUIT_HOLD_DURATION_MS + 200, { meta: false, control: true });
214 await harness.send(makeInput({ type: "keyUp", meta: false, control: true }));
199 expect(harness.quit).toHaveBeenCalledTimes(1);215 expect(harness.quit).toHaveBeenCalledTimes(1);
200 });216 });
201});217});
apps/desktop/src/window/QuitHold.ts+34-12
...@@ -2,7 +2,8 @@...@@ -2,7 +2,8 @@
22
3// Chrome-style hold-to-quit. The quit accelerator is intercepted in3// Chrome-style hold-to-quit. The quit accelerator is intercepted in
4// before-input-event (which runs before the native menu accelerator), and the4// before-input-event (which runs before the native menu accelerator), and the
5// app only quits once the shortcut has been held for QUIT_HOLD_DURATION_MS.5// app only quits after the shortcut has been held for QUIT_HOLD_DURATION_MS
6// and released.
6// A quick tap just shows the renderer's "Hold to Quit" hint, and a second tap7// A quick tap just shows the renderer's "Hold to Quit" hint, and a second tap
7// within QUIT_DOUBLE_TAP_MS quits immediately. Quitting from the application8// within QUIT_DOUBLE_TAP_MS quits immediately. Quitting from the application
8// menu itself is untouched and quits immediately.9// menu itself is untouched and quits immediately.
...@@ -10,11 +11,11 @@ export const QUIT_HOLD_DURATION_MS = 1200;...@@ -10,11 +11,11 @@ export const QUIT_HOLD_DURATION_MS = 1200;
10// A second quick tap of the shortcut is the user insisting: quit immediately.11// A second quick tap of the shortcut is the user insisting: quit immediately.
11export const QUIT_DOUBLE_TAP_MS = 500;12export const QUIT_DOUBLE_TAP_MS = 500;
12// "Still held" is proven by auto-repeat keydowns, not by the absence of a13// "Still held" is proven by auto-repeat keydowns, not by the absence of a
13// release: macOS suppresses a letter's keyUp while the command key is down, so14// release: macOS suppresses a letter keyUp while the command key is down, so a
14// a tap's release can go completely unseen and a release-based timer would15// tap release can go completely unseen and a release-based timer would quit
15// quit anyway. The press is treated as released once no key event has arrived16// anyway. Once held, quitting waits for Q keyUp or a quiet grace period after
16// for QUIT_HOLD_RELEASE_GRACE_MS past the hold duration. Keyboards with17// modifier keyUp so repeats cannot reach the next app. Keyboards with
17// auto-repeat disabled cannot hold-to-quit and fall back to the menu's Quit.18// auto-repeat disabled fall back to the application menu Quit action.
18export const QUIT_HOLD_RELEASE_GRACE_MS = 600;19export const QUIT_HOLD_RELEASE_GRACE_MS = 600;
1920
20export type QuitHoldState = "down" | "up";21export type QuitHoldState = "down" | "up";
...@@ -42,8 +43,9 @@ export function makeQuitHoldHandler(...@@ -42,8 +43,9 @@ export function makeQuitHoldHandler(
42 const modifierKey = options.platform === "darwin" ? "meta" : "control";43 const modifierKey = options.platform === "darwin" ? "meta" : "control";
43 let watchdog: NodeJS.Timeout | undefined;44 let watchdog: NodeJS.Timeout | undefined;
44 let holding = false;45 let holding = false;
45 // Set once isEnabled resolves true; auto-repeats may only quit when armed.46 // Set once isEnabled resolves true; auto-repeats may only complete the hold when armed.
46 let armed = false;47 let armed = false;
48 let quitOnRelease = false;
47 let heldSince = 0;49 let heldSince = 0;
48 let lastPressAt = 0;50 let lastPressAt = 0;
49 // Incremented on every new press and every release/quit so a pending51 // Incremented on every new press and every release/quit so a pending
...@@ -60,14 +62,16 @@ export function makeQuitHoldHandler(...@@ -60,14 +62,16 @@ export function makeQuitHoldHandler(
6062
61 const release = () => {63 const release = () => {
62 if (!holding) return;64 if (!holding) return;
65 const shouldNotify = armed || quitOnRelease;
63 generation += 1;66 generation += 1;
64 holding = false;67 holding = false;
65 armed = false;68 armed = false;
69 quitOnRelease = false;
66 clearWatchdog();70 clearWatchdog();
67 options.notify("up");71 if (shouldNotify) options.notify("up");
68 };72 };
6973
70 // Dismisses the overlay first: if the quit is cancelled downstream the74 // Dismisses any overlay first: if the quit is cancelled downstream the
71 // renderer must not be left with a stuck "Hold to Quit" hint.75 // renderer must not be left with a stuck "Hold to Quit" hint.
72 const quitNow = () => {76 const quitNow = () => {
73 release();77 release();
...@@ -77,11 +81,27 @@ export function makeQuitHoldHandler(...@@ -77,11 +81,27 @@ export function makeQuitHoldHandler(
77 return (event, input) => {81 return (event, input) => {
78 const key = input.key.toLowerCase();82 const key = input.key.toLowerCase();
79 if (input.type === "keyUp") {83 if (input.type === "keyUp") {
80 if (key === "q" || key === modifierKey) release();84 if (key === "q") {
85 const shouldQuit = quitOnRelease;
86 release();
87 if (shouldQuit) options.quit();
88 } else if (key === modifierKey) {
89 if (!quitOnRelease) {
90 release();
91 } else {
92 watchdog = setTimeout(quitNow, QUIT_HOLD_RELEASE_GRACE_MS);
93 }
94 }
81 return;95 return;
82 }96 }
83 if (input.type !== "keyDown") return;97 if (input.type !== "keyDown") return;
8498
99 if (quitOnRelease && input.isAutoRepeat && key === "q") {
100 event.preventDefault();
101 clearWatchdog();
102 return;
103 }
104
85 const modifierDown = options.platform === "darwin" ? input.meta : input.control;105 const modifierDown = options.platform === "darwin" ? input.meta : input.control;
86 if (!modifierDown || input.alt || input.shift || key !== "q") {106 if (!modifierDown || input.alt || input.shift || key !== "q") {
87 // Any other key (or an extra modifier) pressed mid-hold breaks the107 // Any other key (or an extra modifier) pressed mid-hold breaks the
...@@ -101,7 +121,9 @@ export function makeQuitHoldHandler(...@@ -101,7 +121,9 @@ export function makeQuitHoldHandler(
101121
102 if (input.isAutoRepeat) {122 if (input.isAutoRepeat) {
103 if (armed && Date.now() - heldSince >= QUIT_HOLD_DURATION_MS) {123 if (armed && Date.now() - heldSince >= QUIT_HOLD_DURATION_MS) {
104 quitNow();124 armed = false;
125 quitOnRelease = true;
126 clearWatchdog();
105 }127 }
106 return;128 return;
107 }129 }
...@@ -121,7 +143,6 @@ export function makeQuitHoldHandler(...@@ -121,7 +143,6 @@ export function makeQuitHoldHandler(
121 const pressGeneration = generation;143 const pressGeneration = generation;
122 holding = true;144 holding = true;
123 heldSince = now;145 heldSince = now;
124 options.notify("down");
125 void options.isEnabled().then(146 void options.isEnabled().then(
126 (enabled) => {147 (enabled) => {
127 if (generation !== pressGeneration) return;148 if (generation !== pressGeneration) return;
...@@ -131,6 +152,7 @@ export function makeQuitHoldHandler(...@@ -131,6 +152,7 @@ export function makeQuitHoldHandler(
131 return;152 return;
132 }153 }
133 armed = true;154 armed = true;
155 options.notify("down");
134 // No auto-repeat by then means the key was released (possibly with a156 // No auto-repeat by then means the key was released (possibly with a
135 // suppressed keyUp) or repeat is disabled; either way, don't quit.157 // suppressed keyUp) or repeat is disabled; either way, don't quit.
136 watchdog = setTimeout(() => {158 watchdog = setTimeout(() => {