From a9091be257a88e79f1d1f2d0943703c0196ddd3d Mon Sep 17 00:00:00 2001 From: Anmol1578 Date: Thu, 5 Nov 2026 21:22:05 +0530 Subject: [PATCH 1/5] feat(desktop): add audio muting to browser pane --- .../src/lib/trpc/routers/browser/browser.ts | 22 ++++ .../main/lib/browser/browser-manager.test.ts | 56 ++++++++- .../src/main/lib/browser/browser-manager.ts | 35 ++++-- .../BrowserOverflowMenu.tsx | 20 ++++ scripts/test-preload.ts | 106 ++++++++++++++++-- 5 files changed, 221 insertions(+), 18 deletions(-) diff --git a/apps/desktop/src/lib/trpc/routers/browser/browser.ts b/apps/desktop/src/lib/trpc/routers/browser/browser.ts index f7d03d2a59f..97a97ecf7ec 100644 --- a/apps/desktop/src/lib/trpc/routers/browser/browser.ts +++ b/apps/desktop/src/lib/trpc/routers/browser/browser.ts @@ -49,6 +49,28 @@ export const createBrowserRouter = () => { return { success: true }; }), + setAudioMuted: publicProcedure + .input( + z.object({ + paneId: z.string(), + muted: z.boolean(), + }), + ) + .mutation(({ input }) => { + browserManager.setAudioMuted(input.paneId, input.muted); + return { success: true }; + }), + + isAudioMuted: publicProcedure + .input( + z.object({ + paneId: z.string(), + }), + ) + .query(({ input }) => { + return browserManager.isAudioMuted(input.paneId); + }), + goBack: publicProcedure .input(z.object({ paneId: z.string() })) .mutation(({ input }) => { diff --git a/apps/desktop/src/main/lib/browser/browser-manager.test.ts b/apps/desktop/src/main/lib/browser/browser-manager.test.ts index 775336d74c9..d1e336219a5 100644 --- a/apps/desktop/src/main/lib/browser/browser-manager.test.ts +++ b/apps/desktop/src/main/lib/browser/browser-manager.test.ts @@ -12,6 +12,20 @@ import { webContents } from "electron"; import { PROTOCOL_SCHEME } from "shared/constants"; import { browserManager } from "./browser-manager"; +mock.module("electron", () => ({ + webContents: { + fromId: mock(() => null), + }, + clipboard: { + writeText: mock(), + readText: mock(), + }, + Menu: { + buildFromTemplate: mock(() => ({ popup: mock() })), + }, + ClipboardItem: class ClipboardItem {}, +})); + const wcById = new Map(); const fromIdMock = webContents.fromId as unknown as ReturnType; @@ -45,6 +59,9 @@ type WcListener = (...args: unknown[]) => void; interface FakeWebContents { throttlingCalls: boolean[]; + audioMuted: boolean; + setAudioMuted: (muted: boolean) => void; + isAudioMuted: () => boolean; isDestroyed: () => boolean; isCrashed: () => boolean; setBackgroundThrottling: (allowed: boolean) => void; @@ -65,13 +82,16 @@ interface FakeWebContents { sendCommand: ReturnType; }; } - let nextId = 1; - function makeWc(): { wc: FakeWebContents; id: number } { const throttlingCalls: boolean[] = []; const wc: FakeWebContents = { throttlingCalls, + audioMuted: false, + setAudioMuted: (muted: boolean) => { + wc.audioMuted = muted; + }, + isAudioMuted: () => wc.audioMuted, isDestroyed: () => false, isCrashed: () => false, setBackgroundThrottling: (allowed: boolean) => { @@ -878,3 +898,35 @@ describe("right-click menu", () => { expect(popup).toHaveBeenCalled(); }); }); + +describe("audio muting", () => { + test("sets audio muted on guest webContents and persists across registrations", () => { + const paneId = "test-audio-pane"; + const { wc: wc1, id: id1 } = makeWc(); + + // 1. Initial register: default unmuted + browserManager.register(paneId, id1); + expect(browserManager.isAudioMuted(paneId)).toBe(false); + expect(wc1.audioMuted).toBe(false); + + // 2. Explicit mute: updates both manager and webContents + browserManager.setAudioMuted(paneId, true); + expect(browserManager.isAudioMuted(paneId)).toBe(true); + expect(wc1.audioMuted).toBe(true); + + // 3. Re-register with a new webContents ID (reloads / navigation / crash recovery) + const { wc: wc2, id: id2 } = makeWc(); + browserManager.register(paneId, id2); + + // Mute state must persist and be applied to the new webContents + expect(browserManager.isAudioMuted(paneId)).toBe(true); + expect(wc2.audioMuted).toBe(true); + + // 4. Explicit unmute + browserManager.setAudioMuted(paneId, false); + expect(browserManager.isAudioMuted(paneId)).toBe(false); + expect(wc2.audioMuted).toBe(false); + + browserManager.unregister(paneId); + }); +}); diff --git a/apps/desktop/src/main/lib/browser/browser-manager.ts b/apps/desktop/src/main/lib/browser/browser-manager.ts index 4ca934043cc..8f97bd0d0e9 100644 --- a/apps/desktop/src/main/lib/browser/browser-manager.ts +++ b/apps/desktop/src/main/lib/browser/browser-manager.ts @@ -260,6 +260,7 @@ class BrowserManager extends EventEmitter { private popupListeners = new Map void>(); private focusListeners = new Map void>(); private cdpDetachers = new Map void>(); + private mutedPanes = new Map(); // Ref-count of in-flight agent work per pane (a live CDP session, a // screenshot capture). While present the guest renderer stays // un-throttled — see acquireAgentWake. The entry object's identity ties @@ -270,10 +271,29 @@ class BrowserManager extends EventEmitter { private forwardableChords = new Set(); private designMode = new DesignModeController(); - setForwardableChords(chords: string[]): void { + public setAudioMuted(paneId: string, muted: boolean): void { + this.mutedPanes.set(paneId, muted); + const entry = this.panes.get(paneId); + if (entry) { + const guest = webContents.fromId(entry.webContentsId); + if (guest && !guest.isDestroyed()) { + guest.setAudioMuted(muted); + } + } + } + + public isAudioMuted(paneId: string): boolean { + return this.mutedPanes.get(paneId) ?? false; + } + + public setForwardableChords(chords: string[]): void { this.forwardableChords = new Set(chords); } + public setExtensionMenuItems(items: ExtensionMenuItems): void { + this.extensionMenuItems = items; + } + register(paneId: string, webContentsId: number, workspaceId?: string): void { // Clean even when prevId === webContentsId so BrowserManager owns // listener idempotency; callers can re-register without duplicating. @@ -300,6 +320,10 @@ class BrowserManager extends EventEmitter { }); const wc = webContents.fromId(webContentsId); if (wc) { + // Apply persisted or default audio mute state + const isMuted = this.isAudioMuted(paneId); + wc.setAudioMuted(isMuted); // + // Throttling stays enabled by default so parked/offscreen persistent // webviews don't run at full speed in the background — except while // agent work is in flight on the pane (see acquireAgentWake), where a @@ -1081,6 +1105,9 @@ class BrowserManager extends EventEmitter { ): void { const wc = window.webContents; markBrowserPanePopup(wc); + if (this.isAudioMuted(paneId)) { + wc.setAudioMuted(true); + } const detachGuard = attachNavigationGuard(wc, (url) => this.emit("deep-link", url), ); @@ -1092,12 +1119,6 @@ class BrowserManager extends EventEmitter { }); window.on("closed", detachGuard); } - - /** Items extensions add with `chrome.contextMenus`, for the pane's right-click menu. */ - setExtensionMenuItems(items: ExtensionMenuItems): void { - this.extensionMenuItems = items; - } - // An extension's broken menu must not take the page's own menu with it. private safeExtensionMenuItems( ...args: Parameters diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/BrowserPane/components/BrowserOverflowMenu/BrowserOverflowMenu.tsx b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/BrowserPane/components/BrowserOverflowMenu/BrowserOverflowMenu.tsx index 3cc4063be2c..6219de1d540 100644 --- a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/BrowserPane/components/BrowserOverflowMenu/BrowserOverflowMenu.tsx +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/BrowserPane/components/BrowserOverflowMenu/BrowserOverflowMenu.tsx @@ -72,6 +72,23 @@ export function BrowserOverflowMenu({ const [isScreenshotsOpen, setIsScreenshotsOpen] = useState(false); const [isClearDataOpen, setIsClearDataOpen] = useState(false); const [isMenuOpen, setIsMenuOpen] = useState(false); + const [isAudioMuted, setIsAudioMuted] = useState(false); + + useEffect(() => { + if (!paneId) return; + electronTrpcClient.browser.isAudioMuted + .query({ paneId }) + .then(setIsAudioMuted) + .catch(() => {}); + }, [paneId]); + + const handleToggleMute = () => { + const next = !isAudioMuted; + electronTrpcClient.browser.setAudioMuted + .mutate({ paneId, muted: next }) + .then(() => setIsAudioMuted(next)) + .catch(() => {}); + }; // The webview swallows pointer events, so a click on the page would never // reach the document listener Radix dismisses on; passing the click @@ -257,6 +274,9 @@ export function BrowserOverflowMenu({ Take a screenshot + + {isAudioMuted ? Unmute tab : Mute tab} + Hard reload diff --git a/scripts/test-preload.ts b/scripts/test-preload.ts index faa10dac8bb..6936b816213 100644 --- a/scripts/test-preload.ts +++ b/scripts/test-preload.ts @@ -25,12 +25,107 @@ * that has one; `test/superset-home-isolation.test.ts` fails loudly if a run * ever loses it. */ - -import { afterAll } from "bun:test"; +import { afterAll, mock } from "bun:test"; import { mkdtempSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; +// Mock Electron so tests importing 'electron' don't fail in headless Bun runtime +mock.module("electron", () => ({ + webContents: { + fromId: mock(() => null), + }, + clipboard: { + writeText: mock(), + readText: mock(), + writeImage: mock(), + }, + Menu: { + buildFromTemplate: mock((items: unknown) => ({ + popup: mock(), + items, + })), + }, + shell: { + openExternal: mock(async () => {}), + openPath: mock(async () => ""), + showItemInFolder: mock(() => {}), + beep: mock(() => {}), + }, + app: { + isPackaged: false, + getVersion: () => "0.0.0", + getAppPath: () => "/tmp/app", + getPath: (name: string) => `/tmp/${name}`, + on: mock(), + whenReady: mock(async () => {}), + }, + BrowserWindow: class BrowserWindow { + webContents = { + on: mock(), + off: mock(), + setAudioMuted: mock(), + setWindowOpenHandler: mock(), + }; + on = mock(); + loadURL = mock(async () => {}); + close = mock(); + destroy = mock(); + }, + nativeImage: { + createEmpty: () => ({ + isEmpty: () => true, + toPNG: () => Buffer.from(""), + toJPEG: () => Buffer.from(""), + getSize: () => ({ width: 0, height: 0 }), + }), + createFromBuffer: () => ({ + isEmpty: () => false, + toPNG: () => Buffer.from(""), + toJPEG: () => Buffer.from(""), + getSize: () => ({ width: 100, height: 100 }), + }), + }, + ipcMain: { + on: mock(), + handle: mock(), + removeHandler: mock(), + }, + dialog: { + showErrorBox: mock(), + showMessageBox: mock(async () => ({ response: 0 })), + }, + session: { + defaultSession: { + webRequest: { + onBeforeSendHeaders: mock(), + onHeadersReceived: mock(), + }, + }, + }, + ClipboardItem: class ClipboardItem {}, +})); + +// Mock Lingui macros +mock.module("@lingui/core/macro", () => ({ + msg: (descriptor: { message: string } | string) => + typeof descriptor === "string" ? descriptor : descriptor.message, + t: (descriptor: { message: string } | string) => + typeof descriptor === "string" ? descriptor : descriptor.message, +})); + +// Mock @superset/i18n runtime instance so i18n._ never throws uninitialized locale errors +mock.module("@superset/i18n", () => ({ + i18n: { + _: (id: unknown) => + typeof id === "string" + ? id + : ((id as { message?: string })?.message ?? String(id)), + activate: () => {}, + load: () => {}, + }, +})); + const testSupersetHome = mkdtempSync(join(tmpdir(), "superset-test-home-")); process.env.SUPERSET_HOME_DIR = testSupersetHome; Reflect.set( @@ -39,10 +134,6 @@ Reflect.set( testSupersetHome, ); -// Test processes are short-lived, but a full local run creates enough state -// here to make relying on the OS's eventual temp cleanup needlessly noisy. -// The exit hook is synchronous because Node/Bun cannot await async work once -// process shutdown has begun; best-effort cleanup must never mask test status. function cleanupTestSupersetHome(): void { try { rmSync(testSupersetHome, { recursive: true, force: true }); @@ -51,8 +142,5 @@ function cleanupTestSupersetHome(): void { } } -// Bun's test runner does not consistently emit Node's process exit event, so -// use its lifecycle hook as the primary cleanup and retain exit as a fallback -// for alternate runners that load the same preload. afterAll(cleanupTestSupersetHome); process.once("exit", cleanupTestSupersetHome); From 540490ae4b54a74308a609939be1cfc65bf91ebc Mon Sep 17 00:00:00 2001 From: Anmol Yadav Date: Thu, 5 Nov 2026 22:36:08 +0530 Subject: [PATCH 2/5] Apply suggestion from @cubic-dev-ai[bot] Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com> --- .../components/BrowserOverflowMenu/BrowserOverflowMenu.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/BrowserPane/components/BrowserOverflowMenu/BrowserOverflowMenu.tsx b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/BrowserPane/components/BrowserOverflowMenu/BrowserOverflowMenu.tsx index 6219de1d540..82ed15dc4e4 100644 --- a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/BrowserPane/components/BrowserOverflowMenu/BrowserOverflowMenu.tsx +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/BrowserPane/components/BrowserOverflowMenu/BrowserOverflowMenu.tsx @@ -79,7 +79,7 @@ export function BrowserOverflowMenu({ electronTrpcClient.browser.isAudioMuted .query({ paneId }) .then(setIsAudioMuted) - .catch(() => {}); + .catch((error) => console.warn("[BrowserOverflowMenu] could not read audio mute state", { paneId, error })); }, [paneId]); const handleToggleMute = () => { From 16c5d87de9babf257e41d4f2e6ede2e1ec799dfb Mon Sep 17 00:00:00 2001 From: Anmol1578 Date: Thu, 5 Nov 2026 23:25:26 +0530 Subject: [PATCH 3/5] fix(desktop): address review feedback for browser pane audio muting --- .../src/lib/trpc/routers/browser/browser.ts | 2 + .../main/lib/browser/browser-manager.test.ts | 145 +++++++++++++----- .../src/main/lib/browser/browser-manager.ts | 40 ++++- .../BrowserOverflowMenu.tsx | 57 ++++++- scripts/test-preload.ts | 28 +--- 5 files changed, 201 insertions(+), 71 deletions(-) diff --git a/apps/desktop/src/lib/trpc/routers/browser/browser.ts b/apps/desktop/src/lib/trpc/routers/browser/browser.ts index 97a97ecf7ec..72453f900c0 100644 --- a/apps/desktop/src/lib/trpc/routers/browser/browser.ts +++ b/apps/desktop/src/lib/trpc/routers/browser/browser.ts @@ -49,6 +49,7 @@ export const createBrowserRouter = () => { return { success: true }; }), + /** Sets whether audio is muted for the given browser pane. */ setAudioMuted: publicProcedure .input( z.object({ @@ -61,6 +62,7 @@ export const createBrowserRouter = () => { return { success: true }; }), + /** Queries whether audio is muted for the given browser pane. */ isAudioMuted: publicProcedure .input( z.object({ diff --git a/apps/desktop/src/main/lib/browser/browser-manager.test.ts b/apps/desktop/src/main/lib/browser/browser-manager.test.ts index d1e336219a5..cd1a05acd68 100644 --- a/apps/desktop/src/main/lib/browser/browser-manager.test.ts +++ b/apps/desktop/src/main/lib/browser/browser-manager.test.ts @@ -8,24 +8,17 @@ import { mock, test, } from "bun:test"; + +mock.module("@lingui/core/macro", () => ({ + msg: (descriptor: unknown) => descriptor, + t: (descriptor: unknown) => descriptor, +})); + +import { i18n } from "@superset/i18n"; import { webContents } from "electron"; import { PROTOCOL_SCHEME } from "shared/constants"; import { browserManager } from "./browser-manager"; -mock.module("electron", () => ({ - webContents: { - fromId: mock(() => null), - }, - clipboard: { - writeText: mock(), - readText: mock(), - }, - Menu: { - buildFromTemplate: mock(() => ({ popup: mock() })), - }, - ClipboardItem: class ClipboardItem {}, -})); - const wcById = new Map(); const fromIdMock = webContents.fromId as unknown as ReturnType; @@ -33,6 +26,10 @@ const preloadFromId = fromIdMock.getMockImplementation(); beforeAll(() => { fromIdMock.mockImplementation((id: number) => wcById.get(id) ?? null); + if (!i18n.locale) { + i18n.load("en", {}); + i18n.activate("en"); + } }); afterAll(() => { @@ -902,31 +899,101 @@ describe("right-click menu", () => { describe("audio muting", () => { test("sets audio muted on guest webContents and persists across registrations", () => { const paneId = "test-audio-pane"; - const { wc: wc1, id: id1 } = makeWc(); - - // 1. Initial register: default unmuted - browserManager.register(paneId, id1); - expect(browserManager.isAudioMuted(paneId)).toBe(false); - expect(wc1.audioMuted).toBe(false); - - // 2. Explicit mute: updates both manager and webContents - browserManager.setAudioMuted(paneId, true); - expect(browserManager.isAudioMuted(paneId)).toBe(true); - expect(wc1.audioMuted).toBe(true); - - // 3. Re-register with a new webContents ID (reloads / navigation / crash recovery) - const { wc: wc2, id: id2 } = makeWc(); - browserManager.register(paneId, id2); - - // Mute state must persist and be applied to the new webContents - expect(browserManager.isAudioMuted(paneId)).toBe(true); - expect(wc2.audioMuted).toBe(true); - - // 4. Explicit unmute - browserManager.setAudioMuted(paneId, false); - expect(browserManager.isAudioMuted(paneId)).toBe(false); - expect(wc2.audioMuted).toBe(false); + registered.push(paneId); + try { + const { wc: wc1, id: id1 } = makeWc(); + + // 1. Initial register: default unmuted + browserManager.register(paneId, id1); + expect(browserManager.isAudioMuted(paneId)).toBe(false); + expect(wc1.audioMuted).toBe(false); + + // 2. Explicit mute: updates both manager and webContents + browserManager.setAudioMuted(paneId, true); + expect(browserManager.isAudioMuted(paneId)).toBe(true); + expect(wc1.audioMuted).toBe(true); + + // 3. Re-register with a new webContents ID (reloads / navigation / crash recovery) + const { wc: wc2, id: id2 } = makeWc(); + browserManager.register(paneId, id2); + + // Mute state must persist and be applied to the new webContents + expect(browserManager.isAudioMuted(paneId)).toBe(true); + expect(wc2.audioMuted).toBe(true); + + // 4. Explicit unmute + browserManager.setAudioMuted(paneId, false); + expect(browserManager.isAudioMuted(paneId)).toBe(false); + expect(wc2.audioMuted).toBe(false); + } finally { + browserManager.unregister(paneId); + } + }); - browserManager.unregister(paneId); + test("applies mute state to child popups both at creation and dynamically", () => { + const paneId = "test-popup-audio-pane"; + registered.push(paneId); + try { + const { wc: guestWc, id: guestId } = makeWc(); + browserManager.register(paneId, guestId); + + const { wc: popupWc } = makeWc(); + const windowListeners = new Map void)[]>(); + const fakeWindow = { + webContents: popupWc, + on: (event: string, fn: () => void) => { + windowListeners.set(event, [ + ...(windowListeners.get(event) ?? []), + fn, + ]); + }, + once: (event: string, fn: () => void) => { + windowListeners.set(event, [ + ...(windowListeners.get(event) ?? []), + fn, + ]); + }, + close: () => { + for (const fn of windowListeners.get("closed") ?? []) { + fn(); + } + }, + }; + + // Simulate child popup creation while unmuted + for (const listener of guestWc.listeners.get("did-create-window") ?? []) { + listener(fakeWindow); + } + expect(popupWc.audioMuted).toBe(false); + + // Muting pane updates the existing popup + browserManager.setAudioMuted(paneId, true); + expect(popupWc.audioMuted).toBe(true); + + // Unmuting pane updates the existing popup + browserManager.setAudioMuted(paneId, false); + expect(popupWc.audioMuted).toBe(false); + + // Muting pane again, then creating a second popup opens it muted + browserManager.setAudioMuted(paneId, true); + const { wc: popupWc2 } = makeWc(); + const fakeWindow2 = { + webContents: popupWc2, + on: () => {}, + once: () => {}, + }; + for (const listener of guestWc.listeners.get("did-create-window") ?? []) { + listener(fakeWindow2); + } + expect(popupWc2.audioMuted).toBe(true); + + // Closing window unhooks it + fakeWindow.close(); + browserManager.setAudioMuted(paneId, false); + // popupWc was closed, so it stays as it was + expect(popupWc2.audioMuted).toBe(false); + } finally { + browserManager.unregister(paneId); + } }); }); diff --git a/apps/desktop/src/main/lib/browser/browser-manager.ts b/apps/desktop/src/main/lib/browser/browser-manager.ts index 8f97bd0d0e9..4dfa8d593dc 100644 --- a/apps/desktop/src/main/lib/browser/browser-manager.ts +++ b/apps/desktop/src/main/lib/browser/browser-manager.ts @@ -261,6 +261,7 @@ class BrowserManager extends EventEmitter { private focusListeners = new Map void>(); private cdpDetachers = new Map void>(); private mutedPanes = new Map(); + private popupWebContents = new Map>(); // Ref-count of in-flight agent work per pane (a live CDP session, a // screenshot capture). While present the guest renderer stays // un-throttled — see acquireAgentWake. The entry object's identity ties @@ -271,17 +272,31 @@ class BrowserManager extends EventEmitter { private forwardableChords = new Set(); private designMode = new DesignModeController(); + /** Sets audio muted state for the given pane and its child popups. */ public setAudioMuted(paneId: string, muted: boolean): void { this.mutedPanes.set(paneId, muted); const entry = this.panes.get(paneId); if (entry) { const guest = webContents.fromId(entry.webContentsId); - if (guest && !guest.isDestroyed()) { + if ( + guest && + !guest.isDestroyed() && + typeof guest.setAudioMuted === "function" + ) { guest.setAudioMuted(muted); } } + const popups = this.popupWebContents.get(paneId); + if (popups) { + for (const popup of popups) { + if (!popup.isDestroyed() && typeof popup.setAudioMuted === "function") { + popup.setAudioMuted(muted); + } + } + } } + /** Returns true if the pane is currently set to mute audio. */ public isAudioMuted(paneId: string): boolean { return this.mutedPanes.get(paneId) ?? false; } @@ -322,7 +337,9 @@ class BrowserManager extends EventEmitter { if (wc) { // Apply persisted or default audio mute state const isMuted = this.isAudioMuted(paneId); - wc.setAudioMuted(isMuted); // + if (typeof wc.setAudioMuted === "function") { + wc.setAudioMuted(isMuted); + } // Throttling stays enabled by default so parked/offscreen persistent // webviews don't run at full speed in the background — except while @@ -362,6 +379,8 @@ class BrowserManager extends EventEmitter { this.designMode.cancel(paneId, "destroyed"); this.panes.delete(paneId); this.consoleLogs.delete(paneId); + this.mutedPanes.delete(paneId); + this.popupWebContents.delete(paneId); // Tell subscribers when a live wake dies with the pane, so the renderer // doesn't keep a stale pane id in its exemption set. if (this.agentWakes.delete(paneId)) this.emitAgentActive(); @@ -1105,9 +1124,16 @@ class BrowserManager extends EventEmitter { ): void { const wc = window.webContents; markBrowserPanePopup(wc); - if (this.isAudioMuted(paneId)) { + if (this.isAudioMuted(paneId) && typeof wc.setAudioMuted === "function") { wc.setAudioMuted(true); } + let popups = this.popupWebContents.get(paneId); + if (!popups) { + popups = new Set(); + this.popupWebContents.set(paneId, popups); + } + popups.add(wc); + const detachGuard = attachNavigationGuard(wc, (url) => this.emit("deep-link", url), ); @@ -1117,7 +1143,13 @@ class BrowserManager extends EventEmitter { wc.on("did-create-window", (child) => { this.configurePopupWindow(paneId, child); }); - window.on("closed", detachGuard); + window.once("closed", () => { + popups?.delete(wc); + if (popups?.size === 0) { + this.popupWebContents.delete(paneId); + } + detachGuard(); + }); } // An extension's broken menu must not take the page's own menu with it. private safeExtensionMenuItems( diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/BrowserPane/components/BrowserOverflowMenu/BrowserOverflowMenu.tsx b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/BrowserPane/components/BrowserOverflowMenu/BrowserOverflowMenu.tsx index 82ed15dc4e4..3376e2224e5 100644 --- a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/BrowserPane/components/BrowserOverflowMenu/BrowserOverflowMenu.tsx +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/BrowserPane/components/BrowserOverflowMenu/BrowserOverflowMenu.tsx @@ -8,7 +8,14 @@ import { } from "@superset/ui/dropdown-menu"; import { toast } from "@superset/ui/sonner"; import { useNavigate } from "@tanstack/react-router"; -import { CheckIcon, MinusIcon, PlusIcon, RotateCcwIcon } from "lucide-react"; +import { + CheckIcon, + MinusIcon, + PlusIcon, + RotateCcwIcon, + Volume2Icon, + VolumeXIcon, +} from "lucide-react"; import { useEffect, useState } from "react"; import { TbDots } from "react-icons/tb"; import { ImportHistoryDialog } from "renderer/components/ImportHistoryDialog"; @@ -72,22 +79,47 @@ export function BrowserOverflowMenu({ const [isScreenshotsOpen, setIsScreenshotsOpen] = useState(false); const [isClearDataOpen, setIsClearDataOpen] = useState(false); const [isMenuOpen, setIsMenuOpen] = useState(false); - const [isAudioMuted, setIsAudioMuted] = useState(false); + const [audioMuteState, setAudioMuteState] = useState<{ + paneId: string; + muted: boolean; + } | null>(null); + + const isAudioMuteLoaded = audioMuteState?.paneId === paneId; + const isAudioMuted = isAudioMuteLoaded ? audioMuteState.muted : false; useEffect(() => { if (!paneId) return; + let cancelled = false; electronTrpcClient.browser.isAudioMuted .query({ paneId }) - .then(setIsAudioMuted) - .catch((error) => console.warn("[BrowserOverflowMenu] could not read audio mute state", { paneId, error })); + .then((muted) => { + if (!cancelled) { + setAudioMuteState({ paneId, muted }); + } + }) + .catch((error) => { + console.warn("[BrowserOverflowMenu] could not read audio mute state", { + paneId, + error, + }); + }); + return () => { + cancelled = true; + }; }, [paneId]); const handleToggleMute = () => { + if (!paneId || !isAudioMuteLoaded) return; const next = !isAudioMuted; electronTrpcClient.browser.setAudioMuted .mutate({ paneId, muted: next }) - .then(() => setIsAudioMuted(next)) - .catch(() => {}); + .then(() => setAudioMuteState({ paneId, muted: next })) + .catch((error) => { + console.warn("[BrowserOverflowMenu] could not set audio mute state", { + paneId, + error, + }); + }); }; // The webview swallows pointer events, so a click on the page would never @@ -274,8 +306,17 @@ export function BrowserOverflowMenu({ Take a screenshot - - {isAudioMuted ? Unmute tab : Mute tab} + + {isAudioMuted ? Unmute : Mute} + {isAudioMuted ? ( + + ) : ( + + )} Hard reload diff --git a/scripts/test-preload.ts b/scripts/test-preload.ts index 6936b816213..8d4c6f15c15 100644 --- a/scripts/test-preload.ts +++ b/scripts/test-preload.ts @@ -25,6 +25,7 @@ * that has one; `test/superset-home-isolation.test.ts` fails loudly if a run * ever loses it. */ + import { afterAll, mock } from "bun:test"; import { mkdtempSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; @@ -106,26 +107,6 @@ mock.module("electron", () => ({ ClipboardItem: class ClipboardItem {}, })); -// Mock Lingui macros -mock.module("@lingui/core/macro", () => ({ - msg: (descriptor: { message: string } | string) => - typeof descriptor === "string" ? descriptor : descriptor.message, - t: (descriptor: { message: string } | string) => - typeof descriptor === "string" ? descriptor : descriptor.message, -})); - -// Mock @superset/i18n runtime instance so i18n._ never throws uninitialized locale errors -mock.module("@superset/i18n", () => ({ - i18n: { - _: (id: unknown) => - typeof id === "string" - ? id - : ((id as { message?: string })?.message ?? String(id)), - activate: () => {}, - load: () => {}, - }, -})); - const testSupersetHome = mkdtempSync(join(tmpdir(), "superset-test-home-")); process.env.SUPERSET_HOME_DIR = testSupersetHome; Reflect.set( @@ -134,6 +115,10 @@ Reflect.set( testSupersetHome, ); +// Test processes are short-lived, but a full local run creates enough state +// here to make relying on the OS's eventual temp cleanup needlessly noisy. +// The exit hook is synchronous because Node/Bun cannot await async work once +// process shutdown has begun; best-effort cleanup must never mask test status. function cleanupTestSupersetHome(): void { try { rmSync(testSupersetHome, { recursive: true, force: true }); @@ -142,5 +127,8 @@ function cleanupTestSupersetHome(): void { } } +// Bun's test runner does not consistently emit Node's process exit event, so +// use its lifecycle hook as the primary cleanup and retain exit as a fallback +// for alternate runners that load the same preload. afterAll(cleanupTestSupersetHome); process.once("exit", cleanupTestSupersetHome); From 47c1536c0b2c634ceca47efdb1df5033d7565ef4 Mon Sep 17 00:00:00 2001 From: Anmol1578 Date: Thu, 5 Nov 2026 23:30:10 +0530 Subject: [PATCH 4/5] fix(desktop): restore docstring and complete electron test mocks --- .../src/main/lib/browser/browser-manager.ts | 14 ++-- scripts/test-preload.ts | 74 +++++++++++++++---- 2 files changed, 69 insertions(+), 19 deletions(-) diff --git a/apps/desktop/src/main/lib/browser/browser-manager.ts b/apps/desktop/src/main/lib/browser/browser-manager.ts index 4dfa8d593dc..666b6cf79d7 100644 --- a/apps/desktop/src/main/lib/browser/browser-manager.ts +++ b/apps/desktop/src/main/lib/browser/browser-manager.ts @@ -305,10 +305,6 @@ class BrowserManager extends EventEmitter { this.forwardableChords = new Set(chords); } - public setExtensionMenuItems(items: ExtensionMenuItems): void { - this.extensionMenuItems = items; - } - register(paneId: string, webContentsId: number, workspaceId?: string): void { // Clean even when prevId === webContentsId so BrowserManager owns // listener idempotency; callers can re-register without duplicating. @@ -1124,8 +1120,8 @@ class BrowserManager extends EventEmitter { ): void { const wc = window.webContents; markBrowserPanePopup(wc); - if (this.isAudioMuted(paneId) && typeof wc.setAudioMuted === "function") { - wc.setAudioMuted(true); + if (typeof wc.setAudioMuted === "function") { + wc.setAudioMuted(this.isAudioMuted(paneId)); } let popups = this.popupWebContents.get(paneId); if (!popups) { @@ -1151,6 +1147,12 @@ class BrowserManager extends EventEmitter { detachGuard(); }); } + + /** Items extensions add with `chrome.contextMenus`, for the pane's right-click menu. */ + setExtensionMenuItems(items: ExtensionMenuItems): void { + this.extensionMenuItems = items; + } + // An extension's broken menu must not take the page's own menu with it. private safeExtensionMenuItems( ...args: Parameters diff --git a/scripts/test-preload.ts b/scripts/test-preload.ts index 8d4c6f15c15..7b304a81fbd 100644 --- a/scripts/test-preload.ts +++ b/scripts/test-preload.ts @@ -37,8 +37,9 @@ mock.module("electron", () => ({ fromId: mock(() => null), }, clipboard: { - writeText: mock(), - readText: mock(), + write: mock(() => Promise.resolve()), + writeText: mock(() => Promise.resolve()), + readText: mock(() => ""), writeImage: mock(), }, Menu: { @@ -46,6 +47,7 @@ mock.module("electron", () => ({ popup: mock(), items, })), + setApplicationMenu: mock(), }, shell: { openExternal: mock(async () => {}), @@ -56,23 +58,37 @@ mock.module("electron", () => ({ app: { isPackaged: false, getVersion: () => "0.0.0", + getName: () => "test-app", getAppPath: () => "/tmp/app", getPath: (name: string) => `/tmp/${name}`, on: mock(), whenReady: mock(async () => {}), }, - BrowserWindow: class BrowserWindow { - webContents = { + BrowserWindow: Object.assign( + mock(() => ({ + webContents: { + on: mock(), + off: mock(), + setAudioMuted: mock(), + setWindowOpenHandler: mock(), + send: mock(), + }, on: mock(), - off: mock(), - setAudioMuted: mock(), - setWindowOpenHandler: mock(), - }; - on = mock(); - loadURL = mock(async () => {}); - close = mock(); - destroy = mock(); - }, + once: mock(), + loadURL: mock(async () => {}), + close: mock(), + destroy: mock(), + })), + { getFocusedWindow: mock(() => null), getAllWindows: mock(() => []) }, + ), + WebContentsView: mock(() => ({ + webContents: { + send: mock(), + once: mock(), + loadURL: mock(() => Promise.resolve()), + close: mock(), + }, + })), nativeImage: { createEmpty: () => ({ isEmpty: () => true, @@ -93,6 +109,12 @@ mock.module("electron", () => ({ removeHandler: mock(), }, dialog: { + showOpenDialog: mock(() => + Promise.resolve({ canceled: false, filePaths: [] }), + ), + showSaveDialog: mock(() => + Promise.resolve({ canceled: false, filePath: "" }), + ), showErrorBox: mock(), showMessageBox: mock(async () => ({ response: 0 })), }, @@ -103,7 +125,33 @@ mock.module("electron", () => ({ onHeadersReceived: mock(), }, }, + fromPartition: mock(() => ({})), + }, + screen: { + getPrimaryDisplay: mock(() => ({ + workAreaSize: { width: 1920, height: 1080 }, + bounds: { x: 0, y: 0, width: 1920, height: 1080 }, + })), + getAllDisplays: mock(() => [ + { + bounds: { x: 0, y: 0, width: 1920, height: 1080 }, + workAreaSize: { width: 1920, height: 1080 }, + }, + ]), + }, + Notification: mock(() => ({ + show: mock(), + on: mock(), + })), + powerMonitor: { + getSystemIdleState: mock(() => "active"), + }, + contextBridge: { + executeInMainWorld: mock(), + exposeInMainWorld: mock(), }, + ipcRenderer: undefined, + net: {}, ClipboardItem: class ClipboardItem {}, })); From 28fb2a065a99511807d7ac595bf31a1f70e50c14 Mon Sep 17 00:00:00 2001 From: Anmol1578 Date: Thu, 5 Nov 2026 23:41:30 +0530 Subject: [PATCH 5/5] docs(desktop): add missing docstrings for browser mute and router functions --- apps/desktop/src/lib/trpc/routers/browser/browser.ts | 1 + .../src/main/lib/browser/browser-manager.test.ts | 2 ++ apps/desktop/src/main/lib/browser/browser-manager.ts | 6 +++--- .../BrowserOverflowMenu/BrowserOverflowMenu.tsx | 5 +++++ scripts/test-preload.ts | 12 ++++++++---- 5 files changed, 19 insertions(+), 7 deletions(-) diff --git a/apps/desktop/src/lib/trpc/routers/browser/browser.ts b/apps/desktop/src/lib/trpc/routers/browser/browser.ts index 72453f900c0..f9a34e92329 100644 --- a/apps/desktop/src/lib/trpc/routers/browser/browser.ts +++ b/apps/desktop/src/lib/trpc/routers/browser/browser.ts @@ -12,6 +12,7 @@ import { z } from "zod"; import { publicProcedure, router } from "../.."; import { createBrowserExtensionsRouter } from "./extensions"; +/** Creates the tRPC router for browser pane management. */ export const createBrowserRouter = () => { return router({ extensions: createBrowserExtensionsRouter(), diff --git a/apps/desktop/src/main/lib/browser/browser-manager.test.ts b/apps/desktop/src/main/lib/browser/browser-manager.test.ts index cd1a05acd68..292b4dfa4e5 100644 --- a/apps/desktop/src/main/lib/browser/browser-manager.test.ts +++ b/apps/desktop/src/main/lib/browser/browser-manager.test.ts @@ -80,6 +80,8 @@ interface FakeWebContents { }; } let nextId = 1; + +/** Creates a fake WebContents test stub with event emitter and mute tracking. */ function makeWc(): { wc: FakeWebContents; id: number } { const throttlingCalls: boolean[] = []; const wc: FakeWebContents = { diff --git a/apps/desktop/src/main/lib/browser/browser-manager.ts b/apps/desktop/src/main/lib/browser/browser-manager.ts index 666b6cf79d7..98eb5e9d091 100644 --- a/apps/desktop/src/main/lib/browser/browser-manager.ts +++ b/apps/desktop/src/main/lib/browser/browser-manager.ts @@ -273,7 +273,7 @@ class BrowserManager extends EventEmitter { private designMode = new DesignModeController(); /** Sets audio muted state for the given pane and its child popups. */ - public setAudioMuted(paneId: string, muted: boolean): void { + setAudioMuted(paneId: string, muted: boolean): void { this.mutedPanes.set(paneId, muted); const entry = this.panes.get(paneId); if (entry) { @@ -297,11 +297,11 @@ class BrowserManager extends EventEmitter { } /** Returns true if the pane is currently set to mute audio. */ - public isAudioMuted(paneId: string): boolean { + isAudioMuted(paneId: string): boolean { return this.mutedPanes.get(paneId) ?? false; } - public setForwardableChords(chords: string[]): void { + setForwardableChords(chords: string[]): void { this.forwardableChords = new Set(chords); } diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/BrowserPane/components/BrowserOverflowMenu/BrowserOverflowMenu.tsx b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/BrowserPane/components/BrowserOverflowMenu/BrowserOverflowMenu.tsx index 3376e2224e5..9463aca5388 100644 --- a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/BrowserPane/components/BrowserOverflowMenu/BrowserOverflowMenu.tsx +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/BrowserPane/components/BrowserOverflowMenu/BrowserOverflowMenu.tsx @@ -60,6 +60,10 @@ function openAfterClose(setOpen: (open: boolean) => void) { }; } +/** + * Renders the overflow dropdown menu for browser pane actions, including + * zoom controls, developer tools, audio muting, navigation history, and clear data. + */ export function BrowserOverflowMenu({ paneId, currentUrl, @@ -108,6 +112,7 @@ export function BrowserOverflowMenu({ }; }, [paneId]); + /** Toggles the audio mute state for the active browser pane. */ const handleToggleMute = () => { if (!paneId || !isAudioMuteLoaded) return; const next = !isAudioMuted; diff --git a/scripts/test-preload.ts b/scripts/test-preload.ts index 7b304a81fbd..c8dd3a3c37e 100644 --- a/scripts/test-preload.ts +++ b/scripts/test-preload.ts @@ -163,10 +163,14 @@ Reflect.set( testSupersetHome, ); -// Test processes are short-lived, but a full local run creates enough state -// here to make relying on the OS's eventual temp cleanup needlessly noisy. -// The exit hook is synchronous because Node/Bun cannot await async work once -// process shutdown has begun; best-effort cleanup must never mask test status. +/** + * Cleans up the isolated test superset home directory synchronously on exit. + * + * Test processes are short-lived, but a full local run creates enough state + * here to make relying on the OS's eventual temp cleanup needlessly noisy. + * The exit hook is synchronous because Node/Bun cannot await async work once + * process shutdown has begun; best-effort cleanup must never mask test status. + */ function cleanupTestSupersetHome(): void { try { rmSync(testSupersetHome, { recursive: true, force: true });