From 3d24a717966d0d676713bcd6e7461feff57737db Mon Sep 17 00:00:00 2001 From: ujiro99 Date: Thu, 22 Oct 2026 14:09:46 +0900 Subject: [PATCH 1/3] feat(extension): reuse existing options page tab when opening settings Use chrome.runtime.openOptionsPage() for the action icon click and the openOption command so an already opened options page in the current window is activated instead of opening a duplicate tab. Co-Authored-By: Claude Opus 5.5 --- packages/extension/src/service_worker.ts | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/packages/extension/src/service_worker.ts b/packages/extension/src/service_worker.ts index 602f4b7d..34eca47b 100644 --- a/packages/extension/src/service_worker.ts +++ b/packages/extension/src/service_worker.ts @@ -155,9 +155,7 @@ const commandFuncs = { [ServiceWorkerCommand.execApi]: ActionHelper.execApi, [ServiceWorkerCommand.openOption]: (): boolean => { - chrome.tabs.create({ - url: OPTION_PAGE_PATH, - }) + chrome.runtime.openOptionsPage() return false }, @@ -391,9 +389,8 @@ const updateActiveTabId = async (activeTabId?: number) => { } chrome.action.onClicked.addListener(() => { - chrome.tabs.create({ - url: OPTION_PAGE_PATH, - }) + // Reuses the existing options page tab in the current window if any. + chrome.runtime.openOptionsPage() }) chrome.windows.onFocusChanged.addListener(async (windowId: number) => { From 31f93e20b1a32722634a1a45ea9b9d0d2d2b7705 Mon Sep 17 00:00:00 2001 From: ujiro99 Date: Fri, 23 Oct 2026 11:00:49 +0900 Subject: [PATCH 2/3] feat(extension): reuse options page tab across windows and for parameterized URLs Replace chrome.runtime.openOptionsPage() with an openOptionPage() helper that searches all windows for an existing options page tab, and use it for the action icon, the openOption command, adding/editing page rules, and editing a command from the Hub. When a query is given, the existing tab is navigated to it so the target dialog opens. When the Hub edit reuses an existing tab, the tab is kept open after the edit instead of being closed. Co-Authored-By: Claude Opus 5.5 --- .../src/__tests__/service_worker.test.ts | 9 +- packages/extension/src/service_worker.ts | 28 +++-- .../__tests__/chrome.openOptionPage.test.ts | 71 ++++++++++++ packages/extension/src/services/chrome.ts | 30 ++++- .../hub/__tests__/serviceWorker.test.ts | 106 +++++++++++------- .../src/services/hub/serviceWorker.ts | 33 ++++-- 6 files changed, 205 insertions(+), 72 deletions(-) create mode 100644 packages/extension/src/services/__tests__/chrome.openOptionPage.test.ts diff --git a/packages/extension/src/__tests__/service_worker.test.ts b/packages/extension/src/__tests__/service_worker.test.ts index f2f330d4..98ade4ab 100644 --- a/packages/extension/src/__tests__/service_worker.test.ts +++ b/packages/extension/src/__tests__/service_worker.test.ts @@ -2,6 +2,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest" import { enhancedSettings } from "@/services/settings/enhancedSettings" import { Settings } from "@/services/settings/settings" import { ServiceWorkerCommand } from "@/services/ipc" +import { openOptionPage } from "@/services/chrome" import { NEW_HUB_URL, VERSION, SHORTCUT_NO_SELECTION_BEHAVIOR } from "@/const" // Imported before any vi.doMock, so this is the real event name table. import { ANALYTICS_EVENTS } from "@/services/analytics" @@ -138,12 +139,8 @@ describe("Service Worker Migration", () => { expect(mockEnhancedSettings.get).toHaveBeenCalled() expect(mockSettings.set).not.toHaveBeenCalled() - expect(chrome.tabs.create).toHaveBeenCalledWith( - expect.objectContaining({ - url: expect.stringContaining( - `addPageRule=${encodeURIComponent(newUrl)}`, - ), - }), + expect(openOptionPage).toHaveBeenCalledWith( + expect.stringContaining(`addPageRule=${encodeURIComponent(newUrl)}`), ) }) diff --git a/packages/extension/src/service_worker.ts b/packages/extension/src/service_worker.ts index 34eca47b..10543ec4 100644 --- a/packages/extension/src/service_worker.ts +++ b/packages/extension/src/service_worker.ts @@ -21,7 +21,12 @@ import { CACHE_SECTIONS } from "@/services/settings/settingsCache" import * as PageActionServiceWorker from "@/services/pageAction/serviceWorker" import { ServiceWorkerData } from "@/services/serviceWorkerData" import { ContextMenu } from "@/services/contextMenus" -import { closeWindow, windowExists, getCurrentTab } from "@/services/chrome" +import { + closeWindow, + windowExists, + getCurrentTab, + openOptionPage, +} from "@/services/chrome" import { WindowStackManager } from "@/services/windowStackManager" import { PopupAutoClose } from "@/services/popupAutoClose" import { findMatchingPageRule, isEmpty } from "@/lib/utils" @@ -155,7 +160,7 @@ const commandFuncs = { [ServiceWorkerCommand.execApi]: ActionHelper.execApi, [ServiceWorkerCommand.openOption]: (): boolean => { - chrome.runtime.openOptionsPage() + openOptionPage() return false }, @@ -172,13 +177,13 @@ const commandFuncs = { const pageRules = settings.pageRules ?? [] const matchingRule = findMatchingPageRule(pageRules, param.url) if (matchingRule == null) { - chrome.tabs.create({ - url: `${OPTION_PAGE_PATH}?addPageRule=${encodeURIComponent(param.url)}#pageRules`, - }) + await openOptionPage( + `${OPTION_PAGE_PATH}?addPageRule=${encodeURIComponent(param.url)}#pageRules`, + ) } else { - chrome.tabs.create({ - url: `${OPTION_PAGE_PATH}?editPageRule=${encodeURIComponent(matchingRule.urlPattern)}#pageRules`, - }) + await openOptionPage( + `${OPTION_PAGE_PATH}?editPageRule=${encodeURIComponent(matchingRule.urlPattern)}#pageRules`, + ) } } add() @@ -389,8 +394,7 @@ const updateActiveTabId = async (activeTabId?: number) => { } chrome.action.onClicked.addListener(() => { - // Reuses the existing options page tab in the current window if any. - chrome.runtime.openOptionsPage() + openOptionPage() }) chrome.windows.onFocusChanged.addListener(async (windowId: number) => { @@ -763,7 +767,9 @@ chrome.commands.onCommand.addListener(handleShortcutCommand) // Expose only in the E2E worker for Playwright evaluation, without a message endpoint. if (import.meta.env.MODE === "e2e") { - Object.assign(globalThis, { __e2eHandleShortcutCommand: handleShortcutCommand }) + Object.assign(globalThis, { + __e2eHandleShortcutCommand: handleShortcutCommand, + }) } // Side panel is an optional capability (e.g. not available on Opera), so diff --git a/packages/extension/src/services/__tests__/chrome.openOptionPage.test.ts b/packages/extension/src/services/__tests__/chrome.openOptionPage.test.ts new file mode 100644 index 00000000..be08f5ea --- /dev/null +++ b/packages/extension/src/services/__tests__/chrome.openOptionPage.test.ts @@ -0,0 +1,71 @@ +import { beforeEach, describe, expect, it, vi } from "vitest" +import { openOptionPage } from "../chrome" + +vi.mock("../offscreenClipboard", () => ({ readOffscreenClipboard: vi.fn() })) + +const BASE_URL = "chrome-extension://test-extension-id/" +const OPTION_URL = `${BASE_URL}src/options_page.html` + +describe("openOptionPage", () => { + beforeEach(() => { + vi.clearAllMocks() + vi.mocked(chrome.runtime.getURL).mockImplementation( + (path: string) => `${BASE_URL}${path}`, + ) + vi.mocked(chrome.tabs.create).mockImplementation(() => + Promise.resolve({ id: 20 } as chrome.tabs.Tab), + ) + vi.mocked(chrome.tabs.update).mockImplementation( + async (tabId) => ({ id: tabId }) as chrome.tabs.Tab, + ) + ;(chrome.windows as any).update = vi.fn().mockResolvedValue({}) + }) + + it("COP-01: creates a new tab when no options page is open", async () => { + vi.mocked(chrome.tabs.query).mockImplementation(() => + Promise.resolve([ + { id: 1, windowId: 1, url: "https://example.com/" }, + ] as chrome.tabs.Tab[]), + ) + + const result = await openOptionPage() + + expect(chrome.tabs.create).toHaveBeenCalledWith({ + url: "src/options_page.html", + }) + expect(chrome.tabs.update).not.toHaveBeenCalled() + expect(result).toEqual({ tab: { id: 20 }, created: true }) + }) + + it("COP-02: activates the existing tab without navigating", async () => { + vi.mocked(chrome.tabs.query).mockImplementation(() => + Promise.resolve([ + { id: 5, windowId: 2, url: `${OPTION_URL}#commands` }, + ] as chrome.tabs.Tab[]), + ) + + const result = await openOptionPage() + + expect(chrome.tabs.create).not.toHaveBeenCalled() + expect(chrome.tabs.update).toHaveBeenCalledWith(5, { active: true }) + expect(chrome.windows.update).toHaveBeenCalledWith(2, { focused: true }) + expect(result).toEqual({ tab: { id: 5 }, created: false }) + }) + + it("COP-03: navigates the existing tab when a query is given", async () => { + vi.mocked(chrome.tabs.query).mockImplementation(() => + Promise.resolve([ + { id: 5, windowId: 2, url: OPTION_URL }, + ] as chrome.tabs.Tab[]), + ) + + await openOptionPage("src/options_page.html?addPageRule=x#pageRules") + + expect(chrome.tabs.create).not.toHaveBeenCalled() + expect(chrome.tabs.update).toHaveBeenCalledWith(5, { + active: true, + url: `${OPTION_URL}?addPageRule=x#pageRules`, + }) + expect(chrome.windows.update).toHaveBeenCalledWith(2, { focused: true }) + }) +}) diff --git a/packages/extension/src/services/chrome.ts b/packages/extension/src/services/chrome.ts index 5f764efa..7869c865 100644 --- a/packages/extension/src/services/chrome.ts +++ b/packages/extension/src/services/chrome.ts @@ -1,7 +1,12 @@ import { sleep, toUrl, isOverBytes, needsClipboard } from "@/lib/utils" import type { ScreenSize } from "@/services/dom" import type { ShowToastParam, UrlParam, WindowLayer } from "@/types" -import { POPUP_OFFSET, POPUP_TYPE, WINDOW_STATE } from "@/const" +import { + OPTION_PAGE_PATH, + POPUP_OFFSET, + POPUP_TYPE, + WINDOW_STATE, +} from "@/const" import { PopupOption } from "@/services/option/defaultSettings" import { ServiceWorkerData } from "@/services/serviceWorkerData" import { WindowStackManager } from "@/services/windowStackManager" @@ -507,6 +512,29 @@ export async function getCurrentTab(): Promise { return tab } +/** + * Open the options page, reusing an existing options page tab if any. + * @param path - The options page path. It may include a query and a hash. + * @returns The tab showing the options page, and whether it was newly created. + */ +export async function openOptionPage( + path: string = OPTION_PAGE_PATH, +): Promise<{ tab: chrome.tabs.Tab | undefined; created: boolean }> { + const optionPageUrl = chrome.runtime.getURL(OPTION_PAGE_PATH) + const tabs = await chrome.tabs.query({}) + const existing = tabs.find((t) => t.url?.startsWith(optionPageUrl)) + if (existing?.id == null) { + const tab = await chrome.tabs.create({ url: path }) + return { tab, created: true } + } + // Navigate only when parameters are given, to keep the current page state. + const props: chrome.tabs.UpdateProperties = { active: true } + if (path !== OPTION_PAGE_PATH) props.url = chrome.runtime.getURL(path) + const tab = await chrome.tabs.update(existing.id, props) + await chrome.windows.update(existing.windowId, { focused: true }) + return { tab: tab ?? existing, created: false } +} + /** * Close a window. * @param windowId - The ID of the window to close. diff --git a/packages/extension/src/services/hub/__tests__/serviceWorker.test.ts b/packages/extension/src/services/hub/__tests__/serviceWorker.test.ts index ee31d06b..8793c850 100644 --- a/packages/extension/src/services/hub/__tests__/serviceWorker.test.ts +++ b/packages/extension/src/services/hub/__tests__/serviceWorker.test.ts @@ -18,6 +18,7 @@ import { Settings } from "@/services/settings/settings" import { sendEvent, getOrCreateClientId } from "@/services/analytics" import { Ipc, TabCommand } from "@/services/ipc" import { calcContentHash } from "@/services/hub/commandContent" +import { openOptionPage } from "@/services/chrome" vi.mock("@/services/storage", () => ({ Storage: { @@ -35,6 +36,8 @@ vi.mock("@/services/storage", () => ({ }, })) +vi.mock("@/services/chrome", () => ({ openOptionPage: vi.fn() })) + vi.mock("@/services/settings/settings", () => ({ Settings: { addCommands: vi.fn(), updateCommandId: vi.fn() }, })) @@ -84,6 +87,13 @@ type MessageListener = ( sendResponse: (response?: unknown) => void, ) => boolean +function mockOpenOptionPage(tab: Partial, created = true) { + vi.mocked(openOptionPage).mockResolvedValue({ + tab: tab as chrome.tabs.Tab, + created, + }) +} + function getRegisteredListener(): MessageListener { initHubExternalListener() const calls = vi.mocked(chrome.runtime.onMessageExternal.addListener).mock @@ -719,10 +729,7 @@ describe("handleEditCommand", () => { }) it("EC-02: creates options tab with correct URL and responds with result:true", async () => { - vi.mocked(chrome.tabs.create).mockImplementation((_opts, cb) => { - cb?.({ id: 42 } as chrome.tabs.Tab) - return Promise.resolve({ id: 42 } as chrome.tabs.Tab) - }) + mockOpenOptionPage({ id: 42 }) const sendResponse = vi.fn() const result = handleEditCommand( "cmd-123", @@ -730,13 +737,8 @@ describe("handleEditCommand", () => { sendResponse, ) expect(result).toBe(true) - expect(chrome.tabs.create).toHaveBeenCalledWith( - { - url: expect.stringContaining( - "src/options_page.html?editCommand=cmd-123&syncHub=1#commands", - ), - }, - expect.any(Function), + expect(openOptionPage).toHaveBeenCalledWith( + "src/options_page.html?editCommand=cmd-123&syncHub=1#commands", ) await vi.waitFor(() => expect(sendResponse).toHaveBeenCalledWith({ result: true }), @@ -744,10 +746,7 @@ describe("handleEditCommand", () => { }) it("EC-03: responds with error when tab creation returns no id", async () => { - vi.mocked(chrome.tabs.create).mockImplementation((_opts, cb) => { - cb?.({} as chrome.tabs.Tab) - return Promise.resolve({} as chrome.tabs.Tab) - }) + mockOpenOptionPage({}) const sendResponse = vi.fn() handleEditCommand( "cmd-1", @@ -761,6 +760,23 @@ describe("handleEditCommand", () => { }), ) }) + + it("EC-04: responds with error when opening the options page fails", async () => { + vi.mocked(openOptionPage).mockRejectedValue(new Error("No window")) + const sendResponse = vi.fn() + handleEditCommand( + "cmd-1", + { tab: { id: 10 } as chrome.tabs.Tab }, + sendResponse, + ) + await vi.waitFor(() => + expect(sendResponse).toHaveBeenCalledWith({ + result: false, + error: "Failed to create edit tab", + }), + ) + expect(chrome.runtime.onConnectExternal.removeListener).toHaveBeenCalled() + }) }) // --------------------------------------------------------------------------- @@ -769,10 +785,7 @@ describe("handleEditCommand", () => { describe("onMessageExternal - EditCommand routing", () => { it("EC-R-01: opens options page with editCommand and syncHub params", async () => { - vi.mocked(chrome.tabs.create).mockImplementation((_opts, cb) => { - cb?.({ id: 42 } as chrome.tabs.Tab) - return Promise.resolve({ id: 42 } as chrome.tabs.Tab) - }) + mockOpenOptionPage({ id: 42 }) const listener = getRegisteredListener() const sendResponse = vi.fn() const result = listener( @@ -795,14 +808,11 @@ describe("onMessageExternal - EditCommand routing", () => { sendResponse, ) expect(result).toBe(false) - expect(chrome.tabs.create).not.toHaveBeenCalled() + expect(openOptionPage).not.toHaveBeenCalled() }) it("EC-R-03: ignores hub-edit port when sender tab id does not match", () => { - vi.mocked(chrome.tabs.create).mockImplementation((_opts, cb) => { - cb?.({ id: 42 } as chrome.tabs.Tab) - return Promise.resolve({ id: 42 } as chrome.tabs.Tab) - }) + mockOpenOptionPage({ id: 42 }) const listener = getRegisteredListener() listener( { action: "EditCommand", id: "cmd-123" }, @@ -825,10 +835,7 @@ describe("onMessageExternal - EditCommand routing", () => { }) it("EC-R-04: ignores hub-edit port from wrong origin", () => { - vi.mocked(chrome.tabs.create).mockImplementation((_opts, cb) => { - cb?.({ id: 42 } as chrome.tabs.Tab) - return Promise.resolve({ id: 42 } as chrome.tabs.Tab) - }) + mockOpenOptionPage({ id: 42 }) const listener = getRegisteredListener() listener( { action: "EditCommand", id: "cmd-123" }, @@ -851,10 +858,7 @@ describe("onMessageExternal - EditCommand routing", () => { it("EC-R-05: hub-edit connect listener times out after EDIT_CONNECT_TIMEOUT_MS", () => { vi.useFakeTimers() - vi.mocked(chrome.tabs.create).mockImplementation((_opts, cb) => { - cb?.({ id: 42 } as chrome.tabs.Tab) - return Promise.resolve({ id: 42 } as chrome.tabs.Tab) - }) + mockOpenOptionPage({ id: 42 }) const listener = getRegisteredListener() listener( { action: "EditCommand", id: "cmd-123" }, @@ -881,11 +885,8 @@ describe("editCommandToHub", () => { targetUrl: "https://example.com", } as any - function setupConnectedHubEditPort() { - vi.mocked(chrome.tabs.create).mockImplementation((_opts, cb) => { - cb?.({ id: 77 } as chrome.tabs.Tab) - return Promise.resolve({ id: 77 } as chrome.tabs.Tab) - }) + async function setupConnectedHubEditPort(created = true) { + mockOpenOptionPage({ id: 77 }, created) const listener = getRegisteredListener() listener( { action: "EditCommand", id: "cmd-1" }, @@ -903,6 +904,9 @@ describe("editCommandToHub", () => { onDisconnect: { addListener: vi.fn() }, } hubEditConnectListener(port as any) + // Let the opened options page tab be registered to the edit session. + await Promise.resolve() + await Promise.resolve() return port } @@ -914,7 +918,7 @@ describe("editCommandToHub", () => { }) it("EH-02: on ack, updates storage excluding locale/targetUrl", async () => { - const port = setupConnectedHubEditPort() + const port = await setupConnectedHubEditPort() let onAckMessage: ((msg: unknown) => void) | undefined vi.mocked(port.onMessage.addListener).mockImplementation((fn) => { onAckMessage = fn @@ -949,9 +953,27 @@ describe("editCommandToHub", () => { }) }) - it("EH-03: responds with false when ack timeout passes", () => { + it("EH-02-a: keeps the reused options page tab open after ack", async () => { + const port = await setupConnectedHubEditPort(false) + let onAckMessage: ((msg: unknown) => void) | undefined + vi.mocked(port.onMessage.addListener).mockImplementation((fn) => { + onAckMessage = fn + }) + const response = vi.fn() + + editCommandToHub(editParam, sender, response) + onAckMessage?.({ type: "edit-command-ack" }) + + await vi.waitFor(() => { + expect(response).toHaveBeenCalledWith(true) + expect(chrome.tabs.update).toHaveBeenCalledWith(10, { active: true }) + }) + expect(chrome.tabs.remove).not.toHaveBeenCalled() + }) + + it("EH-03: responds with false when ack timeout passes", async () => { vi.useFakeTimers() - const port = setupConnectedHubEditPort() + const port = await setupConnectedHubEditPort() const response = vi.fn() editCommandToHub(editParam, sender, response) @@ -962,9 +984,9 @@ describe("editCommandToHub", () => { vi.useRealTimers() }) - it("EH-04: rejects duplicate request while previous ack is pending", () => { + it("EH-04: rejects duplicate request while previous ack is pending", async () => { vi.useFakeTimers() - setupConnectedHubEditPort() + await setupConnectedHubEditPort() const firstResponse = vi.fn() const secondResponse = vi.fn() diff --git a/packages/extension/src/services/hub/serviceWorker.ts b/packages/extension/src/services/hub/serviceWorker.ts index 2f987c68..39c10f03 100644 --- a/packages/extension/src/services/hub/serviceWorker.ts +++ b/packages/extension/src/services/hub/serviceWorker.ts @@ -7,6 +7,7 @@ import { SCREEN, } from "@/const" import { Ipc, TabCommand } from "@/services/ipc" +import { openOptionPage } from "@/services/chrome" import type { Sender, ConfirmCommandUpdateProps } from "@/services/ipc" import { Storage, LOCAL_STORAGE_KEY } from "@/services/storage" import type { SubmitCommandInput } from "@/services/hubShare" @@ -602,26 +603,34 @@ export function handleEditCommand( if (_editSession === newSession) _editSession = undefined }, EDIT_CONNECT_TIMEOUT_MS) - chrome.tabs.create( - { - url: `${OPTION_PAGE_PATH}?editCommand=${encodeURIComponent(id)}&syncHub=1#commands`, - }, - (tab) => { + const failToOpen = () => { + cleanupHubEditConnectListener() + if (_editSession === newSession) _editSession = undefined + sendResponse({ result: false, error: "Failed to create edit tab" }) + } + + openOptionPage( + `${OPTION_PAGE_PATH}?editCommand=${encodeURIComponent(id)}&syncHub=1#commands`, + ) + .then(({ tab, created }) => { if (_editSession !== newSession) { - if (tab?.id != null) chrome.tabs.remove(tab.id) + if (created && tab?.id != null) chrome.tabs.remove(tab.id) sendResponse({ result: false, error: "Edit session timed out" }) return } if (tab?.id == null) { - cleanupHubEditConnectListener() - _editSession = undefined - sendResponse({ result: false, error: "Failed to create edit tab" }) + failToOpen() return } - newSession.editTabId = tab.id + // Close the tab after editing only if it was opened for this edit, so + // that an options page the user already had open is kept. + if (created) newSession.editTabId = tab.id sendResponse({ result: true }) - }, - ) + }) + .catch((err) => { + console.error("[handleEditCommand] Failed to open options page:", err) + failToOpen() + }) return true } From 8974ef5de279d47d4702f43aa0a106a987a302f7 Mon Sep 17 00:00:00 2001 From: ujiro99 Date: Fri, 23 Oct 2026 11:14:04 +0900 Subject: [PATCH 3/3] fix(extension): address review comments on options page tab reuse - Match options page tabs that are still loading via pendingUrl - Prefer the options page tab in the last focused window - Do not reuse the edit tab of a cancelled Hub edit session - Catch errors from openOptionPage() in the service worker - Update stale comments on reading location.search - Await the opened tab in tests instead of a fixed number of ticks Co-Authored-By: Claude Opus 5.5 --- .../components/option/editor/CommandList.tsx | 5 +- .../components/option/editor/PageRuleList.tsx | 5 +- packages/extension/src/service_worker.ts | 8 ++- .../__tests__/chrome.openOptionPage.test.ts | 66 ++++++++++++++----- packages/extension/src/services/chrome.ts | 17 ++++- .../hub/__tests__/serviceWorker.test.ts | 21 +++++- .../src/services/hub/serviceWorker.ts | 3 + 7 files changed, 98 insertions(+), 27 deletions(-) diff --git a/packages/extension/src/components/option/editor/CommandList.tsx b/packages/extension/src/components/option/editor/CommandList.tsx index c4356f20..cdea1bcb 100644 --- a/packages/extension/src/components/option/editor/CommandList.tsx +++ b/packages/extension/src/components/option/editor/CommandList.tsx @@ -277,8 +277,9 @@ export const CommandList = ({ control }: CommandListProps) => { } } - // location.search is fixed at page load (set via chrome.tabs.create), so it is - // safe to read it without adding it to the dependency array. + // location.search is fixed at page load (the service worker opens or navigates + // the tab with these params via openOptionPage), so it is safe to read it + // without adding it to the dependency array. useEffect(() => { if (editParamHandledRef.current) return const params = new URLSearchParams(location.search) diff --git a/packages/extension/src/components/option/editor/PageRuleList.tsx b/packages/extension/src/components/option/editor/PageRuleList.tsx index a63834c8..9c0dfb8e 100644 --- a/packages/extension/src/components/option/editor/PageRuleList.tsx +++ b/packages/extension/src/components/option/editor/PageRuleList.tsx @@ -143,8 +143,9 @@ export const PageRuleList = ({ }) // Auto-open the dialog when addPageRule or editPageRule URL param is set. - // location.search is fixed at page load (set via chrome.tabs.create), so it is - // safe to read it without adding it to the dependency array. + // location.search is fixed at page load (the service worker opens or navigates + // the tab with these params via openOptionPage), so it is safe to read it + // without adding it to the dependency array. useEffect(() => { if (editParamHandledRef.current) return const params = new URLSearchParams(location.search) diff --git a/packages/extension/src/service_worker.ts b/packages/extension/src/service_worker.ts index 10543ec4..10f29071 100644 --- a/packages/extension/src/service_worker.ts +++ b/packages/extension/src/service_worker.ts @@ -160,7 +160,9 @@ const commandFuncs = { [ServiceWorkerCommand.execApi]: ActionHelper.execApi, [ServiceWorkerCommand.openOption]: (): boolean => { - openOptionPage() + openOptionPage().catch((e) => + console.error("Failed to open option page", e), + ) return false }, @@ -186,7 +188,7 @@ const commandFuncs = { ) } } - add() + add().catch((e) => console.error("Failed to open option page", e)) return false }, @@ -394,7 +396,7 @@ const updateActiveTabId = async (activeTabId?: number) => { } chrome.action.onClicked.addListener(() => { - openOptionPage() + openOptionPage().catch((e) => console.error("Failed to open option page", e)) }) chrome.windows.onFocusChanged.addListener(async (windowId: number) => { diff --git a/packages/extension/src/services/__tests__/chrome.openOptionPage.test.ts b/packages/extension/src/services/__tests__/chrome.openOptionPage.test.ts index be08f5ea..d386f02f 100644 --- a/packages/extension/src/services/__tests__/chrome.openOptionPage.test.ts +++ b/packages/extension/src/services/__tests__/chrome.openOptionPage.test.ts @@ -6,6 +6,21 @@ vi.mock("../offscreenClipboard", () => ({ readOffscreenClipboard: vi.fn() })) const BASE_URL = "chrome-extension://test-extension-id/" const OPTION_URL = `${BASE_URL}src/options_page.html` +/** + * Mock chrome.tabs.query to return all tabs, or the active tab of the last + * focused window when queried with `active: true`. + */ +const mockTabs = (tabs: Partial[], currentWindowId = 1) => { + vi.mocked(chrome.tabs.query).mockImplementation((( + info: chrome.tabs.QueryInfo, + ) => + Promise.resolve( + info.active + ? [{ id: 100, windowId: currentWindowId, url: "https://example.com/" }] + : tabs, + )) as typeof chrome.tabs.query) +} + describe("openOptionPage", () => { beforeEach(() => { vi.clearAllMocks() @@ -22,11 +37,7 @@ describe("openOptionPage", () => { }) it("COP-01: creates a new tab when no options page is open", async () => { - vi.mocked(chrome.tabs.query).mockImplementation(() => - Promise.resolve([ - { id: 1, windowId: 1, url: "https://example.com/" }, - ] as chrome.tabs.Tab[]), - ) + mockTabs([{ id: 1, windowId: 1, url: "https://example.com/" }]) const result = await openOptionPage() @@ -38,11 +49,7 @@ describe("openOptionPage", () => { }) it("COP-02: activates the existing tab without navigating", async () => { - vi.mocked(chrome.tabs.query).mockImplementation(() => - Promise.resolve([ - { id: 5, windowId: 2, url: `${OPTION_URL}#commands` }, - ] as chrome.tabs.Tab[]), - ) + mockTabs([{ id: 5, windowId: 2, url: `${OPTION_URL}#commands` }]) const result = await openOptionPage() @@ -53,11 +60,7 @@ describe("openOptionPage", () => { }) it("COP-03: navigates the existing tab when a query is given", async () => { - vi.mocked(chrome.tabs.query).mockImplementation(() => - Promise.resolve([ - { id: 5, windowId: 2, url: OPTION_URL }, - ] as chrome.tabs.Tab[]), - ) + mockTabs([{ id: 5, windowId: 2, url: OPTION_URL }]) await openOptionPage("src/options_page.html?addPageRule=x#pageRules") @@ -68,4 +71,37 @@ describe("openOptionPage", () => { }) expect(chrome.windows.update).toHaveBeenCalledWith(2, { focused: true }) }) + + it("COP-04: prefers the tab in the last focused window", async () => { + mockTabs( + [ + { id: 5, windowId: 2, url: OPTION_URL }, + { id: 6, windowId: 3, url: OPTION_URL }, + ], + 3, + ) + + await openOptionPage() + + expect(chrome.tabs.update).toHaveBeenCalledWith(6, { active: true }) + expect(chrome.windows.update).toHaveBeenCalledWith(3, { focused: true }) + }) + + it("COP-05: reuses a tab that is still loading the options page", async () => { + mockTabs([{ id: 5, windowId: 1, url: "", pendingUrl: OPTION_URL }]) + + await openOptionPage() + + expect(chrome.tabs.create).not.toHaveBeenCalled() + expect(chrome.tabs.update).toHaveBeenCalledWith(5, { active: true }) + }) + + it("COP-06: does not reuse the excluded tab", async () => { + mockTabs([{ id: 5, windowId: 1, url: OPTION_URL }]) + + const result = await openOptionPage(undefined, 5) + + expect(chrome.tabs.update).not.toHaveBeenCalled() + expect(result).toEqual({ tab: { id: 20 }, created: true }) + }) }) diff --git a/packages/extension/src/services/chrome.ts b/packages/extension/src/services/chrome.ts index 7869c865..5cc35769 100644 --- a/packages/extension/src/services/chrome.ts +++ b/packages/extension/src/services/chrome.ts @@ -514,15 +514,28 @@ export async function getCurrentTab(): Promise { /** * Open the options page, reusing an existing options page tab if any. + * A tab in the last focused window is preferred over tabs in other windows. * @param path - The options page path. It may include a query and a hash. + * @param excludeTabId - A tab not to reuse, e.g. one that is being closed. * @returns The tab showing the options page, and whether it was newly created. */ export async function openOptionPage( path: string = OPTION_PAGE_PATH, + excludeTabId?: number, ): Promise<{ tab: chrome.tabs.Tab | undefined; created: boolean }> { const optionPageUrl = chrome.runtime.getURL(OPTION_PAGE_PATH) - const tabs = await chrome.tabs.query({}) - const existing = tabs.find((t) => t.url?.startsWith(optionPageUrl)) + const [tabs, currentTab] = await Promise.all([ + chrome.tabs.query({}), + getCurrentTab(), + ]) + // A tab that is still loading has its destination only in pendingUrl. + const candidates = tabs.filter( + (t) => + t.id !== excludeTabId && + (t.pendingUrl || t.url)?.startsWith(optionPageUrl), + ) + const existing = + candidates.find((t) => t.windowId === currentTab?.windowId) ?? candidates[0] if (existing?.id == null) { const tab = await chrome.tabs.create({ url: path }) return { tab, created: true } diff --git a/packages/extension/src/services/hub/__tests__/serviceWorker.test.ts b/packages/extension/src/services/hub/__tests__/serviceWorker.test.ts index 8793c850..9b7b31cc 100644 --- a/packages/extension/src/services/hub/__tests__/serviceWorker.test.ts +++ b/packages/extension/src/services/hub/__tests__/serviceWorker.test.ts @@ -739,6 +739,7 @@ describe("handleEditCommand", () => { expect(result).toBe(true) expect(openOptionPage).toHaveBeenCalledWith( "src/options_page.html?editCommand=cmd-123&syncHub=1#commands", + undefined, ) await vi.waitFor(() => expect(sendResponse).toHaveBeenCalledWith({ result: true }), @@ -761,6 +762,21 @@ describe("handleEditCommand", () => { ) }) + it("EC-05: does not reuse the edit tab of the cancelled session", async () => { + mockOpenOptionPage({ id: 77 }) + const sender = { tab: { id: 10 } as chrome.tabs.Tab } + handleEditCommand("cmd-1", sender, vi.fn()) + await vi.mocked(openOptionPage).mock.results.at(-1)?.value + + handleEditCommand("cmd-2", sender, vi.fn()) + + expect(chrome.tabs.remove).toHaveBeenCalledWith(77) + expect(openOptionPage).toHaveBeenLastCalledWith( + "src/options_page.html?editCommand=cmd-2&syncHub=1#commands", + 77, + ) + }) + it("EC-04: responds with error when opening the options page fails", async () => { vi.mocked(openOptionPage).mockRejectedValue(new Error("No window")) const sendResponse = vi.fn() @@ -904,9 +920,8 @@ describe("editCommandToHub", () => { onDisconnect: { addListener: vi.fn() }, } hubEditConnectListener(port as any) - // Let the opened options page tab be registered to the edit session. - await Promise.resolve() - await Promise.resolve() + // Wait until the opened options page tab is registered to the session. + await vi.mocked(openOptionPage).mock.results.at(-1)?.value return port } diff --git a/packages/extension/src/services/hub/serviceWorker.ts b/packages/extension/src/services/hub/serviceWorker.ts index 39c10f03..687f4cc9 100644 --- a/packages/extension/src/services/hub/serviceWorker.ts +++ b/packages/extension/src/services/hub/serviceWorker.ts @@ -555,6 +555,8 @@ export function handleEditCommand( } // Cancel any in-progress session; the new request takes over (last-write-wins). + // Its edit tab is being closed, so it must not be reused for this request. + const closingTabId = _editSession?.editTabId cancelEditSession() const newSession: EditSession = { @@ -611,6 +613,7 @@ export function handleEditCommand( openOptionPage( `${OPTION_PAGE_PATH}?editCommand=${encodeURIComponent(id)}&syncHub=1#commands`, + closingTabId, ) .then(({ tab, created }) => { if (_editSession !== newSession) {