mirror of
https://github.com/got-feedBack/feedBack-desktop.git
synced 2026-09-10 23:04:09 +00:00
fix(panes): let toggleWindow be the authority, not a stale hasWindow() check
The tray menu asked "do we own a window for this pane?" and then acted on the answer. Windows are destroyed asynchronously, so between the question and the act the answer can go stale: hasWindow() says yes, the window is destroyed, toggleWindow() returns false, the handler has already committed to the main-process path and returns — and the click lands on nothing. A tray item that silently does nothing is the worst possible failure here, because the tray IS the recovery path when a pane is out of sight. toggleWindow() already reports whether it did anything. Use that: if it toggled, we're done; if it didn't, we never had that window (or just lost it), and only the renderer can decide what opening the pane means — it might belong in the dock, and its element lives there. hasWindow()/hasPaneWindow() existed only to support the racy check, so they're gone rather than left lying around for someone to reintroduce the race with. Signed-off-by: topkoa <topkoa@gmail.com>
This commit is contained in:
+1
-2
@@ -128,7 +128,7 @@ import { installAppMenu } from './app-menu';
|
|||||||
import { sanitizeWindowBounds, MIN_WIDTH, MIN_HEIGHT } from './window-bounds';
|
import { sanitizeWindowBounds, MIN_WIDTH, MIN_HEIGHT } from './window-bounds';
|
||||||
import {
|
import {
|
||||||
initPaneHosts, closeAllPanes, adoptPaneWindow, paneIdFromFrameName,
|
initPaneHosts, closeAllPanes, adoptPaneWindow, paneIdFromFrameName,
|
||||||
togglePaneWindow, showAllPaneWindows, hideAllPaneWindows, hasPaneWindow,
|
togglePaneWindow, showAllPaneWindows, hideAllPaneWindows,
|
||||||
} from './pane-hosts';
|
} from './pane-hosts';
|
||||||
import { initTray, destroyTray } from './pane-tray';
|
import { initTray, destroyTray } from './pane-tray';
|
||||||
|
|
||||||
@@ -1199,7 +1199,6 @@ async function startup(): Promise<void> {
|
|||||||
toggleWindow: togglePaneWindow,
|
toggleWindow: togglePaneWindow,
|
||||||
showAll: showAllPaneWindows,
|
showAll: showAllPaneWindows,
|
||||||
hideAll: hideAllPaneWindows,
|
hideAll: hideAllPaneWindows,
|
||||||
hasWindow: hasPaneWindow,
|
|
||||||
});
|
});
|
||||||
|
|
||||||
// Install our application menu (replaces Electron's default so View →
|
// Install our application menu (replaces Electron's default so View →
|
||||||
|
|||||||
@@ -269,11 +269,6 @@ export function hideAllPaneWindows(): void {
|
|||||||
refreshTray();
|
refreshTray();
|
||||||
}
|
}
|
||||||
|
|
||||||
export function hasPaneWindow(paneId: string): boolean {
|
|
||||||
const win = windows.get(paneId);
|
|
||||||
return !!win && !win.isDestroyed();
|
|
||||||
}
|
|
||||||
|
|
||||||
// ── Wiring ──────────────────────────────────────────────────────────────────
|
// ── Wiring ──────────────────────────────────────────────────────────────────
|
||||||
|
|
||||||
export function initPaneHosts(deps: { getMainWindow: () => BrowserWindow | null }): void {
|
export function initPaneHosts(deps: { getMainWindow: () => BrowserWindow | null }): void {
|
||||||
|
|||||||
+11
-5
@@ -36,10 +36,11 @@ export interface TrayPane {
|
|||||||
// through initTray().
|
// through initTray().
|
||||||
export interface TrayPaneActions {
|
export interface TrayPaneActions {
|
||||||
getMainWindow: () => Electron.BrowserWindow | null;
|
getMainWindow: () => Electron.BrowserWindow | null;
|
||||||
|
// Returns true if it owned a window for this pane and toggled it; false if it
|
||||||
|
// did not, in which case only the renderer can decide what opening it means.
|
||||||
toggleWindow: (paneId: string) => boolean;
|
toggleWindow: (paneId: string) => boolean;
|
||||||
showAll: () => void;
|
showAll: () => void;
|
||||||
hideAll: () => void;
|
hideAll: () => void;
|
||||||
hasWindow: (paneId: string) => boolean;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
let tray: Tray | null = null;
|
let tray: Tray | null = null;
|
||||||
@@ -71,10 +72,15 @@ function buildMenu(): Menu {
|
|||||||
type: 'checkbox',
|
type: 'checkbox',
|
||||||
checked: p.open === true,
|
checked: p.open === true,
|
||||||
click: () => {
|
click: () => {
|
||||||
// If we already own a window for this pane, showing/hiding it is a
|
// Let toggleWindow() be the authority, rather than asking "do we have a
|
||||||
// main-process job and instant. If we don't, only the renderer can
|
// window?" and then acting on the answer. Windows are destroyed
|
||||||
// decide what opening it means (it might belong in the dock), so ask.
|
// asynchronously, so between the question and the act the answer can go
|
||||||
if (actions?.hasWindow(p.id)) { actions.toggleWindow(p.id); return; }
|
// stale — and the click would land on nothing and silently do nothing.
|
||||||
|
//
|
||||||
|
// If it toggled, we're done. If it didn't, we never had that window (or
|
||||||
|
// just lost it), and only the renderer can decide what opening the pane
|
||||||
|
// means — it might belong in the dock, and its element lives there.
|
||||||
|
if (actions?.toggleWindow(p.id)) return;
|
||||||
const win = actions?.getMainWindow() ?? null;
|
const win = actions?.getMainWindow() ?? null;
|
||||||
if (win && !win.isDestroyed()) win.webContents.send(IPC_PANE_EVENT_TOGGLE, { paneId: p.id });
|
if (win && !win.isDestroyed()) win.webContents.send(IPC_PANE_EVENT_TOGGLE, { paneId: p.id });
|
||||||
},
|
},
|
||||||
|
|||||||
Reference in New Issue
Block a user