From 9b7f52b6290b49f2d0c5ec4e1249ebdfb17af945 Mon Sep 17 00:00:00 2001 From: "L.H." <117188168+default-student@users.noreply.github.com> Date: Wed, 22 Jul 2026 21:37:44 +0200 Subject: [PATCH] Fix Guacamole tab visibility lifecycle (#1074) Co-authored-by: default-student Co-authored-by: Luke Gustafson <88517757+LukeGus@users.noreply.github.com> --- package.json | 2 +- scripts/patch-guacamole-common-js.cjs | 66 +++++++++++++++++++ .../hosts/guacamole/guacamole-server.ts | 1 - src/backend/hosts/guacamole/routes.ts | 4 +- src/ui/features/guacamole/GuacamoleApp.tsx | 9 ++- .../features/guacamole/GuacamoleDisplay.tsx | 62 ++++++++++++----- src/ui/shell/tabUtils.tsx | 1 + 7 files changed, 120 insertions(+), 25 deletions(-) create mode 100644 scripts/patch-guacamole-common-js.cjs diff --git a/package.json b/package.json index 7757ffae..f66d52a0 100644 --- a/package.json +++ b/package.json @@ -14,7 +14,7 @@ "format:check": "prettier --check .", "biome:check": "biome check biome.json package.json", "biome:fix": "biome check --write biome.json package.json", - "postinstall": "node scripts/patch-app-builder-lib.cjs && node scripts/patch-guacamole-lite.cjs && node scripts/patch-better-sqlite3.cjs && node scripts/patch-nan.cjs && node scripts/patch-xterm-android-ime.cjs", + "postinstall": "node scripts/patch-app-builder-lib.cjs && node scripts/patch-guacamole-lite.cjs && node scripts/patch-guacamole-common-js.cjs && node scripts/patch-better-sqlite3.cjs && node scripts/patch-nan.cjs && node scripts/patch-xterm-android-ime.cjs", "prebuild": "node scripts/write-electron-build-info.cjs", "lint": "eslint .", "lint:fix": "eslint --fix .", diff --git a/scripts/patch-guacamole-common-js.cjs b/scripts/patch-guacamole-common-js.cjs new file mode 100644 index 00000000..69e6751a --- /dev/null +++ b/scripts/patch-guacamole-common-js.cjs @@ -0,0 +1,66 @@ +const fs = require("fs"); +const path = require("path"); + +const packageRoot = path.join( + __dirname, + "..", + "node_modules", + "guacamole-common-js", +); + +const bundlePaths = [ + path.join(packageRoot, "dist", "esm", "guacamole-common.js"), + path.join(packageRoot, "dist", "cjs", "guacamole-common.js"), +]; + +const oldFlushBlock = + " if (window.requestAnimationFrame && document.hasFocus())\n" + + " asyncFlush();\n" + + " else\n" + + " syncFlush();"; + +const newFlushBlock = + " // Electron can throttle or skip requestAnimationFrame() for inactive\n" + + " // windows/tabs even while guacd is still sending display frames. Flush\n" + + " // synchronously so Guacamole connections do not stall while waiting for\n" + + " // a frame callback that may never run.\n" + + " syncFlush();"; + +let patched = false; +let foundBundle = false; + +for (const bundlePath of bundlePaths) { + if (!fs.existsSync(bundlePath)) { + console.log(`[patch-guacamole-common-js] ${bundlePath} not found, skipping`); + continue; + } + + foundBundle = true; + let content = fs.readFileSync(bundlePath, "utf8"); + if (content.includes(newFlushBlock)) continue; + + if (!content.includes(oldFlushBlock)) { + console.log( + `[patch-guacamole-common-js] Flush target not found in ${bundlePath}, skipping`, + ); + continue; + } + + content = content.replace(oldFlushBlock, newFlushBlock); + fs.writeFileSync(bundlePath, content); + patched = true; +} + +if (!foundBundle) { + console.log("[patch-guacamole-common-js] File not found, skipping"); + process.exit(0); +} + +if (!patched) { + console.log("[patch-guacamole-common-js] Already patched"); + process.exit(0); +} + +console.log( + "[patch-guacamole-common-js] Patched display flush to avoid Electron requestAnimationFrame stalls", +); diff --git a/src/backend/hosts/guacamole/guacamole-server.ts b/src/backend/hosts/guacamole/guacamole-server.ts index e1e53c06..efc8654d 100644 --- a/src/backend/hosts/guacamole/guacamole-server.ts +++ b/src/backend/hosts/guacamole/guacamole-server.ts @@ -170,7 +170,6 @@ const clientOptions = { vnc: { "swap-red-blue": false, cursor: "remote", - security: "any", width: 1280, height: 720, }, diff --git a/src/backend/hosts/guacamole/routes.ts b/src/backend/hosts/guacamole/routes.ts index 3840ee1c..f5b07d5d 100644 --- a/src/backend/hosts/guacamole/routes.ts +++ b/src/backend/hosts/guacamole/routes.ts @@ -589,7 +589,8 @@ router.post( ? { guacdPort: perConnectionGuacdPort } : {}), }; - const recordingEnabled = host.enableSessionLogging !== false; + const recordingEnabled = + connectionType !== "vnc" && host.enableSessionLogging !== false; const recordingName = `${crypto.randomUUID()}.guac`; const recordingPath = process.env.GUACD_RECORDING_PATH || @@ -657,7 +658,6 @@ router.post( password, { port, - security: "any", ...guacConfig, ...guacdOverrides, }, diff --git a/src/ui/features/guacamole/GuacamoleApp.tsx b/src/ui/features/guacamole/GuacamoleApp.tsx index 69656bf3..a978557d 100644 --- a/src/ui/features/guacamole/GuacamoleApp.tsx +++ b/src/ui/features/guacamole/GuacamoleApp.tsx @@ -39,6 +39,7 @@ interface GuacamoleAppProps { hostId?: string; tabId?: string; protocol?: "rdp" | "vnc" | "telnet"; + isVisible?: boolean; } export interface GuacamoleAppHandle { @@ -49,7 +50,7 @@ export interface GuacamoleAppHandle { } const GuacamoleApp = React.forwardRef( - function GuacamoleApp({ hostId, tabId, protocol }, ref) { + function GuacamoleApp({ hostId, tabId, protocol, isVisible = true }, ref) { const { t } = useTranslation(); const [hostConfig, setHostConfig] = useState(null); const [loading, setLoading] = useState(true); @@ -103,6 +104,7 @@ const GuacamoleApp = React.forwardRef( hostName={hostConfig.name || hostConfig.ip || String(hostId)} tabId={tabId} protocol={protocol} + isVisible={isVisible} ref={ref} /> ); @@ -118,13 +120,14 @@ interface GuacamoleAppInnerProps { hostName: string; tabId?: string; protocol?: "rdp" | "vnc" | "telnet"; + isVisible: boolean; } const GuacamoleAppInner = React.forwardRef< GuacamoleAppHandle, GuacamoleAppInnerProps >(function GuacamoleAppInner( - { hostId, hostConfig, hostName, tabId, protocol }, + { hostId, hostConfig, hostName, tabId, protocol, isVisible }, ref, ) { const { t } = useTranslation(); @@ -402,7 +405,7 @@ const GuacamoleAppInner = React.forwardRef< ? configuredDpi : undefined, }} - isVisible={true} + isVisible={isVisible} touchMode={touchMode} onError={(err) => setConnectionError(err)} /> diff --git a/src/ui/features/guacamole/GuacamoleDisplay.tsx b/src/ui/features/guacamole/GuacamoleDisplay.tsx index 502b5817..671fa6f7 100644 --- a/src/ui/features/guacamole/GuacamoleDisplay.tsx +++ b/src/ui/features/guacamole/GuacamoleDisplay.tsx @@ -130,11 +130,11 @@ export const GuacamoleDisplay = forwardRef< }, })); - const getWebSocketUrl = useCallback( + const getWebSocketConnection = useCallback( async ( containerWidth: number, containerHeight: number, - ): Promise => { + ): Promise<{ url: string; query: string } | null> => { try { let token: string; const connectionProtocol = @@ -205,7 +205,7 @@ export const GuacamoleDisplay = forwardRef< height: String(displaySize.height), }); if (displaySize.dpi) params.set("dpi", String(displaySize.dpi)); - return `${wsBase}?${params.toString()}`; + return { url: wsBase, query: params.toString() }; } catch (error) { const errorMessage = error instanceof Error ? error.message : "Unknown error"; @@ -308,10 +308,9 @@ export const GuacamoleDisplay = forwardRef< setIsReady(false); setHasError(false); - // Wait two frames so the container is fully laid out before measuring. - await new Promise((resolve) => - requestAnimationFrame(() => requestAnimationFrame(() => resolve())), - ); + // Let layout settle before measuring without depending on animation frames, + // which may be throttled while Electron windows or tabs are inactive. + await new Promise((resolve) => setTimeout(resolve, 0)); if (!isMountedRef.current) { isConnectingRef.current = false; return; @@ -333,9 +332,7 @@ export const GuacamoleDisplay = forwardRef< (containerWidth < 100 || containerHeight < 100) && attempt < 40; attempt++ ) { - await new Promise((resolve) => - requestAnimationFrame(() => resolve()), - ); + await new Promise((resolve) => setTimeout(resolve, 25)); if (!isMountedRef.current) { isConnectingRef.current = false; return; @@ -348,19 +345,28 @@ export const GuacamoleDisplay = forwardRef< containerHeight = window.innerHeight || 720; } - const wsUrl = await getWebSocketUrl(containerWidth, containerHeight); + const wsConnection = await getWebSocketConnection( + containerWidth, + containerHeight, + ); if (!isMountedRef.current) { isConnectingRef.current = false; return; } - if (!wsUrl) { + if (!wsConnection) { isConnectingRef.current = false; return; } - const tunnel = new Guacamole.WebSocketTunnel(wsUrl); + const tunnel = new Guacamole.WebSocketTunnel(wsConnection.url); const client = new Guacamole.Client(tunnel); clientRef.current = client; + let connectWatchdog: ReturnType | null = null; + const clearConnectWatchdog = () => { + if (!connectWatchdog) return; + clearTimeout(connectWatchdog); + connectWatchdog = null; + }; const display = client.getDisplay(); const displayElement = display.getElement(); @@ -401,7 +407,7 @@ export const GuacamoleDisplay = forwardRef< } display.onresize = () => { - if (!isMountedRef.current) return; + if (!isMountedRef.current || clientRef.current !== client) return; rescaleDisplay(true); setIsReady(true); }; @@ -474,7 +480,7 @@ export const GuacamoleDisplay = forwardRef< refreshKeyboardHandlers(); client.onstatechange = (state: number) => { - if (!isMountedRef.current) return; + if (!isMountedRef.current || clientRef.current !== client) return; switch (state) { case 0: break; @@ -483,6 +489,7 @@ export const GuacamoleDisplay = forwardRef< case 2: break; case 3: + clearConnectWatchdog(); isConnectingRef.current = false; setIsReady(true); onConnect?.(); @@ -502,16 +509,21 @@ export const GuacamoleDisplay = forwardRef< case 4: break; case 5: + clearConnectWatchdog(); + isConnectingRef.current = false; setIsReady(false); + setHasError(true); hasKeyboardFocusRef.current = false; refreshKeyboardHandlers(); + onError?.(t("guacamole.connectionError")); onDisconnect?.(); break; } }; client.onerror = (error: Guacamole.Status) => { - if (!isMountedRef.current) return; + if (!isMountedRef.current || clientRef.current !== client) return; + clearConnectWatchdog(); const errorMessage = error.message || t("guacamole.connectionError"); setIsReady(false); setHasError(true); @@ -555,8 +567,21 @@ export const GuacamoleDisplay = forwardRef< }; try { - client.connect(); + connectWatchdog = setTimeout(() => { + if ( + !isMountedRef.current || + clientRef.current !== client || + !isConnectingRef.current + ) { + return; + } + + disconnectClient(); + void connect(); + }, 8000); + client.connect(wsConnection.query); } catch (error) { + clearConnectWatchdog(); isConnectingRef.current = false; if (!isMountedRef.current) return; setIsReady(false); @@ -566,12 +591,13 @@ export const GuacamoleDisplay = forwardRef< ); } }, [ - getWebSocketUrl, + getWebSocketConnection, onConnect, onDisconnect, onError, refreshKeyboardHandlers, rescaleDisplay, + disconnectClient, connectionConfig.protocol, connectionConfig.type, connectionConfig.dpi, diff --git a/src/ui/shell/tabUtils.tsx b/src/ui/shell/tabUtils.tsx index 05c8ce05..bbcdf412 100644 --- a/src/ui/shell/tabUtils.tsx +++ b/src/ui/shell/tabUtils.tsx @@ -390,6 +390,7 @@ export function renderTabContent( hostId={host.id} tabId={tab.id} protocol={tab.type as "rdp" | "vnc" | "telnet"} + isVisible={isVisible} />, );