[RQ-799] fix: Add fix to resolve editors freezing on windows (Discard Changes dialog)#310
[RQ-799] fix: Add fix to resolve editors freezing on windows (Discard Changes dialog)#310ajinkya-browserstack wants to merge 1 commit intomasterfrom
Conversation
WalkthroughThese changes implement Windows-specific native dialog functionality for confirmation dialogs. The main process registers a synchronous IPC listener that displays a native dialog.showMessageBoxSync with OK/Cancel buttons and returns the user's selection as a boolean. The preload script exposes a Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/main.ts`:
- Around line 264-279: The IPC handler ipcMain.on("rq:show-confirm-dialog") is
currently registered inside createWindow() and parents dialogs to the mutable
webAppWindow, causing duplicate handlers and incorrect parenting; move this
registration out of createWindow() (or add a guard using
ipcMain.listenerCount("rq:show-confirm-dialog") to avoid multiple
registrations), resolve the parent window with
BrowserWindow.fromWebContents(event.sender) instead of webAppWindow, and wrap
the call to dialog.showMessageBoxSync(...) in a try/catch so that
event.returnValue is always set (true/false) even on error.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e3b01281-70d3-49b0-9f9c-d88986088f5d
📒 Files selected for processing (2)
src/main/main.tssrc/main/preload.js
| if (process.platform === "win32") { | ||
| ipcMain.on("rq:show-confirm-dialog", (event, message) => { | ||
| const buttonIdx = dialog.showMessageBoxSync( | ||
| webAppWindow as BrowserWindow, | ||
| { | ||
| type: "question", | ||
| buttons: ["OK", "Cancel"], | ||
| defaultId: 0, | ||
| cancelId: 1, | ||
| detail: String(message ?? ""), | ||
| message: "", | ||
| } | ||
| ); | ||
| // sendSync expects event.returnValue to be set | ||
| event.returnValue = buttonIdx === 0; | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
printf '%s\n' '--- confirm IPC block ---'
sed -n '257,290p' src/main/main.ts
printf '\n%s\n' '--- createWindow references ---'
rg -n -C2 '\bcreateWindow\s*\(' src/main/main.ts
printf '\n%s\n' '--- confirm channel references ---'
rg -n -C2 'rq:show-confirm-dialog' src/main/main.ts src/main/preload.jsRepository: requestly/requestly-desktop-app
Length of output: 2560
🏁 Script executed:
#!/bin/bash
# Find the createWindow function definition and its boundaries
rg -n 'async\s+function\s+createWindow|function\s+createWindow' src/main/main.ts
# Show the structure around createWindow definition with line numbers
sed -n '200,300p' src/main/main.ts | cat -n | head -50Repository: requestly/requestly-desktop-app
Length of output: 1630
🏁 Script executed:
#!/bin/bash
# Show the IPC handler registration and the end of createWindow function
sed -n '260,330p' src/main/main.ts | cat -nRepository: requestly/requestly-desktop-app
Length of output: 2722
🏁 Script executed:
#!/bin/bash
# Continue looking for the end of createWindow function
sed -n '320,380p' src/main/main.ts | cat -nRepository: requestly/requestly-desktop-app
Length of output: 1986
🏁 Script executed:
#!/bin/bash
# Find where createWindow ends
sed -n '380,420p' src/main/main.ts | cat -nRepository: requestly/requestly-desktop-app
Length of output: 1548
Hoist the confirm IPC handler outside createWindow() and use BrowserWindow.fromWebContents(event.sender) to resolve the parent window.
The ipcMain.on("rq:show-confirm-dialog") registration at lines 265–279 runs inside createWindow(), which is invoked at lines 477 and 525. This causes the handler to stack on each window creation, allowing a single confirm() call to trigger multiple native dialogs. Additionally, the handler parents the dialog to the mutable global webAppWindow rather than the actual sender window, and lacks error handling on the synchronous dialog.showMessageBoxSync() call—if it throws, event.returnValue is never set, blocking the renderer indefinitely.
Move this handler registration outside createWindow() (or guard it with ipcMain.listenerCount()), resolve the parent with BrowserWindow.fromWebContents(event.sender), and wrap the dialog call in try-catch to ensure event.returnValue is always set.
Suggested fix
if (process.platform === "win32") {
- ipcMain.on("rq:show-confirm-dialog", (event, message) => {
- const buttonIdx = dialog.showMessageBoxSync(
- webAppWindow as BrowserWindow,
- {
- type: "question",
- buttons: ["OK", "Cancel"],
- defaultId: 0,
- cancelId: 1,
- detail: String(message ?? ""),
- message: "",
- }
- );
- // sendSync expects event.returnValue to be set
- event.returnValue = buttonIdx === 0;
- });
+ if (!ipcMain.listenerCount("rq:show-confirm-dialog")) {
+ ipcMain.on("rq:show-confirm-dialog", (event, message) => {
+ const options = {
+ type: "question" as const,
+ buttons: ["OK", "Cancel"],
+ defaultId: 0,
+ cancelId: 1,
+ detail: String(message ?? ""),
+ message: "",
+ };
+ const parentWindow = BrowserWindow.fromWebContents(event.sender);
+
+ try {
+ const buttonIdx =
+ parentWindow && !parentWindow.isDestroyed()
+ ? dialog.showMessageBoxSync(parentWindow, options)
+ : dialog.showMessageBoxSync(options);
+
+ event.returnValue = buttonIdx === 0;
+ } catch (error) {
+ logger.warn("[Windows confirm] Failed to show confirm dialog", error);
+ event.returnValue = false;
+ }
+ });
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/main.ts` around lines 264 - 279, The IPC handler
ipcMain.on("rq:show-confirm-dialog") is currently registered inside
createWindow() and parents dialogs to the mutable webAppWindow, causing
duplicate handlers and incorrect parenting; move this registration out of
createWindow() (or add a guard using
ipcMain.listenerCount("rq:show-confirm-dialog") to avoid multiple
registrations), resolve the parent window with
BrowserWindow.fromWebContents(event.sender) instead of webAppWindow, and wrap
the call to dialog.showMessageBoxSync(...) in a try/catch so that
event.returnValue is always set (true/false) even on error.
1 participant
Approach:
Work around Electron Windows bug where built-in confirm/alert cause input
fields (including CodeMirror editors) to lose the visible caret and stop
accepting text until the window is re-focused.
See: electron/electron#20400
contextIsolation is enabled (default since Electron 12), so the preload's
window is isolated from the renderer's window. Assigning
window.confirm = ... in preload.js only affects the preload world, NOT the web page.
To override the renderer's window.confirm we:
Built-in window.confirm on Windows causes input fields to lose their
caret / freeze. We replace it with dialog.showMessageBoxSync via IPC.
The preload exposes __rqConfirmSync (ipcRenderer.sendSync wrapper) into
the renderer world.
Here we:
Task: https://browserstack.atlassian.net/browse/RQ-799