Skip to content

[RQ-799] fix: Add fix to resolve editors freezing on windows (Discard Changes dialog) #310

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ajinkya-browserstack wants to merge 1 commit into
base: master
Choose a base branch
from
+60 −5
Open

[RQ-799] fix: Add fix to resolve editors freezing on windows (Discard Changes dialog) #310

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 39 additions & 4 deletions src/main/main.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,41 @@ const createWindow = async () => {
new AutoUpdate(webAppWindow);
remote.enable(webAppWindow.webContents);

// -- Fix: Electron Windows bug #20400 --
// 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:
// a) Handle the synchronous IPC call with the native dialog.
// b) Inject JS after dom-ready that wires window.confirm → __rqConfirmSync.
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;
});
Comment on lines +264 to +279
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Mar 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 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.js

Repository: 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 -50

Repository: 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 -n

Repository: 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 -n

Repository: 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 -n

Repository: 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.


webAppWindow.webContents.on("dom-ready", () => {
webAppWindow?.webContents.executeJavaScript(`
if (typeof window.__rqConfirmSync === 'function') {
window.confirm = function(message) {
return window.__rqConfirmSync(message);
};
}
`);
});
}

// TODO @sahil: Prod and Local Urls should be supplied by @requestly/requestly-core-npm package.
const DESKTOP_APP_URL = getWebAppURL();
webAppWindow.loadURL(DESKTOP_APP_URL, {
Expand Down Expand Up @@ -504,17 +539,17 @@ export const loadWebAppUrl = async (newURL: string) => {
if (!webAppWindow || webAppWindow.isDestroyed()) {
throw new Error("Web app window is not available");
}

await webAppWindow.loadURL(newURL, {
extraHeaders: "pragma: no-cache\n",
});

customWebAppURL = newURL;

if (!webAppWindow.isVisible()) {
webAppWindow.show();
}

webAppWindow.focus();
};

Expand Down
22 changes: 21 additions & 1 deletion src/main/preload.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
require("core-js/stable");
require("regenerator-runtime/runtime");
// Core
const { contextBridge } = require("electron");
const { contextBridge, ipcRenderer } = require("electron");
const { app } = require("@electron/remote");

const DesktopStorageService = require("./preload-apis/DesktopStorageService");
Expand All @@ -20,6 +20,26 @@ if (process.env.NODE_ENV === "development") {
appVersion = app.getVersion();
}

// 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: https://github.com/electron/electron/issues/20400
//
// contextIsolation is enabled (default since Electron 12), so the preload's
// window is isolated from the renderer's window. Assigning
// window.confirm = ... here only affects the preload world, NOT the web
// page. To override the renderer's window.confirm we:
// 1. Expose a synchronous helper (__rqConfirmSync) via contextBridge that
// uses ipcRenderer.sendSync to call the main process.
// 2. In main.ts, after dom-ready, inject JS that replaces window.confirm
// with a call to window.__rqConfirmSync.
if (process.platform === "win32") {
contextBridge.exposeInMainWorld("__rqConfirmSync", (message) => {
// sendSync blocks the renderer until the main process replies.
return ipcRenderer.sendSync("rq:show-confirm-dialog", String(message ?? ""));
});
}

(function (window) {
// Build the RQ object
const RQ = window.RQ || {};
Expand Down