From 2bb17ad7117623e0c305e357b6f4b915ebce3e2c Mon Sep 17 00:00:00 2001 From: Barack Sokullu Date: Fri, 24 Jul 2026 02:06:36 +0300 Subject: [PATCH] Fix detached side panel window routing --- src/chrome/src/ui/sidepanel-window-scope.js | 110 +++++++++++++++++++ src/chrome/src/ui/sidepanel.js | 38 ++++--- src/firefox/src/ui/sidepanel-window-scope.js | 110 +++++++++++++++++++ src/firefox/src/ui/sidepanel.js | 31 ++++-- test/run.js | 81 +++++++++++++- 5 files changed, 344 insertions(+), 26 deletions(-) create mode 100644 src/chrome/src/ui/sidepanel-window-scope.js create mode 100644 src/firefox/src/ui/sidepanel-window-scope.js diff --git a/src/chrome/src/ui/sidepanel-window-scope.js b/src/chrome/src/ui/sidepanel-window-scope.js new file mode 100644 index 000000000..5d9a6ec9a --- /dev/null +++ b/src/chrome/src/ui/sidepanel-window-scope.js @@ -0,0 +1,110 @@ +function sameTabId(a, b) { + if (a == null || b == null) return false; + return String(a) === String(b); +} + +/** + * Keep one side-panel document scoped to the browser window that currently + * contains it. Tab activation events are extension-wide, and a browser can keep + * the same panel document alive while its tab/group is dragged to a different + * window, so a window ID captured once at startup eventually becomes stale. + */ +export function createSidePanelWindowScope({ + browserApi, + initialWindowId = null, + getCurrentTabId, + getRenderedTabId, + switchToTab, + settleWindowTransfer = () => new Promise(resolve => setTimeout(resolve, 0)), +}) { + let ownWindowId = initialWindowId; + let tabTransfer = null; + let syncGeneration = 0; + + async function refreshOwnWindowId() { + try { + const ownWindow = await browserApi.windows.getCurrent(); + if (ownWindow?.id != null) ownWindowId = ownWindow.id; + } catch { + // Keep the last confirmed ID. A transient windows API failure should + // not make an unrelated window eligible to control this panel. + } + return ownWindowId; + } + + async function syncActiveTab({ expectedWindowId = null, expectedTabId = null } = {}) { + const windowId = await refreshOwnWindowId(); + if (windowId == null) return null; + if (expectedWindowId != null && expectedWindowId !== windowId) return null; + // Only a confirmed event from this panel's window supersedes an older + // in-flight sync. Noise from another browser window must not cancel it. + const generation = ++syncGeneration; + + let activeTab = null; + try { + [activeTab] = await browserApi.tabs.query({ active: true, windowId }); + } catch { + return null; + } + if (generation !== syncGeneration || !activeTab?.id) return null; + if (activeTab.windowId != null && activeTab.windowId !== windowId) return null; + if (expectedTabId != null && !sameTabId(activeTab.id, expectedTabId)) return null; + + await switchToTab(activeTab.id); + return activeTab; + } + + function handleDetached(tabId, detachInfo = {}) { + if (!sameTabId(tabId, getCurrentTabId()) && !sameTabId(tabId, getRenderedTabId())) { + return false; + } + tabTransfer = { + tabId, + oldWindowId: detachInfo.oldWindowId ?? ownWindowId, + }; + // The cached owner is intentionally invalid during transfer. If the live + // windows lookup fails after attach, fail closed instead of falling back + // to the window the panel just left. + ownWindowId = null; + // Invalidate an activation lookup that may have started just before the + // detach event. Its result belongs to the window the panel is leaving. + syncGeneration += 1; + return true; + } + + async function handleAttached(tabId) { + if (!tabTransfer || !sameTabId(tabId, tabTransfer.tabId)) return null; + const transfer = tabTransfer; + // Let the browser finish reparenting the side-panel document before + // asking getCurrent() where it lives. During this turn, old-window + // activation events remain suppressed by tabTransfer. + await settleWindowTransfer(); + if (tabTransfer !== transfer) return null; + tabTransfer = null; + // getCurrent() tells us whether the browser moved this panel document with + // the tab or kept it in the old window. Follow the actual panel location. + return await syncActiveTab(); + } + + async function handleActivated(info = {}) { + if (tabTransfer && info.windowId === tabTransfer.oldWindowId) return null; + return await syncActiveTab({ + expectedWindowId: info.windowId, + expectedTabId: info.tabId, + }); + } + + async function handleFocusChanged(windowId, windowIdNone) { + if (windowId === windowIdNone) return null; + if (tabTransfer && windowId === tabTransfer.oldWindowId) return null; + return await syncActiveTab({ expectedWindowId: windowId }); + } + + return { + syncActiveTab, + handleDetached, + handleAttached, + handleActivated, + handleFocusChanged, + }; +} diff --git a/src/chrome/src/ui/sidepanel.js b/src/chrome/src/ui/sidepanel.js index e2344f347..e846a8f50 100644 --- a/src/chrome/src/ui/sidepanel.js +++ b/src/chrome/src/ui/sidepanel.js @@ -34,6 +34,7 @@ import { } from './store-review-prompt.js'; import { providerIconUrl } from './provider-icons.js'; import { TAB_CHAT_PREFIX, persistTabChatToSession } from './tab-chat-persistence.js'; +import { createSidePanelWindowScope } from './sidepanel-window-scope.js'; // Hydrate the theme from chrome.storage.local (the inline bootstrap // only sees localStorage; if the user changes the theme on another device @@ -3346,7 +3347,11 @@ function clearScratchpad(tabId = currentTabId) { // --- Initialization --- async function init() { - const [tab] = await chrome.tabs.query({ active: true, currentWindow: true }); + const initialWindow = await chrome.windows.getCurrent().catch(() => null); + const initialWindowId = initialWindow?.id ?? null; + const [tab] = await chrome.tabs.query(initialWindowId != null + ? { active: true, windowId: initialWindowId } + : { active: true, currentWindow: true }); currentTabId = tab?.id; renderedTabId = currentTabId; @@ -3354,21 +3359,29 @@ async function init() { // browser window fires them, and each window has its own side panel // instance. Without scoping, activity in window B would silently // retarget window A's panel to B's tab. - const ownWindowId = tab?.windowId ?? (await chrome.windows.getCurrent()).id; + const windowScope = createSidePanelWindowScope({ + browserApi: chrome, + initialWindowId: initialWindowId ?? tab?.windowId ?? null, + getCurrentTabId: () => currentTabId, + getRenderedTabId: () => renderedTabId, + switchToTab, + }); chrome.tabs.onActivated.addListener(async (info) => { - if (info.windowId !== ownWindowId) return; - switchToTab(info.tabId); + await windowScope.handleActivated(info); + }); + + chrome.tabs.onDetached.addListener((tabId, detachInfo) => { + windowScope.handleDetached(tabId, detachInfo); + }); + + chrome.tabs.onAttached.addListener(async (tabId, attachInfo) => { + await windowScope.handleAttached(tabId, attachInfo); }); // Also handle window focus changes chrome.windows.onFocusChanged.addListener(async (windowId) => { - if (windowId === chrome.windows.WINDOW_ID_NONE) return; - if (windowId !== ownWindowId) return; - const [tab] = await chrome.tabs.query({ active: true, windowId }); - if (tab?.id && tab.id !== currentTabId) { - switchToTab(tab.id); - } + await windowScope.handleFocusChanged(windowId, chrome.windows.WINDOW_ID_NONE); }); chrome.tabs.onUpdated?.addListener?.((tabId, changeInfo) => { @@ -3403,10 +3416,7 @@ async function init() { await loadProviders(); await testConnection({ skipWebBrainCloud: true }); - const [activeTab] = await chrome.tabs.query({ active: true, currentWindow: true }); - if (activeTab?.id && activeTab.id !== currentTabId) { - await switchToTab(activeTab.id); - } + await windowScope.syncActiveTab(); refreshScheduledJobs({ tabId: currentTabId }); refreshRecommendedActions(); await consumePendingContextMenuPrompt(); diff --git a/src/firefox/src/ui/sidepanel-window-scope.js b/src/firefox/src/ui/sidepanel-window-scope.js new file mode 100644 index 000000000..5d9a6ec9a --- /dev/null +++ b/src/firefox/src/ui/sidepanel-window-scope.js @@ -0,0 +1,110 @@ +function sameTabId(a, b) { + if (a == null || b == null) return false; + return String(a) === String(b); +} + +/** + * Keep one side-panel document scoped to the browser window that currently + * contains it. Tab activation events are extension-wide, and a browser can keep + * the same panel document alive while its tab/group is dragged to a different + * window, so a window ID captured once at startup eventually becomes stale. + */ +export function createSidePanelWindowScope({ + browserApi, + initialWindowId = null, + getCurrentTabId, + getRenderedTabId, + switchToTab, + settleWindowTransfer = () => new Promise(resolve => setTimeout(resolve, 0)), +}) { + let ownWindowId = initialWindowId; + let tabTransfer = null; + let syncGeneration = 0; + + async function refreshOwnWindowId() { + try { + const ownWindow = await browserApi.windows.getCurrent(); + if (ownWindow?.id != null) ownWindowId = ownWindow.id; + } catch { + // Keep the last confirmed ID. A transient windows API failure should + // not make an unrelated window eligible to control this panel. + } + return ownWindowId; + } + + async function syncActiveTab({ expectedWindowId = null, expectedTabId = null } = {}) { + const windowId = await refreshOwnWindowId(); + if (windowId == null) return null; + if (expectedWindowId != null && expectedWindowId !== windowId) return null; + // Only a confirmed event from this panel's window supersedes an older + // in-flight sync. Noise from another browser window must not cancel it. + const generation = ++syncGeneration; + + let activeTab = null; + try { + [activeTab] = await browserApi.tabs.query({ active: true, windowId }); + } catch { + return null; + } + if (generation !== syncGeneration || !activeTab?.id) return null; + if (activeTab.windowId != null && activeTab.windowId !== windowId) return null; + if (expectedTabId != null && !sameTabId(activeTab.id, expectedTabId)) return null; + + await switchToTab(activeTab.id); + return activeTab; + } + + function handleDetached(tabId, detachInfo = {}) { + if (!sameTabId(tabId, getCurrentTabId()) && !sameTabId(tabId, getRenderedTabId())) { + return false; + } + tabTransfer = { + tabId, + oldWindowId: detachInfo.oldWindowId ?? ownWindowId, + }; + // The cached owner is intentionally invalid during transfer. If the live + // windows lookup fails after attach, fail closed instead of falling back + // to the window the panel just left. + ownWindowId = null; + // Invalidate an activation lookup that may have started just before the + // detach event. Its result belongs to the window the panel is leaving. + syncGeneration += 1; + return true; + } + + async function handleAttached(tabId) { + if (!tabTransfer || !sameTabId(tabId, tabTransfer.tabId)) return null; + const transfer = tabTransfer; + // Let the browser finish reparenting the side-panel document before + // asking getCurrent() where it lives. During this turn, old-window + // activation events remain suppressed by tabTransfer. + await settleWindowTransfer(); + if (tabTransfer !== transfer) return null; + tabTransfer = null; + // getCurrent() tells us whether the browser moved this panel document with + // the tab or kept it in the old window. Follow the actual panel location. + return await syncActiveTab(); + } + + async function handleActivated(info = {}) { + if (tabTransfer && info.windowId === tabTransfer.oldWindowId) return null; + return await syncActiveTab({ + expectedWindowId: info.windowId, + expectedTabId: info.tabId, + }); + } + + async function handleFocusChanged(windowId, windowIdNone) { + if (windowId === windowIdNone) return null; + if (tabTransfer && windowId === tabTransfer.oldWindowId) return null; + return await syncActiveTab({ expectedWindowId: windowId }); + } + + return { + syncActiveTab, + handleDetached, + handleAttached, + handleActivated, + handleFocusChanged, + }; +} diff --git a/src/firefox/src/ui/sidepanel.js b/src/firefox/src/ui/sidepanel.js index 6af4b6a92..25c14a97e 100644 --- a/src/firefox/src/ui/sidepanel.js +++ b/src/firefox/src/ui/sidepanel.js @@ -34,6 +34,7 @@ import { } from './store-review-prompt.js'; import { providerIconUrl } from './provider-icons.js'; import { TAB_CHAT_PREFIX, persistTabChatToSession } from './tab-chat-persistence.js'; +import { createSidePanelWindowScope } from './sidepanel-window-scope.js'; // Hydrate the theme from browser.storage.local (the inline bootstrap // only sees localStorage; if the user changes the theme on another device @@ -3203,7 +3204,11 @@ function clearScratchpad(tabId = currentTabId) { // --- Initialization --- async function init() { - const [tab] = await browser.tabs.query({ active: true, currentWindow: true }); + const initialWindow = await browser.windows.getCurrent().catch(() => null); + const initialWindowId = initialWindow?.id ?? null; + const [tab] = await browser.tabs.query(initialWindowId != null + ? { active: true, windowId: initialWindowId } + : { active: true, currentWindow: true }); currentTabId = tab?.id; renderedTabId = currentTabId; @@ -3211,11 +3216,24 @@ async function init() { // them, and each window has its own side panel instance. Without // scoping, activity in window B would silently retarget window A's // panel to B's tab. - const ownWindowId = tab?.windowId ?? (await browser.windows.getCurrent()).id; + const windowScope = createSidePanelWindowScope({ + browserApi: browser, + initialWindowId: initialWindowId ?? tab?.windowId ?? null, + getCurrentTabId: () => currentTabId, + getRenderedTabId: () => renderedTabId, + switchToTab, + }); browser.tabs.onActivated.addListener(async (info) => { - if (info.windowId !== ownWindowId) return; - await switchToTab(info.tabId); + await windowScope.handleActivated(info); + }); + + browser.tabs.onDetached.addListener((tabId, detachInfo) => { + windowScope.handleDetached(tabId, detachInfo); + }); + + browser.tabs.onAttached.addListener(async (tabId, attachInfo) => { + await windowScope.handleAttached(tabId, attachInfo); }); browser.tabs.onUpdated?.addListener?.((tabId, changeInfo) => { @@ -3250,10 +3268,7 @@ async function init() { await loadProviders(); await testConnection({ skipWebBrainCloud: true }); - const [activeTab] = await browser.tabs.query({ active: true, currentWindow: true }); - if (activeTab?.id && activeTab.id !== currentTabId) { - await switchToTab(activeTab.id); - } + await windowScope.syncActiveTab(); refreshScheduledJobs({ tabId: currentTabId }); refreshRecommendedActions(); await consumePendingContextMenuPrompt(); diff --git a/test/run.js b/test/run.js index 3904161cf..bd3d7ebd0 100644 --- a/test/run.js +++ b/test/run.js @@ -15302,23 +15302,26 @@ test('sidepanel does not miss startup tab switches before consuming tab-scoped s assert.notEqual(end, -1, `${label}: init boundary missing`); const body = panel.slice(start, end); const listenerIdx = body.indexOf('tabs.onActivated.addListener'); + const detachListenerIdx = body.indexOf('tabs.onDetached.addListener'); + const attachListenerIdx = body.indexOf('tabs.onAttached.addListener'); const loadProvidersIdx = body.indexOf('await loadProviders();'); const testConnectionIdx = body.indexOf("await testConnection({ skipWebBrainCloud: true });"); - const resyncQueryIdx = body.lastIndexOf('tabs.query({ active: true, currentWindow: true })'); - const resyncSwitchIdx = body.indexOf('await switchToTab(activeTab.id);'); + const resyncSwitchIdx = body.indexOf('await windowScope.syncActiveTab();'); const refreshJobsIdx = body.indexOf('refreshScheduledJobs({ tabId: currentTabId });', resyncSwitchIdx); const refreshActionsIdx = body.indexOf('refreshRecommendedActions();', resyncSwitchIdx); const consumeIdx = body.indexOf('await consumePendingContextMenuPrompt();', resyncSwitchIdx); assert.notEqual(listenerIdx, -1, `${label}: startup should register tab activation listener`); + assert.notEqual(detachListenerIdx, -1, `${label}: startup should register tab detach listener`); + assert.notEqual(attachListenerIdx, -1, `${label}: startup should register tab attach listener`); assert.notEqual(loadProvidersIdx, -1, `${label}: startup provider load missing`); assert.notEqual(testConnectionIdx, -1, `${label}: startup connection test missing`); - assert.notEqual(resyncQueryIdx, -1, `${label}: startup should re-query the active tab after async setup`); assert.notEqual(resyncSwitchIdx, -1, `${label}: startup should switch to the active tab after async setup`); assert.notEqual(refreshJobsIdx, -1, `${label}: startup scheduled-job refresh missing`); assert.notEqual(refreshActionsIdx, -1, `${label}: startup recommended-action refresh missing`); assert.notEqual(consumeIdx, -1, `${label}: startup context-menu consume missing`); assert.equal(listenerIdx < loadProvidersIdx, true, `${label}: tab activation listener must be registered before startup awaits`); - assert.equal(testConnectionIdx < resyncQueryIdx && resyncQueryIdx < resyncSwitchIdx, true, `${label}: startup should resync the active tab after async setup`); + assert.equal(detachListenerIdx < loadProvidersIdx && attachListenerIdx < loadProvidersIdx, true, `${label}: tab transfer listeners must be registered before startup awaits`); + assert.equal(testConnectionIdx < resyncSwitchIdx, true, `${label}: startup should resync the active tab after async setup`); assert.equal(resyncSwitchIdx < refreshJobsIdx && resyncSwitchIdx < refreshActionsIdx && resyncSwitchIdx < consumeIdx, true, `${label}: startup must resync before tab-scoped refreshes and context-menu consume`); if (label === 'chrome') { @@ -15333,6 +15336,76 @@ test('sidepanel does not miss startup tab switches before consuming tab-scoped s } }); +test('sidepanel follows its live window when the represented tab is detached', async () => { + const chromeRel = 'src/chrome/src/ui/sidepanel-window-scope.js'; + const firefoxRel = 'src/firefox/src/ui/sidepanel-window-scope.js'; + const chromeSource = fs.readFileSync(path.join(ROOT, chromeRel), 'utf8'); + const firefoxSource = fs.readFileSync(path.join(ROOT, firefoxRel), 'utf8'); + assert.equal(firefoxSource, chromeSource, 'sidepanel window-transfer routing should stay mirrored'); + + for (const [label, rel] of [['chrome', chromeRel], ['firefox', firefoxRel]]) { + const { createSidePanelWindowScope } = await import(pathToFileURL(path.join(ROOT, rel)).href); + let containingWindowId = 10; + let currentTabId = 101; + let renderedTabId = 101; + const activeTabs = new Map([ + [10, { id: 101, windowId: 10 }], + [20, { id: 101, windowId: 20 }], + ]); + const queries = []; + const switches = []; + let releaseWindowTransfer; + const browserApi = { + windows: { + getCurrent: async () => ({ id: containingWindowId }), + }, + tabs: { + query: async (query) => { + queries.push(query); + return activeTabs.has(query.windowId) ? [activeTabs.get(query.windowId)] : []; + }, + }, + }; + const scope = createSidePanelWindowScope({ + browserApi, + initialWindowId: 10, + getCurrentTabId: () => currentTabId, + getRenderedTabId: () => renderedTabId, + settleWindowTransfer: () => new Promise(resolve => { + releaseWindowTransfer = resolve; + }), + switchToTab: async (tabId) => { + switches.push(tabId); + currentTabId = tabId; + renderedTabId = tabId; + }, + }); + + assert.equal(scope.handleDetached(101, { oldWindowId: 10 }), true, `${label}: represented tab detach should start a transfer`); + activeTabs.set(10, { id: 102, windowId: 10 }); + await scope.handleActivated({ tabId: 102, windowId: 10 }); + assert.deepEqual(switches, [], `${label}: old-window activation must be ignored while the panel tab is transferring`); + + const attaching = scope.handleAttached(101, { newWindowId: 20 }); + await scope.handleActivated({ tabId: 102, windowId: 10 }); + assert.deepEqual(switches, [], `${label}: old-window activation must stay suppressed while attach is settling`); + containingWindowId = 20; + releaseWindowTransfer(); + await attaching; + assert.deepEqual(switches, [101], `${label}: attach should resync to the active tab in the panel's new window`); + switches.length = 0; + + activeTabs.set(10, { id: 103, windowId: 10 }); + await scope.handleActivated({ tabId: 103, windowId: 10 }); + assert.deepEqual(switches, [], `${label}: activity in the former window must not retarget the detached panel`); + + activeTabs.set(20, { id: 201, windowId: 20 }); + await scope.handleActivated({ tabId: 201, windowId: 20 }); + assert.deepEqual(switches, [201], `${label}: activity in the panel's live window should still switch tab-scoped chat`); + assert.equal(queries.every(query => query.currentWindow == null && query.windowId != null), true, `${label}: active-tab queries must use an explicit live window ID`); + } +}); + test('sidepanel drops stale recommended-action refreshes after tab changes or run start', () => { for (const [label, panelRel] of [ ['chrome', 'src/chrome/src/ui/sidepanel.js'],