From ee2448d084cc66e2ad5e988ee6c437256729df7d Mon Sep 17 00:00:00 2001 From: NimBold Date: Thu, 6 Aug 2026 00:26:59 +0330 Subject: [PATCH] fix(properties): harden child window startup and lifecycle --- src-tauri/capabilities/properties.json | 1 + src-tauri/src/properties_window.rs | 30 ++++--- src/components/PropertiesWindowApp.tsx | 90 ++++++++++++------- src/components/PropertiesWindowBridgeHost.tsx | 16 +++- src/components/WindowControls.tsx | 4 +- src/index.css | 5 ++ src/main.tsx | 74 ++++++++++++--- src/propertiesBridge.test.ts | 23 +++++ src/propertiesBridge.ts | 17 +++- 9 files changed, 193 insertions(+), 67 deletions(-) diff --git a/src-tauri/capabilities/properties.json b/src-tauri/capabilities/properties.json index 8968b07..f7898e9 100644 --- a/src-tauri/capabilities/properties.json +++ b/src-tauri/capabilities/properties.json @@ -5,6 +5,7 @@ "windows": ["properties-*"], "permissions": [ "core:window:allow-close", + "core:window:allow-destroy", "core:window:allow-minimize", "core:window:allow-set-title", "core:window:allow-start-dragging", diff --git a/src-tauri/src/properties_window.rs b/src-tauri/src/properties_window.rs index 23f0325..386631b 100644 --- a/src-tauri/src/properties_window.rs +++ b/src-tauri/src/properties_window.rs @@ -3,7 +3,7 @@ use std::collections::HashSet; use std::sync::Mutex; use serde::Serialize; -use tauri::{Emitter, Manager, WebviewUrl, WebviewWindowBuilder}; +use tauri::{Manager, WebviewUrl, WebviewWindowBuilder}; use uuid::Uuid; const MAIN_WINDOW_LABEL: &str = "main"; @@ -244,10 +244,18 @@ fn emit_to_main( event: &str, payload: T, ) -> Result<(), String> { - let main_window = app - .get_webview_window(MAIN_WINDOW_LABEL) - .ok_or_else(|| "Firelink main window is unavailable".to_string())?; - main_window.emit(event, payload).map_err(|error| error.to_string()) + use tauri::Emitter; + + if app.get_webview_window(MAIN_WINDOW_LABEL).is_none() { + return Err("Firelink main window is unavailable".to_string()); + } + + app.emit_to( + tauri::EventTarget::webview_window(MAIN_WINDOW_LABEL), + event, + payload, + ) + .map_err(|error| error.to_string()) } fn registered_download_for_caller( @@ -350,16 +358,14 @@ pub fn open_download_properties_window( .min_inner_size(760.0, 560.0) .resizable(true) .always_on_top(false) - .visible(true); + // Let the child renderer paint its rounded loading shell before the + // native window becomes visible. Showing an opaque native surface + // here exposes the webview's unpainted white background. + .visible(false) + .transparent(true); #[cfg(any(target_os = "windows", target_os = "macos", target_os = "linux"))] let builder = builder.decorations(false); let build_result = builder.build(); - if let Ok(window) = &build_result { - // Keep the initial loading/error shell visible even if the child - // renderer has not completed its bridge handshake yet. - let _ = window.show(); - let _ = window.set_focus(); - } if let Err(error) = build_result { // Two rapid main-window requests can race between the native lookup // above and builder creation. If the first request won, retain the diff --git a/src/components/PropertiesWindowApp.tsx b/src/components/PropertiesWindowApp.tsx index 2e78c64..0eb7aca 100644 --- a/src/components/PropertiesWindowApp.tsx +++ b/src/components/PropertiesWindowApp.tsx @@ -26,6 +26,7 @@ import { nextPropertiesRequestId, propertiesDiagnosticPhase, propertiesActionRequestKey, + propertiesWindowEventTarget, resetPropertiesActionState, type PropertiesAction, type PropertiesActionRequest, @@ -98,6 +99,8 @@ const propertiesDiagnosticLifecycleKey = (snapshot: PropertiesSnapshot): string export const PropertiesWindowApp = () => { const { t } = useTranslation(); + const translationRef = useRef(t); + translationRef.current = t; const currentWindow = useMemo(() => getCurrentWindow(), []); const windowLabel = currentWindow.label; const sessionId = useMemo(() => crypto.randomUUID(), []); @@ -160,6 +163,7 @@ export const PropertiesWindowApp = () => { const appearanceCleanupRef = useRef<(() => void) | null>(null); const hasRevealedWindowRef = useRef(false); const revealInFlightRef = useRef(false); + const readyRetryTimerRef = useRef(undefined); const diagnosticsInFlightRef = useRef(new Set()); const snapshotRef = useRef(snapshot); const activeTabRef = useRef(activeTab); @@ -205,6 +209,37 @@ export const PropertiesWindowApp = () => { } }, [currentWindow]); + const revealWindow = useCallback(async () => { + if (hasRevealedWindowRef.current) { + if (latestSnapshotRevisionRef.current > 0 && readyRetryTimerRef.current !== undefined) { + window.clearInterval(readyRetryTimerRef.current); + readyRetryTimerRef.current = undefined; + } + return; + } + if (revealInFlightRef.current) return; + revealInFlightRef.current = true; + try { + await invoke('properties_window_reveal'); + hasRevealedWindowRef.current = true; + if (latestSnapshotRevisionRef.current > 0 && readyRetryTimerRef.current !== undefined) { + window.clearInterval(readyRetryTimerRef.current); + readyRetryTimerRef.current = undefined; + } + } catch (error) { + setErrorMessage(errorText(error)); + } finally { + revealInFlightRef.current = false; + } + }, []); + + useEffect(() => { + // Native creation is hidden. Reveal after React has committed the loading + // shell so the first visible frame is styled and clipped by its radius, + // independent of the bridge snapshot timing. + void revealWindow(); + }, [revealWindow]); + useEffect(() => { draftTabRef.current = draftTab; }, [draftTab]); @@ -335,7 +370,6 @@ export const PropertiesWindowApp = () => { useEffect(() => { let cancelled = false; - let readyRetryTimer: number | undefined; let readyHeartbeatTimer: number | undefined; let unlistenSnapshot: UnlistenFn | undefined; let unlistenResult: UnlistenFn | undefined; @@ -407,25 +441,8 @@ export const PropertiesWindowApp = () => { setSnapshot(event.payload.snapshot); if (draftTabRef.current === null) hydrateDraft(event.payload.snapshot); void currentWindow.setTitle(safeTitle(event.payload.snapshot.fileName)).catch(() => undefined); - if (!hasRevealedWindowRef.current && !revealInFlightRef.current) { - revealInFlightRef.current = true; - try { - await invoke('properties_window_reveal'); - hasRevealedWindowRef.current = true; - if (readyRetryTimer !== undefined) { - window.clearInterval(readyRetryTimer); - readyRetryTimer = undefined; - } - } catch (error) { - // Native visibility is established by the opener. Keep the - // loading shell usable when the optional ready-state update is - // delayed or rejected, and let the next snapshot retry it. - setErrorMessage(errorText(error)); - } finally { - revealInFlightRef.current = false; - } - } - }); + await revealWindow(); + }, { target: propertiesWindowEventTarget(windowLabel) }); if (cancelled) { snapshotListener(); return; @@ -454,7 +471,9 @@ export const PropertiesWindowApp = () => { switchAfterSaveRef.current = null; closeAfterSaveRef.current = false; setErrorMessage(''); - setNotice(completedAction === 'apply-properties' ? t($ => $.properties.saved) : ''); + setNotice(completedAction === 'apply-properties' + ? translationRef.current($ => $.properties.saved) + : ''); if (commitsDraft) { draftTabRef.current = null; setDraftTab(null); @@ -468,7 +487,7 @@ export const PropertiesWindowApp = () => { } } } - }); + }, { target: propertiesWindowEventTarget(windowLabel) }); if (cancelled) { resultListener(); return; @@ -477,9 +496,9 @@ export const PropertiesWindowApp = () => { const removedListener = await listen<{ windowLabel: string; downloadId: string }>(PROPERTIES_WINDOW_REMOVED, event => { if (cancelled) return; if (event.payload.windowLabel === windowLabel && event.payload.downloadId === id) { - if (readyRetryTimer !== undefined) { - window.clearInterval(readyRetryTimer); - readyRetryTimer = undefined; + if (readyRetryTimerRef.current !== undefined) { + window.clearInterval(readyRetryTimerRef.current); + readyRetryTimerRef.current = undefined; } if (readyHeartbeatTimer !== undefined) { window.clearInterval(readyHeartbeatTimer); @@ -501,22 +520,23 @@ export const PropertiesWindowApp = () => { pendingActionSendKeyRef.current = null; allowWindowCloseRef.current = false; setPendingAction(null); - setNotice(t($ => $.downloadTable.noDownloads)); + setNotice(translationRef.current($ => $.downloadTable.noDownloads)); } - }); + }, { target: propertiesWindowEventTarget(windowLabel) }); if (cancelled) { removedListener(); return; } unlistenRemoved = removedListener; - await sendPropertiesReady(sessionId); if (cancelled) return; // Tauri event listeners are registered asynchronously. If the main // bridge was still installing its listener, the first ready event can // legitimately be missed; retry until the first snapshot confirms - // the handshake rather than leaving a permanently hidden window. - readyRetryTimer = window.setInterval(() => { - if (cancelled || hasRevealedWindowRef.current) return; + // the handshake. Install this before the first attempt so an IPC + // rejection is recoverable as well as a missed event. + readyRetryTimerRef.current = window.setInterval(() => { + if (cancelled || latestSnapshotRevisionRef.current > 0) return; + void revealWindow(); void sendPropertiesReady(sessionId).catch(() => undefined); }, 500); // The main webview can restart independently of this child window. @@ -526,6 +546,7 @@ export const PropertiesWindowApp = () => { if (cancelled) return; void sendPropertiesReady(sessionId).catch(() => undefined); }, 2000); + void sendPropertiesReady(sessionId).catch(() => undefined); } catch (error) { if (!cancelled) setErrorMessage(errorText(error)); } @@ -533,7 +554,10 @@ export const PropertiesWindowApp = () => { void start(); return () => { cancelled = true; - if (readyRetryTimer !== undefined) window.clearInterval(readyRetryTimer); + if (readyRetryTimerRef.current !== undefined) { + window.clearInterval(readyRetryTimerRef.current); + readyRetryTimerRef.current = undefined; + } if (readyHeartbeatTimer !== undefined) window.clearInterval(readyHeartbeatTimer); unlistenSnapshot?.(); unlistenResult?.(); @@ -544,7 +568,7 @@ export const PropertiesWindowApp = () => { appearanceCleanupRef.current?.(); appearanceCleanupRef.current = null; }; - }, [closeCurrentWindow, currentWindow, hydrateDraft, sessionId, t, windowLabel]); + }, [closeCurrentWindow, currentWindow, hydrateDraft, revealWindow, sessionId, windowLabel]); useEffect(() => { if (!snapshot || draftTab !== null) return; diff --git a/src/components/PropertiesWindowBridgeHost.tsx b/src/components/PropertiesWindowBridgeHost.tsx index 3d43bdd..23641f4 100644 --- a/src/components/PropertiesWindowBridgeHost.tsx +++ b/src/components/PropertiesWindowBridgeHost.tsx @@ -1,5 +1,6 @@ import { useEffect } from 'react'; import { listen, type UnlistenFn } from '@tauri-apps/api/event'; +import { getCurrentWindow } from '@tauri-apps/api/window'; import { useDownloadStore } from '../store/useDownloadStore'; import type { DownloadItem } from '../store/useDownloadStore'; import { useSettingsStore } from '../store/useSettingsStore'; @@ -31,6 +32,7 @@ import { sendPropertiesActionResult, sendPropertiesRemoved, sendPropertiesSnapshot, + propertiesWindowEventTarget, type PropertiesActionRequest, type PropertiesActionResult, type PropertiesPatch, @@ -175,6 +177,7 @@ const copyEditablePatch = (rawPatch: PropertiesPatch): Partial => export const PropertiesWindowBridgeHost = () => { useEffect(() => { + const mainWindowTarget = propertiesWindowEventTarget(getCurrentWindow().label); const windows = new Map(); const snapshotRevisions = new Map(); const actionsInFlight = new Set(); @@ -324,7 +327,12 @@ export const PropertiesWindowBridgeHost = () => { if (disposed) return; const item = useDownloadStore.getState().downloads.find(download => download.id === payload.downloadId); if (!item) { - await sendPropertiesRemoved(payload.windowLabel, payload.downloadId); + // The store subscription cannot see a window that never completed + // registration. Tear down the native registry entry here as well, + // otherwise a late ready event can leave an empty child window and + // a permanently reserved label for a deleted download. + void sendPropertiesRemoved(payload.windowLabel, payload.downloadId).catch(() => undefined); + await invoke('properties_window_registry_remove_for_download', { id: payload.downloadId }); return; } synchronizeRegistration(payload.windowLabel, payload.downloadId, payload.sessionId); @@ -592,14 +600,14 @@ export const PropertiesWindowBridgeHost = () => { attachAsyncPropertiesListener( listen(PROPERTIES_WINDOW_READY, event => { if (!disposed) void handleReady(event.payload); - }), + }, { target: mainWindowTarget }), () => disposed, value => { unlistenReady = value; }, ); attachAsyncPropertiesListener( listen(PROPERTIES_WINDOW_ACTION_REQUEST, event => { if (!disposed) void handleAction(event.payload); - }), + }, { target: mainWindowTarget }), () => disposed, value => { unlistenAction = value; }, ); @@ -612,7 +620,7 @@ export const PropertiesWindowBridgeHost = () => { clearWindowActionState(event.payload); snapshotCoalescer.cancel(event.payload); if (registration) actionsInFlight.delete(`${event.payload}:${registration.downloadId}`); - }), + }, { target: mainWindowTarget }), () => disposed, value => { unlistenClosed = value; }, ); diff --git a/src/components/WindowControls.tsx b/src/components/WindowControls.tsx index 6a0dda4..6d382a6 100644 --- a/src/components/WindowControls.tsx +++ b/src/components/WindowControls.tsx @@ -31,7 +31,9 @@ export function WindowControls({ side, controlStyle }: WindowControlsProps) { onPointerDown={stopTitlebarDrag} onClick={(event) => { event.stopPropagation(); - void appWindow.close(); + void appWindow.close().catch(error => { + console.error('[WindowControls] close failed', error); + }); }} > diff --git a/src/index.css b/src/index.css index 6775596..bb2e5c8 100644 --- a/src/index.css +++ b/src/index.css @@ -574,6 +574,11 @@ html[data-list-density="relaxed"] { .properties-window-shell { --properties-header-surface: hsl(var(--surface-raised)); --properties-card-surface: hsl(var(--bg-input) / 0.42); + min-height: 100%; + overflow: hidden; + border: 1px solid hsl(var(--border-color)); + border-radius: 18px; + background: hsl(var(--main-bg)); } .properties-window-titlebar { diff --git a/src/main.tsx b/src/main.tsx index 3f1ca1d..c452506 100644 --- a/src/main.tsx +++ b/src/main.tsx @@ -1,4 +1,4 @@ -import { StrictMode } from "react"; +import { StrictMode, type ComponentType } from "react"; import { createRoot } from "react-dom/client"; import "@fontsource-variable/inter/wght.css"; import "@fontsource-variable/noto-sans-hebrew/wght.css"; @@ -12,6 +12,7 @@ import { ErrorBoundary } from "./components/ErrorBoundary"; import { ToastProvider } from "./contexts/ToastContext"; import { error as logError, warn as logWarn, initLogger } from "./utils/logger"; import { getCurrentWindow } from '@tauri-apps/api/window'; +import { invokeCommand as invoke } from './ipc'; const isPropertiesWindow = getCurrentWindow().label.startsWith('properties-'); @@ -43,17 +44,9 @@ console.warn = (...values: unknown[]) => { }; const rootElement = document.getElementById("root"); -const renderApp = async () => { +const renderRoot = (RootComponent: ComponentType) => { if (!rootElement) return; - // Keep the child entrypoint isolated from the main application module. App - // imports the persistent Zustand stores, whose module initialization issues - // main-window-only IPC commands. Loading it in a Properties child creates a - // second persistence owner and can race the bridge handshake. - const RootComponent = isPropertiesWindow - ? (await import('./components/PropertiesWindowApp')).PropertiesWindowApp - : (await import('./App')).default; - createRoot(rootElement).render( @@ -65,10 +58,63 @@ const renderApp = async () => { ); }; -void i18nReady.then(renderApp).catch(error => { - console.error('Failed to initialize localization:', error); - void renderApp(); -}); +const PropertiesStartupFailure = () => ( +
+

Download Properties could not be loaded.

+ +
+); + +const renderMainApp = async () => { + if (!rootElement) return; + + // Keep the child entrypoint isolated from the main application module. App + // imports the persistent Zustand stores, whose module initialization issues + // main-window-only IPC commands. Loading it in a Properties child creates a + // second persistence owner and can race the bridge handshake. + const RootComponent = (await import('./App')).default; + renderRoot(RootComponent); +}; + +const renderPropertiesApp = async () => { + if (!rootElement) return; + + try { + // Properties starts with the synchronous English catalog and changes locale + // after its first paint. Waiting for a lazy locale chunk here delays the + // loading shell and makes native window startup visible to the user. + const RootComponent = (await import('./components/PropertiesWindowApp')).PropertiesWindowApp; + renderRoot(RootComponent); + } catch (error) { + // A failed lazy chunk must not leave the native window hidden forever. Show + // a styled, closable failure state and use the same caller-validated native + // reveal command as the normal child path. + console.error('Failed to initialize the Properties window:', error); + renderRoot(PropertiesStartupFailure); + void invoke('properties_window_reveal').catch(revealError => { + console.error('Failed to reveal the Properties startup error:', revealError); + }); + } +}; + +if (isPropertiesWindow) { + void renderPropertiesApp(); +} else { + void i18nReady.then(renderMainApp).catch(error => { + console.error('Failed to initialize localization:', error); + void renderMainApp(); + }); +} // Prevent the webview's default context menu ("Reload", etc.) on right-click. // Individual components that provide custom context menus call preventDefault() diff --git a/src/propertiesBridge.test.ts b/src/propertiesBridge.test.ts index 986ffd9..e09d446 100644 --- a/src/propertiesBridge.test.ts +++ b/src/propertiesBridge.test.ts @@ -1,4 +1,5 @@ import { describe, expect, it, vi } from 'vitest'; +import { emitTo } from '@tauri-apps/api/event'; import type { DownloadItem } from './store/useDownloadStore'; vi.mock('./ipc', () => ({ @@ -24,12 +25,34 @@ import { propertiesActionRequestKey, propertiesDiagnosticRequestState, propertiesTorrentPeerLimit, + propertiesWindowEventTarget, resetPropertiesActionState, sanitizePropertiesSnapshot, + sendPropertiesSnapshot, shouldAcceptPropertiesActionRequest, } from './propertiesBridge'; describe('Properties window bridge', () => { + it('uses a WebviewWindow target for directed child events', () => { + expect(propertiesWindowEventTarget('properties-1')).toEqual({ + kind: 'WebviewWindow', + label: 'properties-1', + }); + }); + + it('targets the initial snapshot at the child WebviewWindow', async () => { + vi.mocked(emitTo).mockClear(); + const payload = {} as Parameters[1]; + + await sendPropertiesSnapshot('properties-1', payload); + + expect(emitTo).toHaveBeenCalledWith( + { kind: 'WebviewWindow', label: 'properties-1' }, + 'properties-window-snapshot', + payload, + ); + }); + it('sanitizes transfer secrets while preserving presence flags', () => { const item = { id: 'download-1', diff --git a/src/propertiesBridge.ts b/src/propertiesBridge.ts index a5e6c63..ad8610b 100644 --- a/src/propertiesBridge.ts +++ b/src/propertiesBridge.ts @@ -28,6 +28,17 @@ export const DEFAULT_PROPERTIES_WINDOW_CHROME: PropertiesWindowChrome = { side: 'left', }; +// Tauri's listen() default target is `Any`, which only receives events emitted +// globally. Properties snapshots and child actions are emitted to a specific +// WebviewWindow, so the child must register against that exact target. Keeping +// the target construction here prevents a future listener from silently +// falling back to the global target and waiting forever for its first +// snapshot. +export const propertiesWindowEventTarget = (windowLabel: string) => ({ + kind: 'WebviewWindow' as const, + label: windowLabel, +}); + export const propertiesTorrentPeerLimit = (value: unknown): number => typeof value === 'number' && Number.isInteger(value) @@ -457,13 +468,13 @@ export const sendPropertiesActionRequest = (payload: PropertiesActionRequest): P }); export const sendPropertiesSnapshot = (windowLabel: string, payload: PropertiesSnapshotEvent): Promise => - emitTo(windowLabel, PROPERTIES_WINDOW_SNAPSHOT, payload); + emitTo(propertiesWindowEventTarget(windowLabel), PROPERTIES_WINDOW_SNAPSHOT, payload); export const sendPropertiesActionResult = (windowLabel: string, payload: PropertiesActionResult): Promise => - emitTo(windowLabel, PROPERTIES_WINDOW_ACTION_RESULT, payload); + emitTo(propertiesWindowEventTarget(windowLabel), PROPERTIES_WINDOW_ACTION_RESULT, payload); export const sendPropertiesRemoved = (windowLabel: string, downloadId: string): Promise => - emitTo(windowLabel, PROPERTIES_WINDOW_REMOVED, { windowLabel, downloadId }); + emitTo(propertiesWindowEventTarget(windowLabel), PROPERTIES_WINDOW_REMOVED, { windowLabel, downloadId }); export const applySecretPatch = ( patch: unknown,