From 913afbb32fe8c760fbef98cdeb3f37246de29daf Mon Sep 17 00:00:00 2001 From: ujiro99 Date: Mon, 19 Oct 2026 08:19:37 +0900 Subject: [PATCH 01/14] fix: ignore a selection recorded in another tab on shortcut execution The selection text shared via session storage was not cleared on tab switching, and tabs without the content script (e.g. chrome://) can't record their selection. So a shortcut could run with a stale selection from another tab instead of falling back to the clipboard. Record the tab ID with the selection and use it only when it matches the active tab. Also re-record the selection when a tab becomes visible. Closes #498 Co-Authored-By: Claude Opus 5.5 --- .../src/__tests__/service_worker.test.ts | 98 ++++++++++++-- .../src/providers/SelectContextProvider.tsx | 43 +++++- .../__tests__/SelectContextProvider.test.tsx | 124 ++++++++++++++++++ packages/extension/src/service_worker.ts | 26 +++- packages/extension/src/services/storage.ts | 3 +- .../extension/src/services/storage/const.ts | 7 + .../extension/src/services/storage/index.ts | 2 +- 7 files changed, 279 insertions(+), 24 deletions(-) create mode 100644 packages/extension/src/providers/__tests__/SelectContextProvider.test.tsx diff --git a/packages/extension/src/__tests__/service_worker.test.ts b/packages/extension/src/__tests__/service_worker.test.ts index dce53350e..8963ea6f9 100644 --- a/packages/extension/src/__tests__/service_worker.test.ts +++ b/packages/extension/src/__tests__/service_worker.test.ts @@ -2,7 +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 { NEW_HUB_URL, VERSION } from "@/const" +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" @@ -276,18 +276,23 @@ describe("Service Worker Migration", () => { ) }) - it("MG-01-d: should call enhancedSettings.get() in onCommand function and pass correct command to action/serviceWorker.execute", async () => { - // Setup test data - const testShortcutId = "test-shortcut-cmd" - const testCommandId = "test-command-123" + // Sets up the mocks for the onCommand listener and returns it. + const setupOnCommand = async ({ + storedSelection, + tab, + noSelectionBehavior = SHORTCUT_NO_SELECTION_BEHAVIOR.DO_NOTHING, + }: { + storedSelection: { text: string; tabId: number | null } + tab: { id: number; url: string; windowId: number } + noSelectionBehavior?: SHORTCUT_NO_SELECTION_BEHAVIOR + }) => { const initialSettings = createTestSettings() - + initialSettings.shortcuts.shortcuts[0].noSelectionBehavior = + noSelectionBehavior as any mockEnhancedSettings.get.mockResolvedValue(initialSettings as any) - // Mock chrome.tabs.query to return a valid tab - ;(chrome.tabs.query as any).mockResolvedValue([ - { id: 1, url: "https://example.com", windowId: 1 }, - ]) + // Mock chrome.tabs.query to return the active tab + ;(chrome.tabs.query as any).mockResolvedValue([tab]) // Mock the current window used to center the popup ;(chrome.windows as any).getCurrent = vi @@ -295,7 +300,7 @@ describe("Service Worker Migration", () => { .mockResolvedValue({ left: 1920, top: 0, width: 1600, height: 1000 }) // Mock Storage.get for selection text - const mockStorageGet = vi.fn().mockResolvedValue("test selection text") + const mockStorageGet = vi.fn().mockResolvedValue(storedSelection) vi.doMock("@/services/storage", () => ({ Storage: { get: mockStorageGet, @@ -343,7 +348,19 @@ describe("Service Worker Migration", () => { .calls expect(listenerCalls.length).toBeGreaterThan(0) - const commandListener = listenerCalls[0][0] + const commandListener = listenerCalls[listenerCalls.length - 1][0] + return { commandListener, mockExecute, mockIpcSendTab } + } + + it("MG-01-d: should call enhancedSettings.get() in onCommand function and pass correct command to action/serviceWorker.execute", async () => { + // Setup test data + const testShortcutId = "test-shortcut-cmd" + const testCommandId = "test-command-123" + + const { commandListener, mockExecute } = await setupOnCommand({ + storedSelection: { text: "test selection text", tabId: 1 }, + tab: { id: 1, url: "https://example.com", windowId: 1 }, + }) // Call the listener with our test shortcut command await commandListener(testShortcutId) @@ -351,7 +368,6 @@ describe("Service Worker Migration", () => { // Wait for async operations await new Promise((resolve) => setTimeout(resolve, 10)) - // This should fail initially because the function still uses Settings.get() instead of enhancedSettings.get() expect(mockEnhancedSettings.get).toHaveBeenCalled() // Verify that the execute function was called with the correct command @@ -377,6 +393,62 @@ describe("Service Worker Migration", () => { }), ) }) + + it("MG-01-e: should ignore a selection recorded in another tab and fall back to the clipboard", async () => { + // The selection was recorded in tab 1, but the shortcut is pressed in a + // chrome:// tab, where the content script can't record its selection. + const { commandListener, mockExecute, mockIpcSendTab } = + await setupOnCommand({ + storedSelection: { text: "stale selection", tabId: 1 }, + tab: { id: 2, url: "chrome://settings/", windowId: 1 }, + noSelectionBehavior: SHORTCUT_NO_SELECTION_BEHAVIOR.USE_CLIPBOARD, + }) + + await commandListener("test-shortcut-cmd") + await new Promise((resolve) => setTimeout(resolve, 10)) + + expect(mockIpcSendTab).not.toHaveBeenCalled() + expect(mockExecute).toHaveBeenCalledWith( + expect.objectContaining({ + selectionText: "", + allowClipboardFallback: true, + pageUrl: "chrome://settings/", + }), + ) + }) + + it("MG-01-f: should do nothing when the selection was recorded in another tab and noSelectionBehavior is DO_NOTHING", async () => { + const { commandListener, mockExecute, mockIpcSendTab } = + await setupOnCommand({ + storedSelection: { text: "stale selection", tabId: 1 }, + tab: { id: 2, url: "https://example.com", windowId: 1 }, + noSelectionBehavior: SHORTCUT_NO_SELECTION_BEHAVIOR.DO_NOTHING, + }) + + await commandListener("test-shortcut-cmd") + await new Promise((resolve) => setTimeout(resolve, 10)) + + expect(mockIpcSendTab).not.toHaveBeenCalled() + expect(mockExecute).not.toHaveBeenCalled() + }) + + it("MG-01-g: should ignore a selection whose tab is unknown", async () => { + const { commandListener, mockExecute } = await setupOnCommand({ + storedSelection: { text: "unknown tab selection", tabId: null }, + tab: { id: 2, url: "chrome://settings/", windowId: 1 }, + noSelectionBehavior: SHORTCUT_NO_SELECTION_BEHAVIOR.USE_CLIPBOARD, + }) + + await commandListener("test-shortcut-cmd") + await new Promise((resolve) => setTimeout(resolve, 10)) + + expect(mockExecute).toHaveBeenCalledWith( + expect.objectContaining({ + selectionText: "", + allowClipboardFallback: true, + }), + ) + }) }) describe("Popup Auto-Close Delay", () => { diff --git a/packages/extension/src/providers/SelectContextProvider.tsx b/packages/extension/src/providers/SelectContextProvider.tsx index 4d32291ce..017a6a689 100644 --- a/packages/extension/src/providers/SelectContextProvider.tsx +++ b/packages/extension/src/providers/SelectContextProvider.tsx @@ -1,8 +1,22 @@ -import { useState, ReactNode, useEffect, useCallback } from "react" -import { Storage, SESSION_STORAGE_KEY } from "@/services/storage" +import { useState, ReactNode, useEffect, useCallback, useRef } from "react" +import { + Storage, + SESSION_STORAGE_KEY, + type SelectionTextData, +} from "@/services/storage" +import { Ipc } from "@/services/ipc" import { getSelectionText } from "@/services/dom" import { ContextType, selectContext } from "@/hooks/useSelectContext" +const fetchTabId = async (): Promise => { + try { + return (await Ipc.getTabId()) ?? null + } catch { + // e.g. side panel, which does not belong to a tab + return null + } +} + export const SelectContextProvider = ({ children, }: { @@ -11,10 +25,18 @@ export const SelectContextProvider = ({ const [selectionText, _setSelectionText] = useState("") const [target, setTarget] = useState(null) const [detectSelectionEnabled, setDetectSelectionEnabled] = useState(true) + const tabIdRef = useRef | null>(null) const setSelectionText = useCallback(async (text: string) => { _setSelectionText(text) - await Storage.set(SESSION_STORAGE_KEY.SELECTION_TEXT, text) + // Record the tab ID with the text, so that the shortcut handler in the + // service worker can ignore a selection recorded in another tab. + tabIdRef.current ??= fetchTabId() + const tabId = await tabIdRef.current + await Storage.set(SESSION_STORAGE_KEY.SELECTION_TEXT, { + text, + tabId, + }) }, []) useEffect(() => { @@ -24,9 +46,24 @@ export const SelectContextProvider = ({ await setSelectionText(text) } + // The stored selection is shared by all tabs, so re-record this tab's + // selection when the tab becomes active again. An empty selection is not + // recorded, so that a frame without a selection doesn't overwrite the one + // recorded by another frame in this tab. (A selection recorded in another + // tab is ignored by the shortcut handler anyway.) + const onVisibilitychange = async () => { + if (!detectSelectionEnabled) return + if (document.visibilityState !== "visible") return + const text = getSelectionText() + if (!text) return + await setSelectionText(text) + } + document.addEventListener("selectionchange", onSelectionchange) + document.addEventListener("visibilitychange", onVisibilitychange) return () => { document.removeEventListener("selectionchange", onSelectionchange) + document.removeEventListener("visibilitychange", onVisibilitychange) } }, [detectSelectionEnabled, setSelectionText]) diff --git a/packages/extension/src/providers/__tests__/SelectContextProvider.test.tsx b/packages/extension/src/providers/__tests__/SelectContextProvider.test.tsx new file mode 100644 index 000000000..bad0462b3 --- /dev/null +++ b/packages/extension/src/providers/__tests__/SelectContextProvider.test.tsx @@ -0,0 +1,124 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest" +import { render, waitFor } from "@testing-library/react" +import { SelectContextProvider } from "../SelectContextProvider" +import { Storage, SESSION_STORAGE_KEY } from "@/services/storage" +import { Ipc } from "@/services/ipc" +import { getSelectionText } from "@/services/dom" + +vi.mock("@/services/storage", () => ({ + Storage: { + set: vi.fn(), + }, + SESSION_STORAGE_KEY: { + SELECTION_TEXT: "selectionText", + }, +})) + +vi.mock("@/services/ipc", () => ({ + Ipc: { + getTabId: vi.fn(), + }, +})) + +vi.mock("@/services/dom", () => ({ + getSelectionText: vi.fn(), +})) + +const mockSet = vi.mocked(Storage.set) +const mockGetTabId = vi.mocked(Ipc.getTabId) +const mockGetSelectionText = vi.mocked(getSelectionText) + +const setVisibilityState = (state: DocumentVisibilityState) => { + Object.defineProperty(document, "visibilityState", { + configurable: true, + get: () => state, + }) +} + +describe("SelectContextProvider", () => { + beforeEach(() => { + vi.clearAllMocks() + mockSet.mockResolvedValue(true) + mockGetTabId.mockResolvedValue(10) + setVisibilityState("visible") + }) + + afterEach(() => { + setVisibilityState("visible") + }) + + it("SC-01: 正常系: 選択変更時に選択テキストとタブIDを保存する", async () => { + mockGetSelectionText.mockReturnValue("selected") + render({null}) + + document.dispatchEvent(new Event("selectionchange")) + + await waitFor(() => { + expect(mockSet).toHaveBeenCalledWith(SESSION_STORAGE_KEY.SELECTION_TEXT, { + text: "selected", + tabId: 10, + }) + }) + }) + + it("SC-02: 異常系: タブIDを取得できない場合はtabIdをnullとして保存する", async () => { + mockGetTabId.mockRejectedValue(new Error("no tab")) + mockGetSelectionText.mockReturnValue("selected") + render({null}) + + document.dispatchEvent(new Event("selectionchange")) + + await waitFor(() => { + expect(mockSet).toHaveBeenCalledWith(SESSION_STORAGE_KEY.SELECTION_TEXT, { + text: "selected", + tabId: null, + }) + }) + }) + + it("SC-03: 正常系: タブIDの取得は一度だけ行う", async () => { + mockGetSelectionText.mockReturnValue("selected") + render({null}) + + document.dispatchEvent(new Event("selectionchange")) + document.dispatchEvent(new Event("selectionchange")) + + await waitFor(() => expect(mockSet).toHaveBeenCalledTimes(2)) + expect(mockGetTabId).toHaveBeenCalledTimes(1) + }) + + it("SC-04: 正常系: タブが表示されたときに選択中のテキストを再保存する", async () => { + mockGetSelectionText.mockReturnValue("kept selection") + render({null}) + + document.dispatchEvent(new Event("visibilitychange")) + + await waitFor(() => { + expect(mockSet).toHaveBeenCalledWith(SESSION_STORAGE_KEY.SELECTION_TEXT, { + text: "kept selection", + tabId: 10, + }) + }) + }) + + it("SC-05: 境界値: タブが表示されたときに選択が空なら保存しない", async () => { + mockGetSelectionText.mockReturnValue("") + render({null}) + + document.dispatchEvent(new Event("visibilitychange")) + await new Promise((resolve) => setTimeout(resolve, 10)) + + expect(mockSet).not.toHaveBeenCalled() + }) + + it("SC-06: 境界値: タブが非表示になったときは保存しない", async () => { + mockGetSelectionText.mockReturnValue("selected") + setVisibilityState("hidden") + render({null}) + + document.dispatchEvent(new Event("visibilitychange")) + await new Promise((resolve) => setTimeout(resolve, 10)) + + expect(mockSet).not.toHaveBeenCalled() + }) +}) diff --git a/packages/extension/src/service_worker.ts b/packages/extension/src/service_worker.ts index 3c7983038..a6ae86833 100644 --- a/packages/extension/src/service_worker.ts +++ b/packages/extension/src/service_worker.ts @@ -28,7 +28,11 @@ import { findMatchingPageRule, isEmpty } from "@/lib/utils" import { execute } from "@/action/serviceWorker" import * as ActionHelper from "@/action/helper" import type { WindowType } from "@/types" -import { Storage, SESSION_STORAGE_KEY } from "@/services/storage" +import { + Storage, + SESSION_STORAGE_KEY, + type SelectionTextData, +} from "@/services/storage" import { ANALYTICS_EVENTS, sendEvent, @@ -397,7 +401,10 @@ const clearSelectionTextUnlessKeepOpen = async () => { CACHE_SECTIONS.USER_SETTINGS, ) if (!settings.startupMethod?.keepMenuOpenOnFocusChange) { - await Storage.set(SESSION_STORAGE_KEY.SELECTION_TEXT, "") + await Storage.set(SESSION_STORAGE_KEY.SELECTION_TEXT, { + text: "", + tabId: null, + }) } } @@ -696,9 +703,19 @@ chrome.commands.onCommand.addListener(async (commandName) => { return } - const selectionText = await Storage.get( + // Get active tab + const [tab] = await chrome.tabs.query({ active: true, currentWindow: true }) + + // Use the selection only if it was recorded in the active tab. Tabs + // without the content script (e.g. chrome://) can't record their + // selection, so the stored one may belong to another tab. + const selection = await Storage.get( SESSION_STORAGE_KEY.SELECTION_TEXT, ) + const selectionText = + selection?.tabId != null && selection.tabId === tab?.id + ? selection.text + : "" // If no text is selected, handle according to noSelectionBehavior let allowClipboardFallback = false @@ -715,9 +732,6 @@ chrome.commands.onCommand.addListener(async (commandName) => { allowClipboardFallback = true } } - - // Get active tab - const [tab] = await chrome.tabs.query({ active: true, currentWindow: true }) const enableSendTab = tab?.id && !tab.url?.startsWith("chrome") && diff --git a/packages/extension/src/services/storage.ts b/packages/extension/src/services/storage.ts index 39a962e48..98ecefdd9 100644 --- a/packages/extension/src/services/storage.ts +++ b/packages/extension/src/services/storage.ts @@ -5,6 +5,7 @@ import { SESSION_STORAGE_KEY, CMD_PREFIX, KEY, + SelectionTextData, } from "./storage/const" import { BaseStorage, ChangedCallback } from "./storage/index" import { @@ -27,7 +28,7 @@ export { WeeklyBackupManager, } -export type { ChangedCallback, KEY } +export type { ChangedCallback, KEY, SelectionTextData } export const Storage = { // Base storage methods diff --git a/packages/extension/src/services/storage/const.ts b/packages/extension/src/services/storage/const.ts index c0463b3fa..bf0bb8f2d 100644 --- a/packages/extension/src/services/storage/const.ts +++ b/packages/extension/src/services/storage/const.ts @@ -36,6 +36,13 @@ export enum SESSION_STORAGE_KEY { DEV_TOOLS_ENABLED = "devToolsEnabled", } +// Selection text recorded by the tab where the text was selected. +export type SelectionTextData = { + text: string + // ID of the tab that recorded the selection. null if unknown. + tabId: number | null +} + export type KEY = | STORAGE_KEY | LOCAL_STORAGE_KEY diff --git a/packages/extension/src/services/storage/index.ts b/packages/extension/src/services/storage/index.ts index 9339ae985..9621c0281 100644 --- a/packages/extension/src/services/storage/index.ts +++ b/packages/extension/src/services/storage/index.ts @@ -105,7 +105,7 @@ const DEFAULTS = { [SESSION_STORAGE_KEY.PA_CONTEXT]: {}, [SESSION_STORAGE_KEY.PA_RECORDER_OPTION]: {}, [SESSION_STORAGE_KEY.TMP_CAPTURES]: {}, - [SESSION_STORAGE_KEY.SELECTION_TEXT]: "", + [SESSION_STORAGE_KEY.SELECTION_TEXT]: { text: "", tabId: null }, [SESSION_STORAGE_KEY.PA_SIDE_PANEL_PENDING]: null, [SESSION_STORAGE_KEY.DEV_TOOLS_ENABLED]: false, } as const From b8fda60d54ebc254f8a98a2d427850ce32ac9787 Mon Sep 17 00:00:00 2001 From: ujiro99 Date: Mon, 19 Oct 2026 12:30:05 +0900 Subject: [PATCH 02/14] refactor: decide clipboard use for the selection only once The shortcut handler judged the selection with the stored text, while the content script ran the command with the selection in its tab. So when the window lost focus with text selected, the stale selection was used instead of the clipboard. - The shortcut handler passes the selection it decided to the content script, and decides whether to use the clipboard only once. - Rename allowClipboardFallback to selectionFromClipboard, and split UrlParam.useClipboard into selectionFromClipboard and clipboardForSteps. Whether to read the clipboard is derived by needsClipboard(). - Remove the re-check of an empty selection in actions and toUrl. - Stop re-recording the selection on visibilitychange. Refs #498 Co-Authored-By: Claude Opus 5.5 --- docs/test/serviceWorker-test-design.md | 3 +- .../src/__tests__/service_worker.test.ts | 55 +++++++++++-- .../src/action/__tests__/aiPrompt.test.ts | 33 ++++---- .../src/action/__tests__/pageAction.test.ts | 12 +-- packages/extension/src/action/aiPrompt.ts | 62 +++++++------- .../extension/src/action/backgroundTab.ts | 4 +- packages/extension/src/action/executor.ts | 4 +- packages/extension/src/action/index.ts | 4 +- packages/extension/src/action/pageAction.ts | 10 ++- packages/extension/src/action/popup.ts | 4 +- .../extension/src/action/serviceWorker.ts | 4 +- packages/extension/src/action/sidePanel.ts | 8 +- packages/extension/src/action/tab.ts | 4 +- packages/extension/src/action/window.ts | 4 +- .../src/components/menu/InvisibleItem.tsx | 14 ++-- .../menu/__tests__/InvisibleItem.test.tsx | 82 +++++++++++++++++++ .../option/editor/SearchUrlAssistDialog.tsx | 2 +- .../extension/src/hooks/useCommandExecutor.ts | 6 +- .../extension/src/hooks/useContextMenu.ts | 12 ++- .../extension/src/lib/__tests__/utils.test.ts | 57 +++++++++---- packages/extension/src/lib/utils.ts | 22 ++++- .../src/providers/SelectContextProvider.tsx | 15 ---- .../__tests__/SelectContextProvider.test.tsx | 49 +---------- packages/extension/src/service_worker.ts | 15 ++-- packages/extension/src/services/chrome.ts | 12 +-- .../extension/src/services/contextMenus.ts | 9 +- packages/extension/src/services/ipc.ts | 5 +- .../__tests__/serviceWorker-execution.test.ts | 45 ++++++++-- .../__tests__/serviceWorker-setup.ts | 3 + .../__tests__/serviceWorker-sidePanel.test.ts | 41 +++++++++- .../pageAction/serviceWorker-sidePanel.ts | 7 +- .../src/services/pageAction/serviceWorker.ts | 14 ++-- packages/extension/src/types/index.ts | 20 +++-- 33 files changed, 416 insertions(+), 225 deletions(-) create mode 100644 packages/extension/src/components/menu/__tests__/InvisibleItem.test.tsx diff --git a/docs/test/serviceWorker-test-design.md b/docs/test/serviceWorker-test-design.md index 6ce1f928d..a1c488904 100644 --- a/docs/test/serviceWorker-test-design.md +++ b/docs/test/serviceWorker-test-design.md @@ -130,7 +130,8 @@ **テキスト処理:** -- ✅ SWD-44: 正常系: selectedTextが空でuseClipboardがtrueの場合、clipboardTextを使用 +- ✅ SWD-44: 正常系: selectionFromClipboardがtrueの場合、clipboardTextをselectedTextとして使用 +- ✅ SWD-44-b: 正常系: ステップのみがクリップボードを使う場合（clipboardForSteps）、selectedTextは空のまま - ✅ SWD-45: 正常系: selectedTextが存在する場合はそのまま使用 - ✅ SWD-46: 正常系: userVariablesが正しく渡される diff --git a/packages/extension/src/__tests__/service_worker.test.ts b/packages/extension/src/__tests__/service_worker.test.ts index 8963ea6f9..250c67071 100644 --- a/packages/extension/src/__tests__/service_worker.test.ts +++ b/packages/extension/src/__tests__/service_worker.test.ts @@ -281,10 +281,12 @@ describe("Service Worker Migration", () => { storedSelection, tab, noSelectionBehavior = SHORTCUT_NO_SELECTION_BEHAVIOR.DO_NOTHING, + sendTabResult = new Error("Tab execution failed"), }: { storedSelection: { text: string; tabId: number | null } tab: { id: number; url: string; windowId: number } noSelectionBehavior?: SHORTCUT_NO_SELECTION_BEHAVIOR + sendTabResult?: unknown }) => { const initialSettings = createTestSettings() initialSettings.shortcuts.shortcuts[0].noSelectionBehavior = @@ -319,10 +321,9 @@ describe("Service Worker Migration", () => { execute: mockExecute, })) - // Mock Ipc.sendTab to simulate tab execution failure (to force service worker execution) - const mockIpcSendTab = vi - .fn() - .mockResolvedValue(new Error("Tab execution failed")) + // Mock Ipc.sendTab. By default it simulates tab execution failure + // (to force service worker execution). + const mockIpcSendTab = vi.fn().mockResolvedValue(sendTabResult) vi.doMock("@/services/ipc", () => ({ Ipc: { sendTab: mockIpcSendTab, @@ -388,7 +389,7 @@ describe("Service Worker Migration", () => { position: { x: 600, y: 350 }, selectionText: "test selection text", target: null, - allowClipboardFallback: false, + selectionFromClipboard: false, pageUrl: "https://example.com", }), ) @@ -411,7 +412,7 @@ describe("Service Worker Migration", () => { expect(mockExecute).toHaveBeenCalledWith( expect.objectContaining({ selectionText: "", - allowClipboardFallback: true, + selectionFromClipboard: true, pageUrl: "chrome://settings/", }), ) @@ -445,10 +446,50 @@ describe("Service Worker Migration", () => { expect(mockExecute).toHaveBeenCalledWith( expect.objectContaining({ selectionText: "", - allowClipboardFallback: true, + selectionFromClipboard: true, }), ) }) + + it("MG-01-h: should pass the selection decided by the service worker to the content script", async () => { + const { commandListener, mockExecute, mockIpcSendTab } = + await setupOnCommand({ + storedSelection: { text: "selected", tabId: 1 }, + tab: { id: 1, url: "https://example.com", windowId: 1 }, + noSelectionBehavior: SHORTCUT_NO_SELECTION_BEHAVIOR.USE_CLIPBOARD, + sendTabResult: true, + }) + + await commandListener("test-shortcut-cmd") + await new Promise((resolve) => setTimeout(resolve, 10)) + + expect(mockIpcSendTab).toHaveBeenCalledWith(1, "executeAction", { + command: expect.objectContaining({ id: "test-command-123" }), + selectionText: "selected", + selectionFromClipboard: false, + }) + expect(mockExecute).not.toHaveBeenCalled() + }) + + it("MG-01-i: should tell the content script to use the clipboard when the stored selection was cleared", async () => { + // e.g. the selection was cleared when the window lost focus, while the + // text is still selected in the page. + const { commandListener, mockIpcSendTab } = await setupOnCommand({ + storedSelection: { text: "", tabId: null }, + tab: { id: 1, url: "https://example.com", windowId: 1 }, + noSelectionBehavior: SHORTCUT_NO_SELECTION_BEHAVIOR.USE_CLIPBOARD, + sendTabResult: true, + }) + + await commandListener("test-shortcut-cmd") + await new Promise((resolve) => setTimeout(resolve, 10)) + + expect(mockIpcSendTab).toHaveBeenCalledWith(1, "executeAction", { + command: expect.objectContaining({ id: "test-command-123" }), + selectionText: "", + selectionFromClipboard: true, + }) + }) }) describe("Popup Auto-Close Delay", () => { diff --git a/packages/extension/src/action/__tests__/aiPrompt.test.ts b/packages/extension/src/action/__tests__/aiPrompt.test.ts index 405ae32c1..473d1a78f 100644 --- a/packages/extension/src/action/__tests__/aiPrompt.test.ts +++ b/packages/extension/src/action/__tests__/aiPrompt.test.ts @@ -13,6 +13,7 @@ import { SelectorType, } from "@/const" import { INSERT, toInsertTemplate } from "@/services/pageAction" +import { needsClipboard } from "@/lib/utils" import type { AiService } from "@/types" vi.mock("@/services/ipc", () => ({ @@ -513,45 +514,45 @@ describe("AiPrompt.execute", () => { expect(sentArgs.url.searchUrl).toBe("https://chatgpt.com/?prompt=%s") // {{SelectedText}} is expanded, {{Clipboard}} is left for the service worker expect(sentArgs.url.selectionText).toBe("{{Clipboard}} + hello") - expect(sentArgs.url.useClipboard).toBe(true) + expect(needsClipboard(sentArgs.url)).toBe(true) expect(sentArgs.url.clipboardTemplate).toEqual({ urlToMarkdown: false }) }) - it("AP-08b: should leave {{SelectedText}} for the clipboard when there is no selection and allowClipboardFallback is true", async () => { + it("AP-08b: should leave {{SelectedText}} for the clipboard when there is no selection and selectionFromClipboard is true", async () => { vi.mocked(findAiService).mockResolvedValue(makeQueryService()) await AiPrompt.execute({ selectionText: "", command: baseCommand as any, // prompt: "Explain: {{SelectedText}}" position: { x: 0, y: 0 }, - allowClipboardFallback: true, + selectionFromClipboard: true, }) const sentArgs = vi.mocked(Ipc.send).mock.calls[0][1] as any const stepTypes = sentArgs.steps.map((s: any) => s.param.type) expect(stepTypes).not.toContain(PAGE_ACTION_EVENT.input) expect(sentArgs.url.selectionText).toBe("Explain: {{SelectedText}}") - expect(sentArgs.url.useClipboard).toBe(true) + expect(needsClipboard(sentArgs.url)).toBe(true) expect(sentArgs.url.clipboardTemplate).toEqual({ urlToMarkdown: false }) }) - it("AP-08c: should expand the selection and not read the clipboard when a selection exists even if allowClipboardFallback is true", async () => { + it("AP-08c: should expand the selection and not read the clipboard when selectionFromClipboard is false", async () => { vi.mocked(findAiService).mockResolvedValue(makeQueryService()) await AiPrompt.execute({ selectionText: "hello", command: baseCommand as any, position: { x: 0, y: 0 }, - allowClipboardFallback: true, + selectionFromClipboard: false, }) const sentArgs = vi.mocked(Ipc.send).mock.calls[0][1] as any expect(sentArgs.url.selectionText).toBe("Explain: hello") - expect(sentArgs.url.useClipboard).toBe(false) + expect(needsClipboard(sentArgs.url)).toBe(false) expect(sentArgs.url.clipboardTemplate).toBeUndefined() }) - it("AP-08d: should not read the clipboard when allowClipboardFallback is true but the prompt has no {{SelectedText}}", async () => { + it("AP-08d: should not read the clipboard when selectionFromClipboard is true but the prompt has no {{SelectedText}}", async () => { vi.mocked(findAiService).mockResolvedValue(makeQueryService()) await AiPrompt.execute({ @@ -564,13 +565,13 @@ describe("AiPrompt.execute", () => { }, } as any, position: { x: 0, y: 0 }, - allowClipboardFallback: true, + selectionFromClipboard: true, pageUrl: "https://example.com", }) const sentArgs = vi.mocked(Ipc.send).mock.calls[0][1] as any expect(sentArgs.url.selectionText).toBe("Summarize https://example.com") - expect(sentArgs.url.useClipboard).toBe(false) + expect(needsClipboard(sentArgs.url)).toBe(false) }) it("AP-08e: should pass urlToMarkdown to the service worker when the clipboard is needed", async () => { @@ -586,7 +587,7 @@ describe("AiPrompt.execute", () => { }, } as any, position: { x: 0, y: 0 }, - allowClipboardFallback: true, + selectionFromClipboard: true, }) const sentArgs = vi.mocked(Ipc.send).mock.calls[0][1] as any @@ -595,7 +596,7 @@ describe("AiPrompt.execute", () => { }) describe("DOM input approach with clipboard", () => { - it("AP-08f: should request the clipboard when there is no selection and allowClipboardFallback is true", async () => { + it("AP-08f: should request the clipboard when there is no selection and selectionFromClipboard is true", async () => { vi.mocked(findAiService).mockResolvedValue(makeDomService()) await AiPrompt.execute({ @@ -608,13 +609,14 @@ describe("AiPrompt.execute", () => { }, } as any, position: { x: 0, y: 0 }, - allowClipboardFallback: true, + selectionFromClipboard: true, }) const sentArgs = vi.mocked(Ipc.send).mock.calls[0][1] as any const stepTypes = sentArgs.steps.map((s: any) => s.param.type) expect(stepTypes).toContain(PAGE_ACTION_EVENT.input) - expect(sentArgs.url.useClipboard).toBe(true) + expect(sentArgs.url.selectionFromClipboard).toBe(true) + expect(sentArgs.url.clipboardForSteps).toBe(false) expect(sentArgs.url.clipboardTemplate).toBeUndefined() }) }) @@ -683,7 +685,8 @@ describe("AiPrompt.execute", () => { const stepTypes = storedPending.steps.map((s: any) => s.param.type) expect(stepTypes).toContain(PAGE_ACTION_EVENT.input) expect(storedPending.url).toBe("https://chatgpt.com") - expect(storedPending.useClipboard).toBe(true) + expect(storedPending.clipboardForSteps).toBe(true) + expect(storedPending.selectionFromClipboard).toBe(false) }) }) diff --git a/packages/extension/src/action/__tests__/pageAction.test.ts b/packages/extension/src/action/__tests__/pageAction.test.ts index 8ec043888..d2bac16f4 100644 --- a/packages/extension/src/action/__tests__/pageAction.test.ts +++ b/packages/extension/src/action/__tests__/pageAction.test.ts @@ -73,7 +73,7 @@ describe("PageAction.execute", () => { url: expect.objectContaining({ searchUrl: "https://ai.example.com", selectionText: "Selected content", - useClipboard: false, + clipboardForSteps: false, }), }), ) @@ -100,7 +100,7 @@ describe("PageAction.execute", () => { ServiceWorkerCommand.openAndRunPageAction, expect.objectContaining({ url: expect.objectContaining({ - useClipboard: true, + clipboardForSteps: true, }), }), ) @@ -138,7 +138,7 @@ describe("PageAction.execute", () => { ServiceWorkerCommand.openAndRunPageAction, expect.objectContaining({ url: expect.objectContaining({ - useClipboard: false, + clipboardForSteps: false, }), }), ) @@ -181,7 +181,7 @@ describe("PageAction.execute", () => { url: expect.objectContaining({ searchUrl: "https://example.com", selectionText: "sample", - useClipboard: false, + clipboardForSteps: false, }), }), ) @@ -211,7 +211,7 @@ describe("PageAction.execute", () => { selectedText: "side panel content", srcUrl: "https://origin.example.com", clipboardText: "", - useClipboard: false, + clipboardForSteps: false, userVariables: [ { name: "Prompt", value: "Summarize: {{SelectedText}}" }, ], @@ -274,7 +274,7 @@ describe("PageAction.execute", () => { expect(Storage.set).toHaveBeenCalledWith( SESSION_STORAGE_KEY.PA_SIDE_PANEL_PENDING, expect.objectContaining({ - useClipboard: true, + clipboardForSteps: true, userVariables: [ { name: "Prompt", value: "Translate clipboard: {{Clipboard}}" }, ], diff --git a/packages/extension/src/action/aiPrompt.ts b/packages/extension/src/action/aiPrompt.ts index 9275cd13d..685cd76a2 100644 --- a/packages/extension/src/action/aiPrompt.ts +++ b/packages/extension/src/action/aiPrompt.ts @@ -6,7 +6,6 @@ import { import { getWindowPosition } from "@/services/screen" import { isValidString, - isEmpty, generateRandomID, safeInterpolate, toUrl, @@ -80,7 +79,8 @@ const createEndStep = (): PageActionStep => type PromptRequirements = { needClipboard: boolean - selectionFromClipboard: boolean + clipboardInPrompt: boolean + selectedTextFromClipboard: boolean needPageHtml: boolean pageHtml: string | undefined needSelectionHtml: boolean @@ -94,20 +94,19 @@ type PromptRequirements = { const analyzePromptRequirements = ( aiPromptOption: AiPromptOption, service: AiService, - selectionText: string, - allowClipboardFallback: boolean, + selectionFromClipboard: boolean, ): PromptRequirements => { - // {{SelectedText}} falls back to the clipboard text when the command is run - // without a selection and the caller allows it (e.g. from a shortcut key). - const selectionFromClipboard = - allowClipboardFallback && - isEmpty(selectionText) && + // {{SelectedText}} is resolved with the clipboard text when the caller + // decided to use it as the selected text (e.g. from a shortcut key). + const selectedTextFromClipboard = + selectionFromClipboard && aiPromptOption.prompt.includes(toInsertTemplate(INSERT.SELECTED_TEXT)) // Checks if the prompt requires clipboard data - const needClipboard = - aiPromptOption.prompt.includes(toInsertTemplate(INSERT.CLIPBOARD)) || - selectionFromClipboard + const clipboardInPrompt = aiPromptOption.prompt.includes( + toInsertTemplate(INSERT.CLIPBOARD), + ) + const needClipboard = clipboardInPrompt || selectedTextFromClipboard // Detect HTML placeholders that require file-paste upload instead of text embedding. // File paste cannot be used with queryUrl mode (URL length limit), so these force @@ -139,7 +138,8 @@ const analyzePromptRequirements = ( return { needClipboard, - selectionFromClipboard, + clipboardInPrompt, + selectedTextFromClipboard, needPageHtml, pageHtml, needSelectionHtml, @@ -225,7 +225,7 @@ const buildQueryUrlSteps = ( selectionText: string, pageUrl: string | undefined, needClipboard: boolean, - selectionFromClipboard: boolean, + selectedTextFromClipboard: boolean, ): { steps: PageActionStep[]; urlParam: UrlParam; serviceUrl: string } => { // Pre-expand the prompt template with synchronously available variables. // INSERT.CLIPBOARD (and INSERT.SELECTED_TEXT when it falls back to the @@ -236,7 +236,7 @@ const buildQueryUrlSteps = ( [InsertSymbol[INSERT.URL]]: pageUrl ?? "", [InsertSymbol[INSERT.LANG]]: getUILanguage(), } - if (!selectionFromClipboard) { + if (!selectedTextFromClipboard) { variables[InsertSymbol[INSERT.SELECTED_TEXT]] = selectionText } const expandedPrompt = safeInterpolate(aiPromptOption.prompt, variables) @@ -248,7 +248,6 @@ const buildQueryUrlSteps = ( const urlParam: UrlParam = { searchUrl: service.queryUrl!, selectionText: finalPrompt, - useClipboard: needClipboard, clipboardTemplate: needClipboard ? { urlToMarkdown: service.urlToMarkdown ?? false } : undefined, @@ -329,7 +328,8 @@ const buildDomInputSteps = ( service: AiService, aiPromptOption: AiPromptOption, selectionText: string, - needClipboard: boolean, + clipboardInPrompt: boolean, + selectedTextFromClipboard: boolean, needPageHtml: boolean, needSelectionHtml: boolean, needFilePaste: boolean, @@ -341,7 +341,8 @@ const buildDomInputSteps = ( const urlParam: UrlParam = { searchUrl: service.url, selectionText, - useClipboard: needClipboard, + selectionFromClipboard: selectedTextFromClipboard, + clipboardForSteps: clipboardInPrompt, } // When HTML placeholders are present, build filePaste steps that upload @@ -409,7 +410,8 @@ const runSidePanelAction = async (params: { steps: PageActionStep[] selectionText: string pageUrl: string | undefined - needClipboard: boolean + clipboardInPrompt: boolean + selectedTextFromClipboard: boolean pageHtml: string | undefined selectionHtml: string | undefined }): Promise => { @@ -418,7 +420,8 @@ const runSidePanelAction = async (params: { steps, selectionText, pageUrl, - needClipboard, + clipboardInPrompt, + selectedTextFromClipboard, pageHtml, selectionHtml, } = params @@ -430,7 +433,8 @@ const runSidePanelAction = async (params: { srcUrl: pageUrl ?? "", clipboardText: "", // Always false with the query URL approach: see analyzePromptRequirements. - useClipboard: needClipboard, + selectionFromClipboard: selectedTextFromClipboard, + clipboardForSteps: clipboardInPrompt, pageHtml, selectionHtml, } @@ -468,7 +472,7 @@ export const AiPrompt = { command, position, useSecondary, - allowClipboardFallback, + selectionFromClipboard, pageUrl, }: ExecuteCommandParams) { if (!isAiPromptType(command)) { @@ -491,7 +495,8 @@ export const AiPrompt = { const { needClipboard, - selectionFromClipboard, + clipboardInPrompt, + selectedTextFromClipboard, needPageHtml, pageHtml, needSelectionHtml, @@ -501,8 +506,7 @@ export const AiPrompt = { } = analyzePromptRequirements( aiPromptOption, service, - selectionText, - allowClipboardFallback ?? false, + selectionFromClipboard ?? false, ) const { steps, urlParam, serviceUrl } = useQueryUrl @@ -512,13 +516,14 @@ export const AiPrompt = { selectionText, pageUrl, needClipboard, - selectionFromClipboard, + selectedTextFromClipboard, ) : buildDomInputSteps( service, aiPromptOption, selectionText, - needClipboard, + clipboardInPrompt, + selectedTextFromClipboard, needPageHtml, needSelectionHtml, needFilePaste, @@ -533,7 +538,8 @@ export const AiPrompt = { steps, selectionText, pageUrl, - needClipboard, + clipboardInPrompt, + selectedTextFromClipboard, pageHtml, selectionHtml, }) diff --git a/packages/extension/src/action/backgroundTab.ts b/packages/extension/src/action/backgroundTab.ts index a60a287dd..85344c72e 100644 --- a/packages/extension/src/action/backgroundTab.ts +++ b/packages/extension/src/action/backgroundTab.ts @@ -8,7 +8,7 @@ export const BackgroundTab = { async execute({ selectionText, command, - allowClipboardFallback, + selectionFromClipboard, pageUrl, }: ExecuteCommandParams) { if (!isValidString(command.searchUrl)) { @@ -21,7 +21,7 @@ export const BackgroundTab = { searchUrl: command.searchUrl, spaceEncoding: command.spaceEncoding ?? SPACE_ENCODING.PLUS, selectionText, - useClipboard: allowClipboardFallback ?? false, + selectionFromClipboard: selectionFromClipboard ?? false, pageUrl: pageUrl ?? "", }, active: false, diff --git a/packages/extension/src/action/executor.ts b/packages/extension/src/action/executor.ts index ba76c6a6f..2932c73bd 100644 --- a/packages/extension/src/action/executor.ts +++ b/packages/extension/src/action/executor.ts @@ -14,7 +14,7 @@ export async function executeAction({ selectionText, target, useSecondary = false, - allowClipboardFallback = false, + selectionFromClipboard = false, changeState, pageUrl, }: ExecuteCommandParams & { actions: Record }) { @@ -32,7 +32,7 @@ export async function executeAction({ command, position, useSecondary, - allowClipboardFallback, + selectionFromClipboard, changeState: changeState ?? (() => {}), target: target ?? null, pageUrl, diff --git a/packages/extension/src/action/index.ts b/packages/extension/src/action/index.ts index 4eacc9ca2..77239338d 100644 --- a/packages/extension/src/action/index.ts +++ b/packages/extension/src/action/index.ts @@ -37,7 +37,7 @@ export async function execute({ selectionText, target, useSecondary = false, - allowClipboardFallback = false, + selectionFromClipboard = false, changeState, }: ExecuteCommandParams) { return executeAction({ @@ -46,7 +46,7 @@ export async function execute({ selectionText, target, useSecondary, - allowClipboardFallback, + selectionFromClipboard, changeState, // This module only ever runs in the content script's page context, so // location.href here correctly reflects the visited page. diff --git a/packages/extension/src/action/pageAction.ts b/packages/extension/src/action/pageAction.ts index a4e598e4e..0e56d9321 100644 --- a/packages/extension/src/action/pageAction.ts +++ b/packages/extension/src/action/pageAction.ts @@ -23,7 +23,7 @@ export const PageAction = { command, position, useSecondary, - allowClipboardFallback, + selectionFromClipboard, userVariables, pageUrl, }: ExecuteCommandParams & PageActionParams) { @@ -41,7 +41,7 @@ export const PageAction = { userVariables ?? command.pageActionOption.userVariables // Checks if any step needs clipboard data, directly or through a user variable - const needClipboard = command.pageActionOption.steps.some( + const clipboardForSteps = command.pageActionOption.steps.some( (step) => step.param.type === PAGE_ACTION_EVENT.input && templateReferencesInsert( @@ -63,7 +63,8 @@ export const PageAction = { selectedText: selectionText, srcUrl: pageUrl ?? "", clipboardText: "", - useClipboard: needClipboard || (allowClipboardFallback ?? false), + selectionFromClipboard: selectionFromClipboard ?? false, + clipboardForSteps, userVariables: effectiveUserVariables, } try { @@ -89,7 +90,8 @@ export const PageAction = { const url: UrlParam = { searchUrl: command.pageActionOption.startUrl, selectionText, - useClipboard: needClipboard || (allowClipboardFallback ?? false), + selectionFromClipboard: selectionFromClipboard ?? false, + clipboardForSteps, pageUrl: pageUrl ?? "", } diff --git a/packages/extension/src/action/popup.ts b/packages/extension/src/action/popup.ts index 771d4b4ad..8adc1f49f 100644 --- a/packages/extension/src/action/popup.ts +++ b/packages/extension/src/action/popup.ts @@ -11,7 +11,7 @@ export const Popup = { selectionText, command, position, - allowClipboardFallback, + selectionFromClipboard, pageUrl, }: ExecuteCommandParams) { if (!isValidString(command.searchUrl)) { @@ -31,7 +31,7 @@ export const Popup = { searchUrl: command.searchUrl, spaceEncoding: command.spaceEncoding ?? SPACE_ENCODING.PLUS, selectionText, - useClipboard: allowClipboardFallback ?? false, + selectionFromClipboard: selectionFromClipboard ?? false, pageUrl: pageUrl ?? "", }, top: Math.floor(windowPosition.top + position.y), diff --git a/packages/extension/src/action/serviceWorker.ts b/packages/extension/src/action/serviceWorker.ts index 1b7a4109b..12fc86e43 100644 --- a/packages/extension/src/action/serviceWorker.ts +++ b/packages/extension/src/action/serviceWorker.ts @@ -25,7 +25,7 @@ export async function execute({ selectionText, target, useSecondary = false, - allowClipboardFallback = false, + selectionFromClipboard = false, changeState, pageUrl, }: ExecuteCommandParams) { @@ -35,7 +35,7 @@ export async function execute({ selectionText, target, useSecondary, - allowClipboardFallback, + selectionFromClipboard, changeState, pageUrl, actions: actionsForServiceWorker, diff --git a/packages/extension/src/action/sidePanel.ts b/packages/extension/src/action/sidePanel.ts index c9288220f..a5af2b098 100644 --- a/packages/extension/src/action/sidePanel.ts +++ b/packages/extension/src/action/sidePanel.ts @@ -1,4 +1,4 @@ -import { isValidString, toUrl, isEmpty } from "@/lib/utils" +import { isValidString, toUrl } from "@/lib/utils" import { SPACE_ENCODING } from "@/const" import type { ExecuteCommandParams, ShowToastParam } from "@/types" import type { OpenSidePanelProps } from "@/services/chrome" @@ -9,7 +9,7 @@ export const SidePanel = { async execute({ selectionText, command, - allowClipboardFallback, + selectionFromClipboard, pageUrl, }: ExecuteCommandParams) { if (!isValidString(command.searchUrl)) { @@ -19,7 +19,7 @@ export const SidePanel = { // Read clipboard text for interpolation, but don't block execution if it fails. let clipboardText: string = "" try { - if (allowClipboardFallback && isEmpty(selectionText)) { + if (selectionFromClipboard) { clipboardText = await navigator.clipboard.readText() } } catch (e) { @@ -39,7 +39,7 @@ export const SidePanel = { searchUrl: command.searchUrl, spaceEncoding: command.spaceEncoding ?? SPACE_ENCODING.PLUS, selectionText, - useClipboard: allowClipboardFallback ?? false, + selectionFromClipboard: selectionFromClipboard ?? false, pageUrl: pageUrl ?? "", }, clipboardText, diff --git a/packages/extension/src/action/tab.ts b/packages/extension/src/action/tab.ts index 23bb6a799..d74a7f0fb 100644 --- a/packages/extension/src/action/tab.ts +++ b/packages/extension/src/action/tab.ts @@ -8,7 +8,7 @@ export const Tab = { async execute({ selectionText, command, - allowClipboardFallback, + selectionFromClipboard, pageUrl, }: ExecuteCommandParams) { if (!isValidString(command.searchUrl)) { @@ -21,7 +21,7 @@ export const Tab = { searchUrl: command.searchUrl, spaceEncoding: command.spaceEncoding ?? SPACE_ENCODING.PLUS, selectionText, - useClipboard: allowClipboardFallback ?? false, + selectionFromClipboard: selectionFromClipboard ?? false, pageUrl: pageUrl ?? "", }, active: true, diff --git a/packages/extension/src/action/window.ts b/packages/extension/src/action/window.ts index 75c20b8a5..30eb3f47c 100644 --- a/packages/extension/src/action/window.ts +++ b/packages/extension/src/action/window.ts @@ -11,7 +11,7 @@ export const Window = { selectionText, command, position, - allowClipboardFallback, + selectionFromClipboard, pageUrl, }: ExecuteCommandParams) { if (!isValidString(command.searchUrl)) { @@ -31,7 +31,7 @@ export const Window = { searchUrl: command.searchUrl, spaceEncoding: command.spaceEncoding ?? SPACE_ENCODING.PLUS, selectionText, - useClipboard: allowClipboardFallback ?? false, + selectionFromClipboard: selectionFromClipboard ?? false, pageUrl: pageUrl ?? "", }, top: Math.floor(windowPosition.top + position.y), diff --git a/packages/extension/src/components/menu/InvisibleItem.tsx b/packages/extension/src/components/menu/InvisibleItem.tsx index c9c1315f8..91ef01649 100644 --- a/packages/extension/src/components/menu/InvisibleItem.tsx +++ b/packages/extension/src/components/menu/InvisibleItem.tsx @@ -17,7 +17,7 @@ type InvisibleItemProps = { export function InvisibleItem(props: InvisibleItemProps): React.ReactNode { const { selectionText, target } = useSelectContext() - const { command, setCommand, allowClipboardFallback } = useContextMenu() + const { action, clearAction } = useContextMenu() const { itemState, result, executeCommand, clearResult } = useCommandExecutor() const elmRef = useRef(null) @@ -62,15 +62,17 @@ export function InvisibleItem(props: InvisibleItemProps): React.ReactNode { } }, [position]) - if (command != null) { + if (action != null) { executeCommand({ - command: command as SelectionCommand, + command: action.command as SelectionCommand, position, - selectionText, + // Prefer the selection decided by the sender, since the selection in + // this tab may be stale (e.g. after the window lost focus). + selectionText: action.selectionText ?? selectionText, target, - allowClipboardFallback, + selectionFromClipboard: action.selectionFromClipboard ?? false, }) - setCommand(null) + clearAction() } const visible = result != null diff --git a/packages/extension/src/components/menu/__tests__/InvisibleItem.test.tsx b/packages/extension/src/components/menu/__tests__/InvisibleItem.test.tsx new file mode 100644 index 000000000..b38f63043 --- /dev/null +++ b/packages/extension/src/components/menu/__tests__/InvisibleItem.test.tsx @@ -0,0 +1,82 @@ +import { describe, it, expect, vi, beforeEach } from "vitest" +import { render } from "@testing-library/react" +import { InvisibleItem } from "../InvisibleItem" +import { ExecState } from "@/const" +import type { executeActionProps } from "@/services/contextMenus" + +const mockExecuteCommand = vi.fn() +const mockClearAction = vi.fn() +let mockAction: executeActionProps | null = null + +vi.mock("@/hooks/useCommandExecutor", () => ({ + useCommandExecutor: () => ({ + itemState: { state: ExecState.NONE, message: "" }, + result: null, + executeCommand: mockExecuteCommand, + clearResult: vi.fn(), + }), +})) + +vi.mock("@/hooks/useSelectContext", () => ({ + useSelectContext: () => ({ + selectionText: "selection in tab", + target: null, + }), +})) + +vi.mock("@/hooks/useContextMenu", () => ({ + useContextMenu: () => ({ + action: mockAction, + clearAction: mockClearAction, + }), +})) + +const command = { + id: "cmd-1", + title: "Search", + searchUrl: "https://example.com/?q=%s", + openMode: "tab", +} as any + +describe("InvisibleItem", () => { + beforeEach(() => { + vi.clearAllMocks() + mockAction = null + }) + + it("II-01: 正常系: 送信元が決めた選択テキストとクリップボード利用指定で実行する", () => { + mockAction = { command, selectionText: "", selectionFromClipboard: true } + + render() + + expect(mockExecuteCommand).toHaveBeenCalledWith( + expect.objectContaining({ + command, + selectionText: "", + selectionFromClipboard: true, + }), + ) + expect(mockClearAction).toHaveBeenCalled() + }) + + it("II-02: 正常系: 送信元が選択テキストを指定しない場合はタブの選択テキストで実行する", () => { + // e.g. the context menu + mockAction = { command } + + render() + + expect(mockExecuteCommand).toHaveBeenCalledWith( + expect.objectContaining({ + command, + selectionText: "selection in tab", + selectionFromClipboard: false, + }), + ) + }) + + it("II-03: 正常系: 実行要求がなければ何もしない", () => { + render() + + expect(mockExecuteCommand).not.toHaveBeenCalled() + }) +}) diff --git a/packages/extension/src/components/option/editor/SearchUrlAssistDialog.tsx b/packages/extension/src/components/option/editor/SearchUrlAssistDialog.tsx index 3f02e4774..507e55d0a 100644 --- a/packages/extension/src/components/option/editor/SearchUrlAssistDialog.tsx +++ b/packages/extension/src/components/option/editor/SearchUrlAssistDialog.tsx @@ -87,7 +87,7 @@ export const SearchUrlAssistDialog = ({ command, position: { x: 0, y: 0 }, useSecondary: false, - allowClipboardFallback: false, + selectionFromClipboard: false, userVariables: [ { name: "search_keyword", value: data.searchKeyword }, { name: "search_result_url", value: data.searchResultUrl }, diff --git a/packages/extension/src/hooks/useCommandExecutor.ts b/packages/extension/src/hooks/useCommandExecutor.ts index 184d2473f..8a03c200c 100644 --- a/packages/extension/src/hooks/useCommandExecutor.ts +++ b/packages/extension/src/hooks/useCommandExecutor.ts @@ -14,7 +14,7 @@ type ExecuteCommandParams = { selectionText: string target: Element | null useSecondary?: boolean - allowClipboardFallback?: boolean + selectionFromClipboard?: boolean } export function useCommandExecutor() { @@ -33,7 +33,7 @@ export function useCommandExecutor() { selectionText, target, useSecondary = false, - allowClipboardFallback = false, + selectionFromClipboard = false, }: ExecuteCommandParams) => { if (itemState.state !== ExecState.NONE) { return @@ -45,7 +45,7 @@ export function useCommandExecutor() { selectionText, target, useSecondary, - allowClipboardFallback, + selectionFromClipboard, changeState: onChangeState, }) diff --git a/packages/extension/src/hooks/useContextMenu.ts b/packages/extension/src/hooks/useContextMenu.ts index 1b9f81d2e..bf0b9bad2 100644 --- a/packages/extension/src/hooks/useContextMenu.ts +++ b/packages/extension/src/hooks/useContextMenu.ts @@ -2,17 +2,13 @@ import { useState, useEffect } from "react" import { Ipc, TabCommand } from "@/services/ipc" import type { IpcCallback } from "@/services/ipc" import { executeActionProps } from "@/services/contextMenus" -import { Command } from "@/types" export function useContextMenu() { - const [command, setCommand] = useState(null) - const [allowClipboardFallback, setAllowClipboardFallback] = - useState(false) + const [action, setAction] = useState(null) useEffect(() => { Ipc.addListener(TabCommand.executeAction, ((param: executeActionProps) => { - setCommand(param.command) - setAllowClipboardFallback(param.allowClipboardFallback ?? false) + setAction(param) return false }) as IpcCallback) return () => { @@ -20,5 +16,7 @@ export function useContextMenu() { } }, []) - return { command, setCommand, allowClipboardFallback } + const clearAction = () => setAction(null) + + return { action, clearAction } } diff --git a/packages/extension/src/lib/__tests__/utils.test.ts b/packages/extension/src/lib/__tests__/utils.test.ts index caca88761..4bc29cf83 100644 --- a/packages/extension/src/lib/__tests__/utils.test.ts +++ b/packages/extension/src/lib/__tests__/utils.test.ts @@ -4,6 +4,7 @@ import { toUrl, matchesPageActionUrl, getCommandTargetUrl, + needsClipboard, } from "../utils" import { OPEN_MODE, PAGE_ACTION_OPEN_MODE, SPACE_ENCODING } from "@/const" import type { UrlParam } from "@/types" @@ -124,11 +125,11 @@ describe("toUrl", () => { expect(toUrl(param)).toBe("https://example.com/search?q=hello%2Fworld") }) - it("TU-06: uses clipboard text when useClipboard is true and selectionText is empty", () => { + it("TU-06: uses clipboard text when selectionFromClipboard is true", () => { const param: UrlParam = { searchUrl: "https://example.com/search?q=%s", selectionText: "", - useClipboard: true, + selectionFromClipboard: true, } const clipboardText = "clipboard content" expect(toUrl(param, clipboardText)).toBe( @@ -136,11 +137,10 @@ describe("toUrl", () => { ) }) - it("TU-07: prefers selectionText over clipboard when both are present", () => { + it("TU-07: uses selectionText when selectionFromClipboard is not set even if clipboard text is given", () => { const param: UrlParam = { searchUrl: "https://example.com/search?q=%s", selectionText: "selection", - useClipboard: true, } const clipboardText = "clipboard" expect(toUrl(param, clipboardText)).toBe( @@ -148,11 +148,11 @@ describe("toUrl", () => { ) }) - it("TU-08: uses empty string when useClipboard is true but clipboard is undefined", () => { + it("TU-08: uses empty string when selectionFromClipboard is true but clipboard is undefined", () => { const param: UrlParam = { searchUrl: "https://example.com/search?q=%s", selectionText: "", - useClipboard: true, + selectionFromClipboard: true, } expect(toUrl(param)).toBe("https://example.com/search?q=") }) @@ -161,7 +161,6 @@ describe("toUrl", () => { const param: UrlParam = { searchUrl: "https://example.com/?prompt=%s", selectionText: "Explain: {{SelectedText}} / {{Clipboard}}", - useClipboard: true, clipboardTemplate: {}, } expect(toUrl(param, "clip")).toBe( @@ -173,7 +172,6 @@ describe("toUrl", () => { const param: UrlParam = { searchUrl: "https://example.com/?prompt=%s", selectionText: "{{Unknown}} {{Clipboard}}", - useClipboard: true, clipboardTemplate: {}, } expect(toUrl(param, "clip")).toBe( @@ -185,7 +183,6 @@ describe("toUrl", () => { const param: UrlParam = { searchUrl: "https://example.com/?prompt=%s", selectionText: "Explain: {{SelectedText}}", - useClipboard: true, clipboardTemplate: {}, } expect(toUrl(param)).toBe("https://example.com/?prompt=Explain%3A+") @@ -196,7 +193,6 @@ describe("toUrl", () => { searchUrl: "https://example.com/?q=%s", selectionText: "[https://a.com](https://a.com) {{Clipboard}}", spaceEncoding: SPACE_ENCODING.PERCENT, - useClipboard: true, clipboardTemplate: { urlToMarkdown: true }, } // Already-converted links in the template are not converted twice. @@ -208,16 +204,14 @@ describe("toUrl", () => { ) }) - it("TU-08-e: ignores clipboardTemplate when useClipboard is false", () => { + it("TU-08-e: does not check selectionText again when selectionFromClipboard is true", () => { + // Whether to use the clipboard is decided by the caller only once. const param: UrlParam = { - searchUrl: "https://example.com/?prompt=%s", - selectionText: "{{Clipboard}}", - useClipboard: false, - clipboardTemplate: {}, + searchUrl: "https://example.com/search?q=%s", + selectionText: "selection", + selectionFromClipboard: true, } - expect(toUrl(param, "clip")).toBe( - "https://example.com/?prompt=%7B%7BClipboard%7D%7D", - ) + expect(toUrl(param, "clip")).toBe("https://example.com/search?q=clip") }) it("TU-09: URL encodes special characters", () => { @@ -310,6 +304,33 @@ describe("toUrl", () => { }) }) +describe("needsClipboard", () => { + const base: UrlParam = { + searchUrl: "https://example.com/search?q=%s", + selectionText: "", + } + + it("NC-01: returns false for a plain string url", () => { + expect(needsClipboard("https://example.com")).toBe(false) + }) + + it("NC-02: returns false when no part of the url uses the clipboard", () => { + expect(needsClipboard(base)).toBe(false) + }) + + it("NC-03: returns true when selectionFromClipboard is true", () => { + expect(needsClipboard({ ...base, selectionFromClipboard: true })).toBe(true) + }) + + it("NC-04: returns true when clipboardForSteps is true", () => { + expect(needsClipboard({ ...base, clipboardForSteps: true })).toBe(true) + }) + + it("NC-05: returns true when clipboardTemplate is set", () => { + expect(needsClipboard({ ...base, clipboardTemplate: {} })).toBe(true) + }) +}) + describe("matchesPageActionUrl", () => { it("MU-01: exact match without wildcard", () => { expect( diff --git a/packages/extension/src/lib/utils.ts b/packages/extension/src/lib/utils.ts index c32d68640..4d450e53b 100644 --- a/packages/extension/src/lib/utils.ts +++ b/packages/extension/src/lib/utils.ts @@ -52,6 +52,20 @@ export const isUrlParam = (url: string | UrlParam): url is UrlParam => { return typeof url === "object" && "searchUrl" in url && "selectionText" in url } +/** + * Whether the clipboard must be read when opening the url. + * @param url The url to open. + * @returns true if any part of the url uses the clipboard text. + */ +export const needsClipboard = (url: string | UrlParam): boolean => { + if (!isUrlParam(url)) return false + return ( + !!url.selectionFromClipboard || + !!url.clipboardForSteps || + url.clipboardTemplate != null + ) +} + /** * Convert bare URLs in text to Markdown link format [URL](URL). * URLs already in Markdown link format ([text](url)) are returned unchanged, @@ -77,7 +91,7 @@ export function convertUrlsToMarkdown(text: string): string { /** * Convert a string or UrlParam to a URL. * @param param The string or UrlParam to convert. - * @param clipboardText The clipboard text to use if the UrlParam has useClipboard set to true. + * @param clipboardText The clipboard text, read when needsClipboard(param) is true. * @returns The URL. */ export function toUrl( @@ -91,10 +105,10 @@ export function toUrl( searchUrl, selectionText, spaceEncoding = SPACE_ENCODING.PLUS, - useClipboard = false, + selectionFromClipboard = false, } = param let text = selectionText - if (useClipboard && param.clipboardTemplate) { + if (param.clipboardTemplate) { // selectionText is a pre-expanded prompt whose clipboard-dependent // placeholders were left unresolved, because the clipboard can only be // read here in the service worker. @@ -106,7 +120,7 @@ export function toUrl( if (param.clipboardTemplate.urlToMarkdown) { text = convertUrlsToMarkdown(text) } - } else if (useClipboard && isEmpty(text)) { + } else if (selectionFromClipboard) { text = clipboardText ?? "" } // URL encode the text diff --git a/packages/extension/src/providers/SelectContextProvider.tsx b/packages/extension/src/providers/SelectContextProvider.tsx index 017a6a689..431fd60d5 100644 --- a/packages/extension/src/providers/SelectContextProvider.tsx +++ b/packages/extension/src/providers/SelectContextProvider.tsx @@ -46,24 +46,9 @@ export const SelectContextProvider = ({ await setSelectionText(text) } - // The stored selection is shared by all tabs, so re-record this tab's - // selection when the tab becomes active again. An empty selection is not - // recorded, so that a frame without a selection doesn't overwrite the one - // recorded by another frame in this tab. (A selection recorded in another - // tab is ignored by the shortcut handler anyway.) - const onVisibilitychange = async () => { - if (!detectSelectionEnabled) return - if (document.visibilityState !== "visible") return - const text = getSelectionText() - if (!text) return - await setSelectionText(text) - } - document.addEventListener("selectionchange", onSelectionchange) - document.addEventListener("visibilitychange", onVisibilitychange) return () => { document.removeEventListener("selectionchange", onSelectionchange) - document.removeEventListener("visibilitychange", onVisibilitychange) } }, [detectSelectionEnabled, setSelectionText]) diff --git a/packages/extension/src/providers/__tests__/SelectContextProvider.test.tsx b/packages/extension/src/providers/__tests__/SelectContextProvider.test.tsx index bad0462b3..ddce7a559 100644 --- a/packages/extension/src/providers/__tests__/SelectContextProvider.test.tsx +++ b/packages/extension/src/providers/__tests__/SelectContextProvider.test.tsx @@ -1,4 +1,4 @@ -import { describe, it, expect, vi, beforeEach, afterEach } from "vitest" +import { describe, it, expect, vi, beforeEach } from "vitest" import { render, waitFor } from "@testing-library/react" import { SelectContextProvider } from "../SelectContextProvider" import { Storage, SESSION_STORAGE_KEY } from "@/services/storage" @@ -28,23 +28,11 @@ const mockSet = vi.mocked(Storage.set) const mockGetTabId = vi.mocked(Ipc.getTabId) const mockGetSelectionText = vi.mocked(getSelectionText) -const setVisibilityState = (state: DocumentVisibilityState) => { - Object.defineProperty(document, "visibilityState", { - configurable: true, - get: () => state, - }) -} - describe("SelectContextProvider", () => { beforeEach(() => { vi.clearAllMocks() mockSet.mockResolvedValue(true) mockGetTabId.mockResolvedValue(10) - setVisibilityState("visible") - }) - - afterEach(() => { - setVisibilityState("visible") }) it("SC-01: 正常系: 選択変更時に選択テキストとタブIDを保存する", async () => { @@ -86,39 +74,4 @@ describe("SelectContextProvider", () => { await waitFor(() => expect(mockSet).toHaveBeenCalledTimes(2)) expect(mockGetTabId).toHaveBeenCalledTimes(1) }) - - it("SC-04: 正常系: タブが表示されたときに選択中のテキストを再保存する", async () => { - mockGetSelectionText.mockReturnValue("kept selection") - render({null}) - - document.dispatchEvent(new Event("visibilitychange")) - - await waitFor(() => { - expect(mockSet).toHaveBeenCalledWith(SESSION_STORAGE_KEY.SELECTION_TEXT, { - text: "kept selection", - tabId: 10, - }) - }) - }) - - it("SC-05: 境界値: タブが表示されたときに選択が空なら保存しない", async () => { - mockGetSelectionText.mockReturnValue("") - render({null}) - - document.dispatchEvent(new Event("visibilitychange")) - await new Promise((resolve) => setTimeout(resolve, 10)) - - expect(mockSet).not.toHaveBeenCalled() - }) - - it("SC-06: 境界値: タブが非表示になったときは保存しない", async () => { - mockGetSelectionText.mockReturnValue("selected") - setVisibilityState("hidden") - render({null}) - - document.dispatchEvent(new Event("visibilitychange")) - await new Promise((resolve) => setTimeout(resolve, 10)) - - expect(mockSet).not.toHaveBeenCalled() - }) }) diff --git a/packages/extension/src/service_worker.ts b/packages/extension/src/service_worker.ts index a6ae86833..13083c227 100644 --- a/packages/extension/src/service_worker.ts +++ b/packages/extension/src/service_worker.ts @@ -717,8 +717,10 @@ chrome.commands.onCommand.addListener(async (commandName) => { ? selection.text : "" - // If no text is selected, handle according to noSelectionBehavior - let allowClipboardFallback = false + // If no text is selected, handle according to noSelectionBehavior. + // This is the only place to decide whether to use the clipboard text as + // the selected text. + let selectionFromClipboard = false if (isEmpty(selectionText)) { if ( shortcut.noSelectionBehavior === @@ -729,7 +731,7 @@ chrome.commands.onCommand.addListener(async (commandName) => { shortcut.noSelectionBehavior === SHORTCUT_NO_SELECTION_BEHAVIOR.USE_CLIPBOARD ) { - allowClipboardFallback = true + selectionFromClipboard = true } } const enableSendTab = @@ -745,7 +747,10 @@ chrome.commands.onCommand.addListener(async (commandName) => { TabCommand.executeAction, { command, - allowClipboardFallback, + // Pass the selection decided here, so that the selection is + // judged only once. + selectionText, + selectionFromClipboard, }, ) } @@ -762,7 +767,7 @@ chrome.commands.onCommand.addListener(async (commandName) => { position, selectionText, target: null, - allowClipboardFallback, + selectionFromClipboard, pageUrl: tab?.url ?? "", }) } diff --git a/packages/extension/src/services/chrome.ts b/packages/extension/src/services/chrome.ts index 52edba606..1b3661109 100644 --- a/packages/extension/src/services/chrome.ts +++ b/packages/extension/src/services/chrome.ts @@ -1,4 +1,4 @@ -import { sleep, toUrl, isOverBytes, isUrlParam } from "@/lib/utils" +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" @@ -99,7 +99,6 @@ export const fetchIconUrl = async (url: string): Promise => { url: toUrl({ searchUrl: url, selectionText: "test", - useClipboard: false, }), state: "minimized", }) @@ -475,7 +474,7 @@ export const openPopupWindow = async ( let window: chrome.windows.Window let clipboardText = "" - if (isUrlParam(url) && url.useClipboard) { + if (needsClipboard(url)) { const result = await openWindowAndReadClipboard({ commandId: param.commandId, type, @@ -607,12 +606,7 @@ export const openTab = async (param: OpenTabProps): Promise => { console.warn("Failed to get current tab:", e) } - let useClipboard = false - if (isUrlParam(url)) { - useClipboard = url.useClipboard ?? false - } - - if (useClipboard) { + if (needsClipboard(url)) { const tab = await chrome.tabs.create({ url: chrome.runtime.getURL("src/clipboard.html"), active: true, diff --git a/packages/extension/src/services/contextMenus.ts b/packages/extension/src/services/contextMenus.ts index 2a707644c..461ed844e 100644 --- a/packages/extension/src/services/contextMenus.ts +++ b/packages/extension/src/services/contextMenus.ts @@ -11,8 +11,13 @@ import { export type executeActionProps = { command: Command - /** See ExecuteCommandParams.allowClipboardFallback. */ - allowClipboardFallback?: boolean + /** + * The selected text decided by the sender (e.g. the shortcut handler). + * If undefined, the current selection in the tab is used. + */ + selectionText?: string + /** See ExecuteCommandParams.selectionFromClipboard. */ + selectionFromClipboard?: boolean } type Callback = () => void diff --git a/packages/extension/src/services/ipc.ts b/packages/extension/src/services/ipc.ts index ec93cb32b..bb54f9edf 100644 --- a/packages/extension/src/services/ipc.ts +++ b/packages/extension/src/services/ipc.ts @@ -150,7 +150,10 @@ export type SidePanelPendingAction = { selectedText: string srcUrl: string clipboardText: string - useClipboard?: boolean + /** See UrlParam.selectionFromClipboard. */ + selectionFromClipboard?: boolean + /** See UrlParam.clipboardForSteps. */ + clipboardForSteps?: boolean pageHtml?: string selectionHtml?: string userVariables?: Array diff --git a/packages/extension/src/services/pageAction/__tests__/serviceWorker-execution.test.ts b/packages/extension/src/services/pageAction/__tests__/serviceWorker-execution.test.ts index 301fafbae..381f9a4b4 100644 --- a/packages/extension/src/services/pageAction/__tests__/serviceWorker-execution.test.ts +++ b/packages/extension/src/services/pageAction/__tests__/serviceWorker-execution.test.ts @@ -10,9 +10,9 @@ import { mockReadClipboard, mockMatchesPageActionUrl, mockIncrementCommandExecutionCount, - mockIsEmpty, mockIsUrl, mockIsUrlParam, + mockNeedsClipboard, mockConsole, } from "./serviceWorker-setup" import { openAndRun, preview, stopRunner } from "../serviceWorker" @@ -329,10 +329,10 @@ describe("serviceWorker.ts - Execution Operations", () => { expect(mockOpenTab).not.toHaveBeenCalled() }) - it("SWD-45: Normal case: Clipboard is fetched in CURRENT_TAB mode when useClipboard is true", async () => { + it("SWD-45: Normal case: Clipboard is fetched in CURRENT_TAB mode when the url needs the clipboard", async () => { const mockParam = { openMode: PAGE_ACTION_OPEN_MODE.CURRENT_TAB, - url: { url: "https://example.com", useClipboard: true }, + url: { url: "https://example.com", clipboardForSteps: true }, commandId: "cmd-1", selectedText: "selected", userVariables: [], @@ -350,6 +350,7 @@ describe("serviceWorker.ts - Execution Operations", () => { url: "https://example.com/page", }) mockIsUrlParam.mockReturnValue(true) + mockNeedsClipboard.mockReturnValue(true) mockReadClipboard.mockResolvedValue({ clipboardText: "clipboard content", err: null, @@ -368,10 +369,10 @@ describe("serviceWorker.ts - Execution Operations", () => { expect(mockIpc.ensureConnection).toHaveBeenCalledWith(555) }) - it("SWD-46: Normal case: Clipboard is NOT fetched in CURRENT_TAB mode when useClipboard is false", async () => { + it("SWD-46: Normal case: Clipboard is NOT fetched in CURRENT_TAB mode when the url does not need the clipboard", async () => { const mockParam = { openMode: PAGE_ACTION_OPEN_MODE.CURRENT_TAB, - url: { url: "https://example.com", useClipboard: false }, + url: { url: "https://example.com" }, commandId: "cmd-1", selectedText: "selected", userVariables: [], @@ -389,6 +390,7 @@ describe("serviceWorker.ts - Execution Operations", () => { url: "https://example.com/page", }) mockIsUrlParam.mockReturnValue(true) + mockNeedsClipboard.mockReturnValue(false) mockStorage.getCommands.mockResolvedValue([mockCommand]) const result = openAndRun( @@ -403,16 +405,15 @@ describe("serviceWorker.ts - Execution Operations", () => { expect(mockIpc.ensureConnection).toHaveBeenCalledWith(555) }) - it("SWD-44: Normal case: Use clipboardText when selectedText is empty and useClipboard is true", async () => { + it("SWD-44: Normal case: Use clipboardText as selectedText when selectionFromClipboard is true", async () => { const mockParam = { openMode: PAGE_ACTION_OPEN_MODE.TAB, - url: { url: "https://example.com", useClipboard: true }, + url: { url: "https://example.com", selectionFromClipboard: true }, commandId: "cmd-1", selectedText: "", steps: [{ id: "step-1", param: { type: "click" } }], } - mockIsEmpty.mockReturnValue(true) mockIsUrlParam.mockReturnValue(true) mockOpenTab.mockResolvedValue({ tabId: 456, @@ -433,6 +434,34 @@ describe("serviceWorker.ts - Execution Operations", () => { ) }) + it("SWD-44-b: Normal case: Keep selectedText empty when only the steps use the clipboard", async () => { + const mockParam = { + openMode: PAGE_ACTION_OPEN_MODE.TAB, + url: { url: "https://example.com", clipboardForSteps: true }, + commandId: "cmd-1", + selectedText: "", + steps: [{ id: "step-1", param: { type: "click" } }], + } + + mockIsUrlParam.mockReturnValue(true) + mockOpenTab.mockResolvedValue({ + tabId: 456, + clipboardText: "clipboard content", + }) + + openAndRun(mockParam as any, mockSender as any, mockResponse) + await vi.runAllTimersAsync() + + expect(mockIpc.sendTab).toHaveBeenCalledWith( + 456, + "execPageAction", + expect.objectContaining({ + selectedText: "", + clipboardText: "clipboard content", + }), + ) + }) + it("SWD-47: Error case: When tabId retrieval fails", async () => { const mockParam = { openMode: PAGE_ACTION_OPEN_MODE.TAB, diff --git a/packages/extension/src/services/pageAction/__tests__/serviceWorker-setup.ts b/packages/extension/src/services/pageAction/__tests__/serviceWorker-setup.ts index e4525285d..9ef5efdba 100644 --- a/packages/extension/src/services/pageAction/__tests__/serviceWorker-setup.ts +++ b/packages/extension/src/services/pageAction/__tests__/serviceWorker-setup.ts @@ -63,6 +63,7 @@ vi.mock("@/lib/utils", () => ({ isUrl: vi.fn(), isUrlParam: vi.fn(), matchesPageActionUrl: vi.fn(), + needsClipboard: vi.fn(), sleep: vi.fn(), })) @@ -121,6 +122,7 @@ import { isUrl, isUrlParam, matchesPageActionUrl, + needsClipboard, sleep, } from "@/lib/utils" import { resetLastUrl } from "../serviceWorker" @@ -141,6 +143,7 @@ export const mockIsEmpty = isEmpty as any export const mockIsPageActionCommand = isPageActionCommand as any export const mockIsUrl = isUrl as any export const mockIsUrlParam = isUrlParam as any +export const mockNeedsClipboard = needsClipboard as any export const mockMatchesPageActionUrl = matchesPageActionUrl as any export const mockSleep = sleep as any diff --git a/packages/extension/src/services/pageAction/__tests__/serviceWorker-sidePanel.test.ts b/packages/extension/src/services/pageAction/__tests__/serviceWorker-sidePanel.test.ts index 6b6ac7464..75c439dd0 100644 --- a/packages/extension/src/services/pageAction/__tests__/serviceWorker-sidePanel.test.ts +++ b/packages/extension/src/services/pageAction/__tests__/serviceWorker-sidePanel.test.ts @@ -267,9 +267,13 @@ describe("serviceWorker.ts - Side Panel Connection", () => { ) }) - it("SP-12: Does not call readClipboard when useClipboard is false", async () => { + it("SP-12: Does not call readClipboard when the pending action does not need the clipboard", async () => { const port = createMockPort("https://chatgpt.com") - const pending = { ...createPendingAction(), useClipboard: false } + const pending = { + ...createPendingAction(), + selectionFromClipboard: false, + clipboardForSteps: false, + } mockStorage.get.mockResolvedValue(pending) mockStorage.set.mockResolvedValue(undefined) @@ -278,11 +282,11 @@ describe("serviceWorker.ts - Side Panel Connection", () => { expect(mockReadClipboard).not.toHaveBeenCalled() }) - it("SP-13: Reads clipboard from service worker context when useClipboard is true", async () => { + it("SP-13: Reads clipboard from service worker context when the steps use the clipboard", async () => { const port = createMockPort("https://chatgpt.com") const pending = { ...createPendingAction(), - useClipboard: true, + clipboardForSteps: true, clipboardText: "", } mockStorage.get.mockResolvedValue(pending) @@ -300,6 +304,35 @@ describe("serviceWorker.ts - Side Panel Connection", () => { expect.objectContaining({ command: "execPageAction", param: expect.objectContaining({ + selectedText: "test", + clipboardText: "clipboard content", + }), + }), + ) + }) + + it("SP-13-b: Uses the clipboard text as selectedText when selectionFromClipboard is true", async () => { + const port = createMockPort("https://chatgpt.com") + const pending = { + ...createPendingAction(), + selectedText: "", + selectionFromClipboard: true, + clipboardText: "", + } + mockStorage.get.mockResolvedValue(pending) + mockStorage.set.mockResolvedValue(undefined) + mockReadClipboard.mockResolvedValue({ + clipboardText: "clipboard content", + }) + + await handleSidePanelConnect(port as any) + + expect(mockReadClipboard).toHaveBeenCalledTimes(1) + expect(port.postMessage).toHaveBeenCalledWith( + expect.objectContaining({ + command: "execPageAction", + param: expect.objectContaining({ + selectedText: "clipboard content", clipboardText: "clipboard content", }), }), diff --git a/packages/extension/src/services/pageAction/serviceWorker-sidePanel.ts b/packages/extension/src/services/pageAction/serviceWorker-sidePanel.ts index 65ffd4a38..4d34e26a9 100644 --- a/packages/extension/src/services/pageAction/serviceWorker-sidePanel.ts +++ b/packages/extension/src/services/pageAction/serviceWorker-sidePanel.ts @@ -194,13 +194,18 @@ const runPendingSidePanelAction = async ( return } await Storage.set(SESSION_STORAGE_KEY.PA_SIDE_PANEL_PENDING, null) - if (pending.useClipboard) { + if (pending.selectionFromClipboard || pending.clipboardForSteps) { try { const clipboard = await readClipboard() pending.clipboardText = clipboard.clipboardText } catch (e) { console.warn("Failed to read clipboard in background:", e) } + // Use the clipboard text as the selected text if requested + // (e.g. a shortcut key run without a selection). + if (pending.selectionFromClipboard) { + pending.selectedText = pending.clipboardText + } } runViaPort(port, pending) } diff --git a/packages/extension/src/services/pageAction/serviceWorker.ts b/packages/extension/src/services/pageAction/serviceWorker.ts index a13a5a4b8..260c0b514 100644 --- a/packages/extension/src/services/pageAction/serviceWorker.ts +++ b/packages/extension/src/services/pageAction/serviceWorker.ts @@ -28,10 +28,10 @@ import { } from "@/const" import { generateRandomID, - isEmpty, isUrl, isUrlParam, matchesPageActionUrl, + needsClipboard, sleep, } from "@/lib/utils" import type { @@ -382,7 +382,7 @@ export const openAndRun = ( tabId = currentTab.id // Get clipboard text only when needed. - if (isUrlParam(param.url) && param.url.useClipboard) { + if (needsClipboard(param.url)) { const clipboard = await readClipboard() clipboardText = clipboard.clipboardText if (clipboard.err) { @@ -412,13 +412,9 @@ export const openAndRun = ( return } - // Use clipboard text if selected text is empty and useClipboard is true. - // This is for the case for shortcut key. - if ( - isEmpty(selectedText) && - isUrlParam(param.url) && - param.url.useClipboard - ) { + // Use the clipboard text as the selected text if requested + // (e.g. a shortcut key run without a selection). + if (isUrlParam(param.url) && param.url.selectionFromClipboard) { selectedText = clipboardText } diff --git a/packages/extension/src/types/index.ts b/packages/extension/src/types/index.ts index 83d0906fe..9b8abfaa5 100644 --- a/packages/extension/src/types/index.ts +++ b/packages/extension/src/types/index.ts @@ -249,11 +249,12 @@ export type ExecuteCommandParams = { target?: Element | null useSecondary?: boolean /** - * Allows the clipboard text to be used in place of an empty selection - * (e.g. a shortcut key run without a selection). Each action decides whether - * it actually needs the clipboard, and requests it via UrlParam.useClipboard. + * Use the clipboard text as the selected text. This is decided only once, + * where the command is triggered (e.g. a shortcut key run without a + * selection), so selectionText is always empty when this is true and + * actions must not check selectionText again. */ - allowClipboardFallback?: boolean + selectionFromClipboard?: boolean changeState?: (state: ExecState, message?: string) => void pageUrl?: string } @@ -321,8 +322,17 @@ export type UrlParam = { searchUrl: string selectionText: string spaceEncoding?: SPACE_ENCODING - useClipboard?: boolean pageUrl?: string + /** + * Replace the selected text in searchUrl with the clipboard text. + * See ExecuteCommandParams.selectionFromClipboard. + */ + selectionFromClipboard?: boolean + /** + * Set when page action steps reference {{Clipboard}}. The clipboard text is + * read when opening the page and passed to the steps. + */ + clipboardForSteps?: boolean /** * Set when selectionText is a prompt template (AI prompt query URL) that * still contains {{Clipboard}} / {{SelectedText}} placeholders. They are From 8a582caab141455df453911718d8ecaad5736b53 Mon Sep 17 00:00:00 2001 From: ujiro99 Date: Mon, 19 Oct 2026 12:43:47 +0900 Subject: [PATCH 03/14] refactor: handle getTabId failures in Ipc and reuse getCurrentTab - Ipc.getTabId returns null instead of throwing when the tab ID can't be retrieved, so callers don't need their own error handling. - The shortcut handler gets the active tab with getCurrentTab(), like the other places in the service worker. Co-Authored-By: Claude Opus 5.5 --- .../src/__tests__/service_worker.test.ts | 8 +++-- .../src/providers/SelectContextProvider.tsx | 11 +------ .../__tests__/SelectContextProvider.test.tsx | 2 +- packages/extension/src/service_worker.ts | 2 +- .../src/services/__tests__/ipc.test.ts | 31 +++++++++++++++++++ packages/extension/src/services/connection.ts | 4 ++- packages/extension/src/services/ipc.ts | 16 ++++++++-- 7 files changed, 57 insertions(+), 17 deletions(-) diff --git a/packages/extension/src/__tests__/service_worker.test.ts b/packages/extension/src/__tests__/service_worker.test.ts index 250c67071..90bf5c5d1 100644 --- a/packages/extension/src/__tests__/service_worker.test.ts +++ b/packages/extension/src/__tests__/service_worker.test.ts @@ -293,8 +293,12 @@ describe("Service Worker Migration", () => { noSelectionBehavior as any mockEnhancedSettings.get.mockResolvedValue(initialSettings as any) - // Mock chrome.tabs.query to return the active tab - ;(chrome.tabs.query as any).mockResolvedValue([tab]) + // Mock getCurrentTab to return the active tab + vi.doMock("@/services/chrome", () => ({ + getCurrentTab: vi.fn().mockResolvedValue(tab), + closeWindow: vi.fn(), + windowExists: vi.fn(), + })) // Mock the current window used to center the popup ;(chrome.windows as any).getCurrent = vi diff --git a/packages/extension/src/providers/SelectContextProvider.tsx b/packages/extension/src/providers/SelectContextProvider.tsx index 431fd60d5..6ad08319d 100644 --- a/packages/extension/src/providers/SelectContextProvider.tsx +++ b/packages/extension/src/providers/SelectContextProvider.tsx @@ -8,15 +8,6 @@ import { Ipc } from "@/services/ipc" import { getSelectionText } from "@/services/dom" import { ContextType, selectContext } from "@/hooks/useSelectContext" -const fetchTabId = async (): Promise => { - try { - return (await Ipc.getTabId()) ?? null - } catch { - // e.g. side panel, which does not belong to a tab - return null - } -} - export const SelectContextProvider = ({ children, }: { @@ -31,7 +22,7 @@ export const SelectContextProvider = ({ _setSelectionText(text) // Record the tab ID with the text, so that the shortcut handler in the // service worker can ignore a selection recorded in another tab. - tabIdRef.current ??= fetchTabId() + tabIdRef.current ??= Ipc.getTabId() const tabId = await tabIdRef.current await Storage.set(SESSION_STORAGE_KEY.SELECTION_TEXT, { text, diff --git a/packages/extension/src/providers/__tests__/SelectContextProvider.test.tsx b/packages/extension/src/providers/__tests__/SelectContextProvider.test.tsx index ddce7a559..799f59a38 100644 --- a/packages/extension/src/providers/__tests__/SelectContextProvider.test.tsx +++ b/packages/extension/src/providers/__tests__/SelectContextProvider.test.tsx @@ -50,7 +50,7 @@ describe("SelectContextProvider", () => { }) it("SC-02: 異常系: タブIDを取得できない場合はtabIdをnullとして保存する", async () => { - mockGetTabId.mockRejectedValue(new Error("no tab")) + mockGetTabId.mockResolvedValue(null) mockGetSelectionText.mockReturnValue("selected") render({null}) diff --git a/packages/extension/src/service_worker.ts b/packages/extension/src/service_worker.ts index 13083c227..34b5fef96 100644 --- a/packages/extension/src/service_worker.ts +++ b/packages/extension/src/service_worker.ts @@ -704,7 +704,7 @@ chrome.commands.onCommand.addListener(async (commandName) => { } // Get active tab - const [tab] = await chrome.tabs.query({ active: true, currentWindow: true }) + const tab = await getCurrentTab() // Use the selection only if it was recorded in the active tab. Tabs // without the content script (e.g. chrome://) can't record their diff --git a/packages/extension/src/services/__tests__/ipc.test.ts b/packages/extension/src/services/__tests__/ipc.test.ts index 6ab809ea7..61de4a750 100644 --- a/packages/extension/src/services/__tests__/ipc.test.ts +++ b/packages/extension/src/services/__tests__/ipc.test.ts @@ -169,3 +169,34 @@ describe("Ipc.send", () => { expect(vi.getTimerCount()).toBe(0) }) }) + +describe("Ipc.getTabId", () => { + beforeEach(() => { + vi.clearAllMocks() + vi.spyOn(console, "error").mockImplementation(() => {}) + }) + + afterEach(() => { + vi.restoreAllMocks() + }) + + it("IPC-08: 正常系: タブIDを返す", async () => { + vi.mocked(chrome.runtime.sendMessage).mockResolvedValue(10 as never) + + expect(await Ipc.getTabId()).toBe(10) + }) + + it("IPC-09: 正常系: タブに属さない場合（サイドパネル等）は null を返す", async () => { + vi.mocked(chrome.runtime.sendMessage).mockResolvedValue(undefined as never) + + expect(await Ipc.getTabId()).toBeNull() + }) + + it("IPC-10: 異常系: 取得に失敗した場合は例外を投げずに null を返す", async () => { + vi.mocked(chrome.runtime.sendMessage).mockRejectedValue( + new Error("Extension context invalidated") as never, + ) + + expect(await Ipc.getTabId()).toBeNull() + }) +}) diff --git a/packages/extension/src/services/connection.ts b/packages/extension/src/services/connection.ts index 67434ff64..95df0e78a 100644 --- a/packages/extension/src/services/connection.ts +++ b/packages/extension/src/services/connection.ts @@ -20,7 +20,9 @@ Ipc.getTabId().then(async (tabId) => { console.error("Failed to load service worker data:", error) } const serviceWorkerData = ServiceWorkerData.get() - const isConnected = serviceWorkerData?.connectedTabs?.includes(tabId) ?? false + const isConnected = + tabId != null && + (serviceWorkerData?.connectedTabs?.includes(tabId) ?? false) if (!isConnected) { // Setup event listeners for bfcache handling diff --git a/packages/extension/src/services/ipc.ts b/packages/extension/src/services/ipc.ts index bb54f9edf..dc03d270d 100644 --- a/packages/extension/src/services/ipc.ts +++ b/packages/extension/src/services/ipc.ts @@ -566,8 +566,20 @@ export const Ipc = { } }, - async getTabId() { - return Ipc.send(ServiceWorkerCommand.getTabId) + /** + * Get the ID of the tab this script runs in. + * @returns The tab ID, or null if this script doesn't belong to a tab + * (e.g. side panel) or the ID couldn't be retrieved. + */ + async getTabId(): Promise { + try { + return ( + (await Ipc.send(ServiceWorkerCommand.getTabId)) ?? null + ) + } catch { + // Ipc.send has already logged the error. + return null + } }, async getActiveTabId() { From f73394ff893958713626c225c1451aa63aab0698 Mon Sep 17 00:00:00 2001 From: ujiro99 Date: Mon, 19 Oct 2026 12:49:51 +0900 Subject: [PATCH 04/14] chore: remove self-evident comments Co-Authored-By: Claude Opus 5.5 --- packages/extension/src/__tests__/service_worker.test.ts | 3 --- packages/extension/src/action/aiPrompt.ts | 2 -- packages/extension/src/lib/utils.ts | 6 +++--- packages/extension/src/service_worker.ts | 6 +----- .../src/services/pageAction/serviceWorker-sidePanel.ts | 2 -- packages/extension/src/services/pageAction/serviceWorker.ts | 2 -- packages/extension/src/services/storage/const.ts | 3 +-- 7 files changed, 5 insertions(+), 19 deletions(-) diff --git a/packages/extension/src/__tests__/service_worker.test.ts b/packages/extension/src/__tests__/service_worker.test.ts index 90bf5c5d1..84028528b 100644 --- a/packages/extension/src/__tests__/service_worker.test.ts +++ b/packages/extension/src/__tests__/service_worker.test.ts @@ -276,7 +276,6 @@ describe("Service Worker Migration", () => { ) }) - // Sets up the mocks for the onCommand listener and returns it. const setupOnCommand = async ({ storedSelection, tab, @@ -293,7 +292,6 @@ describe("Service Worker Migration", () => { noSelectionBehavior as any mockEnhancedSettings.get.mockResolvedValue(initialSettings as any) - // Mock getCurrentTab to return the active tab vi.doMock("@/services/chrome", () => ({ getCurrentTab: vi.fn().mockResolvedValue(tab), closeWindow: vi.fn(), @@ -358,7 +356,6 @@ describe("Service Worker Migration", () => { } it("MG-01-d: should call enhancedSettings.get() in onCommand function and pass correct command to action/serviceWorker.execute", async () => { - // Setup test data const testShortcutId = "test-shortcut-cmd" const testCommandId = "test-command-123" diff --git a/packages/extension/src/action/aiPrompt.ts b/packages/extension/src/action/aiPrompt.ts index 685cd76a2..0bb8c2d4d 100644 --- a/packages/extension/src/action/aiPrompt.ts +++ b/packages/extension/src/action/aiPrompt.ts @@ -96,8 +96,6 @@ const analyzePromptRequirements = ( service: AiService, selectionFromClipboard: boolean, ): PromptRequirements => { - // {{SelectedText}} is resolved with the clipboard text when the caller - // decided to use it as the selected text (e.g. from a shortcut key). const selectedTextFromClipboard = selectionFromClipboard && aiPromptOption.prompt.includes(toInsertTemplate(INSERT.SELECTED_TEXT)) diff --git a/packages/extension/src/lib/utils.ts b/packages/extension/src/lib/utils.ts index 4d450e53b..7d075720b 100644 --- a/packages/extension/src/lib/utils.ts +++ b/packages/extension/src/lib/utils.ts @@ -53,9 +53,9 @@ export const isUrlParam = (url: string | UrlParam): url is UrlParam => { } /** - * Whether the clipboard must be read when opening the url. - * @param url The url to open. - * @returns true if any part of the url uses the clipboard text. + * Whether the clipboard must be read when opening the url. This is derived + * from how the url uses the clipboard text, rather than kept as a separate + * flag that could disagree with them. */ export const needsClipboard = (url: string | UrlParam): boolean => { if (!isUrlParam(url)) return false diff --git a/packages/extension/src/service_worker.ts b/packages/extension/src/service_worker.ts index 34b5fef96..7a55c75d1 100644 --- a/packages/extension/src/service_worker.ts +++ b/packages/extension/src/service_worker.ts @@ -703,7 +703,6 @@ chrome.commands.onCommand.addListener(async (commandName) => { return } - // Get active tab const tab = await getCurrentTab() // Use the selection only if it was recorded in the active tab. Tabs @@ -717,9 +716,8 @@ chrome.commands.onCommand.addListener(async (commandName) => { ? selection.text : "" - // If no text is selected, handle according to noSelectionBehavior. // This is the only place to decide whether to use the clipboard text as - // the selected text. + // the selected text. Actions trust this decision and don't re-check. let selectionFromClipboard = false if (isEmpty(selectionText)) { if ( @@ -747,8 +745,6 @@ chrome.commands.onCommand.addListener(async (commandName) => { TabCommand.executeAction, { command, - // Pass the selection decided here, so that the selection is - // judged only once. selectionText, selectionFromClipboard, }, diff --git a/packages/extension/src/services/pageAction/serviceWorker-sidePanel.ts b/packages/extension/src/services/pageAction/serviceWorker-sidePanel.ts index 4d34e26a9..0ca88f6e8 100644 --- a/packages/extension/src/services/pageAction/serviceWorker-sidePanel.ts +++ b/packages/extension/src/services/pageAction/serviceWorker-sidePanel.ts @@ -201,8 +201,6 @@ const runPendingSidePanelAction = async ( } catch (e) { console.warn("Failed to read clipboard in background:", e) } - // Use the clipboard text as the selected text if requested - // (e.g. a shortcut key run without a selection). if (pending.selectionFromClipboard) { pending.selectedText = pending.clipboardText } diff --git a/packages/extension/src/services/pageAction/serviceWorker.ts b/packages/extension/src/services/pageAction/serviceWorker.ts index 260c0b514..778abbe58 100644 --- a/packages/extension/src/services/pageAction/serviceWorker.ts +++ b/packages/extension/src/services/pageAction/serviceWorker.ts @@ -412,8 +412,6 @@ export const openAndRun = ( return } - // Use the clipboard text as the selected text if requested - // (e.g. a shortcut key run without a selection). if (isUrlParam(param.url) && param.url.selectionFromClipboard) { selectedText = clipboardText } diff --git a/packages/extension/src/services/storage/const.ts b/packages/extension/src/services/storage/const.ts index bf0bb8f2d..8fd5fe648 100644 --- a/packages/extension/src/services/storage/const.ts +++ b/packages/extension/src/services/storage/const.ts @@ -36,10 +36,9 @@ export enum SESSION_STORAGE_KEY { DEV_TOOLS_ENABLED = "devToolsEnabled", } -// Selection text recorded by the tab where the text was selected. export type SelectionTextData = { text: string - // ID of the tab that recorded the selection. null if unknown. + // null if the selection was cleared or the tab is unknown. tabId: number | null } From 6bb5c0471752b721bc179cfbdde276f88df0dc77 Mon Sep 17 00:00:00 2001 From: ujiro99 Date: Mon, 19 Oct 2026 13:23:55 +0900 Subject: [PATCH 05/14] fix: ask the active tab for its selection on shortcut execution Keeping the selection in session storage raced with the asynchronous writes and the clears on focus change. The shortcut handler now asks the active tab for its current selection, and the tab reports whether the window lost focus after the text was selected, so the clipboard is used instead of a selection that may be older than it. Co-Authored-By: Claude Opus 5.5 --- .../src/__tests__/service_worker.test.ts | 152 ++++++++++-------- .../src/components/menu/InvisibleItem.tsx | 6 +- .../menu/__tests__/InvisibleItem.test.tsx | 1 - .../src/providers/SelectContextProvider.tsx | 45 +++--- .../__tests__/SelectContextProvider.test.tsx | 94 ++++++----- packages/extension/src/service_worker.ts | 66 +++----- .../src/services/__tests__/ipc.test.ts | 31 ---- packages/extension/src/services/connection.ts | 4 +- packages/extension/src/services/ipc.ts | 27 ++-- packages/extension/src/services/storage.ts | 3 +- .../extension/src/services/storage/const.ts | 7 - .../extension/src/services/storage/index.ts | 1 - 12 files changed, 204 insertions(+), 233 deletions(-) diff --git a/packages/extension/src/__tests__/service_worker.test.ts b/packages/extension/src/__tests__/service_worker.test.ts index 84028528b..dd2dae1ea 100644 --- a/packages/extension/src/__tests__/service_worker.test.ts +++ b/packages/extension/src/__tests__/service_worker.test.ts @@ -277,20 +277,23 @@ describe("Service Worker Migration", () => { }) const setupOnCommand = async ({ - storedSelection, tab, + tabSelection, noSelectionBehavior = SHORTCUT_NO_SELECTION_BEHAVIOR.DO_NOTHING, - sendTabResult = new Error("Tab execution failed"), + keepMenuOpenOnFocusChange = false, + executeActionResult = new Error("Tab execution failed"), }: { - storedSelection: { text: string; tabId: number | null } tab: { id: number; url: string; windowId: number } + tabSelection?: unknown noSelectionBehavior?: SHORTCUT_NO_SELECTION_BEHAVIOR - sendTabResult?: unknown + keepMenuOpenOnFocusChange?: boolean + executeActionResult?: unknown }) => { - const initialSettings = createTestSettings() + const initialSettings = createTestSettings() as any initialSettings.shortcuts.shortcuts[0].noSelectionBehavior = - noSelectionBehavior as any - mockEnhancedSettings.get.mockResolvedValue(initialSettings as any) + noSelectionBehavior + initialSettings.startupMethod = { keepMenuOpenOnFocusChange } + mockEnhancedSettings.get.mockResolvedValue(initialSettings) vi.doMock("@/services/chrome", () => ({ getCurrentTab: vi.fn().mockResolvedValue(tab), @@ -303,15 +306,11 @@ describe("Service Worker Migration", () => { .fn() .mockResolvedValue({ left: 1920, top: 0, width: 1600, height: 1000 }) - // Mock Storage.get for selection text - const mockStorageGet = vi.fn().mockResolvedValue(storedSelection) vi.doMock("@/services/storage", () => ({ Storage: { - get: mockStorageGet, - }, - SESSION_STORAGE_KEY: { - SELECTION_TEXT: "selectionText", + get: vi.fn(), }, + SESSION_STORAGE_KEY: {}, LOCAL_STORAGE_KEY: { CLIENT_ID: "clientId", }, @@ -323,9 +322,10 @@ describe("Service Worker Migration", () => { execute: mockExecute, })) - // Mock Ipc.sendTab. By default it simulates tab execution failure - // (to force service worker execution). - const mockIpcSendTab = vi.fn().mockResolvedValue(sendTabResult) + // By default executeAction fails, to force service worker execution. + const mockIpcSendTab = vi.fn(async (_tabId: number, command: string) => + command === "getSelection" ? tabSelection : executeActionResult, + ) vi.doMock("@/services/ipc", () => ({ Ipc: { sendTab: mockIpcSendTab, @@ -337,6 +337,7 @@ describe("Service Worker Migration", () => { }, TabCommand: { executeAction: "executeAction", + getSelection: "getSelection", }, })) @@ -355,13 +356,18 @@ describe("Service Worker Migration", () => { return { commandListener, mockExecute, mockIpcSendTab } } + const runShortcut = async (commandListener: (name: string) => unknown) => { + await commandListener("test-shortcut-cmd") + await new Promise((resolve) => setTimeout(resolve, 10)) + } + it("MG-01-d: should call enhancedSettings.get() in onCommand function and pass correct command to action/serviceWorker.execute", async () => { const testShortcutId = "test-shortcut-cmd" const testCommandId = "test-command-123" const { commandListener, mockExecute } = await setupOnCommand({ - storedSelection: { text: "test selection text", tabId: 1 }, tab: { id: 1, url: "https://example.com", windowId: 1 }, + tabSelection: { text: "test selection text", focusLost: false }, }) // Call the listener with our test shortcut command @@ -396,18 +402,14 @@ describe("Service Worker Migration", () => { ) }) - it("MG-01-e: should ignore a selection recorded in another tab and fall back to the clipboard", async () => { - // The selection was recorded in tab 1, but the shortcut is pressed in a - // chrome:// tab, where the content script can't record its selection. + it("MG-01-e: should not ask a chrome:// tab for its selection and fall back to the clipboard", async () => { const { commandListener, mockExecute, mockIpcSendTab } = await setupOnCommand({ - storedSelection: { text: "stale selection", tabId: 1 }, tab: { id: 2, url: "chrome://settings/", windowId: 1 }, noSelectionBehavior: SHORTCUT_NO_SELECTION_BEHAVIOR.USE_CLIPBOARD, }) - await commandListener("test-shortcut-cmd") - await new Promise((resolve) => setTimeout(resolve, 10)) + await runShortcut(commandListener) expect(mockIpcSendTab).not.toHaveBeenCalled() expect(mockExecute).toHaveBeenCalledWith( @@ -419,51 +421,36 @@ describe("Service Worker Migration", () => { ) }) - it("MG-01-f: should do nothing when the selection was recorded in another tab and noSelectionBehavior is DO_NOTHING", async () => { + it("MG-01-f: should do nothing when the tab has no selection and noSelectionBehavior is DO_NOTHING", async () => { const { commandListener, mockExecute, mockIpcSendTab } = await setupOnCommand({ - storedSelection: { text: "stale selection", tabId: 1 }, - tab: { id: 2, url: "https://example.com", windowId: 1 }, + tab: { id: 1, url: "https://example.com", windowId: 1 }, + tabSelection: { text: "", focusLost: false }, noSelectionBehavior: SHORTCUT_NO_SELECTION_BEHAVIOR.DO_NOTHING, }) - await commandListener("test-shortcut-cmd") - await new Promise((resolve) => setTimeout(resolve, 10)) - - expect(mockIpcSendTab).not.toHaveBeenCalled() - expect(mockExecute).not.toHaveBeenCalled() - }) + await runShortcut(commandListener) - it("MG-01-g: should ignore a selection whose tab is unknown", async () => { - const { commandListener, mockExecute } = await setupOnCommand({ - storedSelection: { text: "unknown tab selection", tabId: null }, - tab: { id: 2, url: "chrome://settings/", windowId: 1 }, - noSelectionBehavior: SHORTCUT_NO_SELECTION_BEHAVIOR.USE_CLIPBOARD, - }) - - await commandListener("test-shortcut-cmd") - await new Promise((resolve) => setTimeout(resolve, 10)) - - expect(mockExecute).toHaveBeenCalledWith( - expect.objectContaining({ - selectionText: "", - selectionFromClipboard: true, - }), + expect(mockIpcSendTab).not.toHaveBeenCalledWith( + 1, + "executeAction", + expect.anything(), ) + expect(mockExecute).not.toHaveBeenCalled() }) - it("MG-01-h: should pass the selection decided by the service worker to the content script", async () => { + it("MG-01-g: should pass the selection of the tab to the content script", async () => { const { commandListener, mockExecute, mockIpcSendTab } = await setupOnCommand({ - storedSelection: { text: "selected", tabId: 1 }, tab: { id: 1, url: "https://example.com", windowId: 1 }, + tabSelection: { text: "selected", focusLost: false }, noSelectionBehavior: SHORTCUT_NO_SELECTION_BEHAVIOR.USE_CLIPBOARD, - sendTabResult: true, + executeActionResult: true, }) - await commandListener("test-shortcut-cmd") - await new Promise((resolve) => setTimeout(resolve, 10)) + await runShortcut(commandListener) + expect(mockIpcSendTab).toHaveBeenCalledWith(1, "getSelection") expect(mockIpcSendTab).toHaveBeenCalledWith(1, "executeAction", { command: expect.objectContaining({ id: "test-command-123" }), selectionText: "selected", @@ -472,18 +459,17 @@ describe("Service Worker Migration", () => { expect(mockExecute).not.toHaveBeenCalled() }) - it("MG-01-i: should tell the content script to use the clipboard when the stored selection was cleared", async () => { - // e.g. the selection was cleared when the window lost focus, while the - // text is still selected in the page. + it("MG-01-h: should use the clipboard when the window lost focus after the text was selected", async () => { + // The text is still selected in the page, but the user may have copied + // other text in another app. const { commandListener, mockIpcSendTab } = await setupOnCommand({ - storedSelection: { text: "", tabId: null }, tab: { id: 1, url: "https://example.com", windowId: 1 }, + tabSelection: { text: "selected", focusLost: true }, noSelectionBehavior: SHORTCUT_NO_SELECTION_BEHAVIOR.USE_CLIPBOARD, - sendTabResult: true, + executeActionResult: true, }) - await commandListener("test-shortcut-cmd") - await new Promise((resolve) => setTimeout(resolve, 10)) + await runShortcut(commandListener) expect(mockIpcSendTab).toHaveBeenCalledWith(1, "executeAction", { command: expect.objectContaining({ id: "test-command-123" }), @@ -491,6 +477,42 @@ describe("Service Worker Migration", () => { selectionFromClipboard: true, }) }) + + it("MG-01-i: should keep the selection after the window lost focus when keepMenuOpenOnFocusChange is on", async () => { + const { commandListener, mockIpcSendTab } = await setupOnCommand({ + tab: { id: 1, url: "https://example.com", windowId: 1 }, + tabSelection: { text: "selected", focusLost: true }, + noSelectionBehavior: SHORTCUT_NO_SELECTION_BEHAVIOR.USE_CLIPBOARD, + keepMenuOpenOnFocusChange: true, + executeActionResult: true, + }) + + await runShortcut(commandListener) + + expect(mockIpcSendTab).toHaveBeenCalledWith(1, "executeAction", { + command: expect.objectContaining({ id: "test-command-123" }), + selectionText: "selected", + selectionFromClipboard: false, + }) + }) + + it("MG-01-j: should treat the selection as empty when the tab doesn't respond", async () => { + // e.g. the content script isn't injected yet. + const { commandListener, mockExecute } = await setupOnCommand({ + tab: { id: 1, url: "https://example.com", windowId: 1 }, + tabSelection: undefined, + noSelectionBehavior: SHORTCUT_NO_SELECTION_BEHAVIOR.USE_CLIPBOARD, + }) + + await runShortcut(commandListener) + + expect(mockExecute).toHaveBeenCalledWith( + expect.objectContaining({ + selectionText: "", + selectionFromClipboard: true, + }), + ) + }) }) describe("Popup Auto-Close Delay", () => { @@ -538,9 +560,7 @@ describe("Popup Auto-Close Delay", () => { Storage: { set: mockStorageSet, }, - SESSION_STORAGE_KEY: { - SELECTION_TEXT: "selectionText", - }, + SESSION_STORAGE_KEY: {}, })) // Clear module cache and re-import @@ -601,9 +621,7 @@ describe("Popup Auto-Close Delay", () => { Storage: { set: mockStorageSet, }, - SESSION_STORAGE_KEY: { - SELECTION_TEXT: "selectionText", - }, + SESSION_STORAGE_KEY: {}, })) // Clear module cache and re-import @@ -667,9 +685,7 @@ describe("Popup Auto-Close Delay", () => { Storage: { set: mockStorageSet, }, - SESSION_STORAGE_KEY: { - SELECTION_TEXT: "selectionText", - }, + SESSION_STORAGE_KEY: {}, })) // Clear module cache and re-import diff --git a/packages/extension/src/components/menu/InvisibleItem.tsx b/packages/extension/src/components/menu/InvisibleItem.tsx index 91ef01649..0a424d407 100644 --- a/packages/extension/src/components/menu/InvisibleItem.tsx +++ b/packages/extension/src/components/menu/InvisibleItem.tsx @@ -66,11 +66,11 @@ export function InvisibleItem(props: InvisibleItemProps): React.ReactNode { executeCommand({ command: action.command as SelectionCommand, position, - // Prefer the selection decided by the sender, since the selection in - // this tab may be stale (e.g. after the window lost focus). + // Prefer the selection decided by the sender, which ignores a selection + // made before the window lost focus. selectionText: action.selectionText ?? selectionText, target, - selectionFromClipboard: action.selectionFromClipboard ?? false, + selectionFromClipboard: action.selectionFromClipboard, }) clearAction() } diff --git a/packages/extension/src/components/menu/__tests__/InvisibleItem.test.tsx b/packages/extension/src/components/menu/__tests__/InvisibleItem.test.tsx index b38f63043..ab6a4b079 100644 --- a/packages/extension/src/components/menu/__tests__/InvisibleItem.test.tsx +++ b/packages/extension/src/components/menu/__tests__/InvisibleItem.test.tsx @@ -69,7 +69,6 @@ describe("InvisibleItem", () => { expect.objectContaining({ command, selectionText: "selection in tab", - selectionFromClipboard: false, }), ) }) diff --git a/packages/extension/src/providers/SelectContextProvider.tsx b/packages/extension/src/providers/SelectContextProvider.tsx index 6ad08319d..b77131220 100644 --- a/packages/extension/src/providers/SelectContextProvider.tsx +++ b/packages/extension/src/providers/SelectContextProvider.tsx @@ -1,10 +1,5 @@ import { useState, ReactNode, useEffect, useCallback, useRef } from "react" -import { - Storage, - SESSION_STORAGE_KEY, - type SelectionTextData, -} from "@/services/storage" -import { Ipc } from "@/services/ipc" +import { Ipc, TabCommand, type TabSelection } from "@/services/ipc" import { getSelectionText } from "@/services/dom" import { ContextType, selectContext } from "@/hooks/useSelectContext" @@ -16,25 +11,17 @@ export const SelectContextProvider = ({ const [selectionText, _setSelectionText] = useState("") const [target, setTarget] = useState(null) const [detectSelectionEnabled, setDetectSelectionEnabled] = useState(true) - const tabIdRef = useRef | null>(null) + const selectionRef = useRef({ text: "", focusLost: false }) - const setSelectionText = useCallback(async (text: string) => { + const setSelectionText = useCallback((text: string) => { _setSelectionText(text) - // Record the tab ID with the text, so that the shortcut handler in the - // service worker can ignore a selection recorded in another tab. - tabIdRef.current ??= Ipc.getTabId() - const tabId = await tabIdRef.current - await Storage.set(SESSION_STORAGE_KEY.SELECTION_TEXT, { - text, - tabId, - }) + selectionRef.current = { text, focusLost: false } }, []) useEffect(() => { - const onSelectionchange = async () => { + const onSelectionchange = () => { if (!detectSelectionEnabled) return - const text = getSelectionText() - await setSelectionText(text) + setSelectionText(getSelectionText()) } document.addEventListener("selectionchange", onSelectionchange) @@ -43,6 +30,26 @@ export const SelectContextProvider = ({ } }, [detectSelectionEnabled, setSelectionText]) + useEffect(() => { + // The selection stays in the page after the window loses focus, so the + // shortcut handler in the service worker needs to know it to decide + // whether to use the selection or the clipboard. + const onBlur = () => { + selectionRef.current = { ...selectionRef.current, focusLost: true } + } + window.addEventListener("blur", onBlur) + + Ipc.addListener(TabCommand.getSelection, (_param, _sender, response) => { + response(selectionRef.current) + return true + }) + + return () => { + window.removeEventListener("blur", onBlur) + Ipc.removeListener(TabCommand.getSelection) + } + }, []) + const value: ContextType = { selectionText, setSelectionText, diff --git a/packages/extension/src/providers/__tests__/SelectContextProvider.test.tsx b/packages/extension/src/providers/__tests__/SelectContextProvider.test.tsx index 799f59a38..1f658a880 100644 --- a/packages/extension/src/providers/__tests__/SelectContextProvider.test.tsx +++ b/packages/extension/src/providers/__tests__/SelectContextProvider.test.tsx @@ -1,22 +1,16 @@ import { describe, it, expect, vi, beforeEach } from "vitest" -import { render, waitFor } from "@testing-library/react" +import { render, act } from "@testing-library/react" import { SelectContextProvider } from "../SelectContextProvider" -import { Storage, SESSION_STORAGE_KEY } from "@/services/storage" -import { Ipc } from "@/services/ipc" +import { Ipc, TabCommand, type IpcCallback } from "@/services/ipc" import { getSelectionText } from "@/services/dom" -vi.mock("@/services/storage", () => ({ - Storage: { - set: vi.fn(), - }, - SESSION_STORAGE_KEY: { - SELECTION_TEXT: "selectionText", - }, -})) - vi.mock("@/services/ipc", () => ({ Ipc: { - getTabId: vi.fn(), + addListener: vi.fn(), + removeListener: vi.fn(), + }, + TabCommand: { + getSelection: "getSelection", }, })) @@ -24,54 +18,70 @@ vi.mock("@/services/dom", () => ({ getSelectionText: vi.fn(), })) -const mockSet = vi.mocked(Storage.set) -const mockGetTabId = vi.mocked(Ipc.getTabId) +const mockAddListener = vi.mocked(Ipc.addListener) const mockGetSelectionText = vi.mocked(getSelectionText) +// Calls the getSelection listener as the service worker does. +const requestSelection = () => { + const call = mockAddListener.mock.calls.find( + ([command]) => command === TabCommand.getSelection, + ) + const listener = call?.[1] as IpcCallback + const response = vi.fn() + listener(undefined, {} as chrome.runtime.MessageSender, response) + return response.mock.calls[0][0] +} + +const selectText = (text: string) => { + mockGetSelectionText.mockReturnValue(text) + act(() => { + document.dispatchEvent(new Event("selectionchange")) + }) +} + describe("SelectContextProvider", () => { beforeEach(() => { vi.clearAllMocks() - mockSet.mockResolvedValue(true) - mockGetTabId.mockResolvedValue(10) }) - it("SC-01: 正常系: 選択変更時に選択テキストとタブIDを保存する", async () => { - mockGetSelectionText.mockReturnValue("selected") + it("SC-01: 正常系: 現在の選択テキストを返す", () => { render({null}) - document.dispatchEvent(new Event("selectionchange")) + selectText("selected") - await waitFor(() => { - expect(mockSet).toHaveBeenCalledWith(SESSION_STORAGE_KEY.SELECTION_TEXT, { - text: "selected", - tabId: 10, - }) - }) + expect(requestSelection()).toEqual({ text: "selected", focusLost: false }) }) - it("SC-02: 異常系: タブIDを取得できない場合はtabIdをnullとして保存する", async () => { - mockGetTabId.mockResolvedValue(null) - mockGetSelectionText.mockReturnValue("selected") + it("SC-02: 正常系: 選択後にウィンドウのフォーカスが外れたことを返す", () => { render({null}) - document.dispatchEvent(new Event("selectionchange")) - - await waitFor(() => { - expect(mockSet).toHaveBeenCalledWith(SESSION_STORAGE_KEY.SELECTION_TEXT, { - text: "selected", - tabId: null, - }) + selectText("selected") + act(() => { + window.dispatchEvent(new Event("blur")) }) + + expect(requestSelection()).toEqual({ text: "selected", focusLost: true }) }) - it("SC-03: 正常系: タブIDの取得は一度だけ行う", async () => { - mockGetSelectionText.mockReturnValue("selected") + it("SC-03: 正常系: フォーカスが外れた後に選択し直すと、フォーカス喪失をリセットする", () => { render({null}) - document.dispatchEvent(new Event("selectionchange")) - document.dispatchEvent(new Event("selectionchange")) + selectText("old") + act(() => { + window.dispatchEvent(new Event("blur")) + }) + selectText("new") + + expect(requestSelection()).toEqual({ text: "new", focusLost: false }) + }) + + it("SC-04: 正常系: アンマウント時にリスナーを解除する", () => { + const { unmount } = render( + {null}, + ) + + unmount() - await waitFor(() => expect(mockSet).toHaveBeenCalledTimes(2)) - expect(mockGetTabId).toHaveBeenCalledTimes(1) + expect(Ipc.removeListener).toHaveBeenCalledWith(TabCommand.getSelection) }) }) diff --git a/packages/extension/src/service_worker.ts b/packages/extension/src/service_worker.ts index 7a55c75d1..d36acb843 100644 --- a/packages/extension/src/service_worker.ts +++ b/packages/extension/src/service_worker.ts @@ -14,7 +14,7 @@ import { TabCommand, CONNECTION_APP, } from "@/services/ipc" -import type { IpcCallback } from "@/services/ipc" +import type { IpcCallback, TabSelection } from "@/services/ipc" import { Settings } from "@/services/settings/settings" import { enhancedSettings } from "@/services/settings/enhancedSettings" import { CACHE_SECTIONS } from "@/services/settings/settingsCache" @@ -28,11 +28,7 @@ import { findMatchingPageRule, isEmpty } from "@/lib/utils" import { execute } from "@/action/serviceWorker" import * as ActionHelper from "@/action/helper" import type { WindowType } from "@/types" -import { - Storage, - SESSION_STORAGE_KEY, - type SelectionTextData, -} from "@/services/storage" +import { Storage } from "@/services/storage" import { ANALYTICS_EVENTS, sendEvent, @@ -394,20 +390,6 @@ const updateActiveTabId = async (activeTabId?: number) => { } } -// Clears the selection text unless the user opted to keep the menu open -// across tab/window changes. -const clearSelectionTextUnlessKeepOpen = async () => { - const settings = await enhancedSettings.getSection( - CACHE_SECTIONS.USER_SETTINGS, - ) - if (!settings.startupMethod?.keepMenuOpenOnFocusChange) { - await Storage.set(SESSION_STORAGE_KEY.SELECTION_TEXT, { - text: "", - tabId: null, - }) - } -} - chrome.action.onClicked.addListener(() => { chrome.tabs.create({ url: OPTION_PAGE_PATH, @@ -415,10 +397,7 @@ chrome.action.onClicked.addListener(() => { }) chrome.windows.onFocusChanged.addListener(async (windowId: number) => { - if (windowId === chrome.windows.WINDOW_ID_NONE) { - await clearSelectionTextUnlessKeepOpen() - return - } + if (windowId === chrome.windows.WINDOW_ID_NONE) return // Update active tab ID await updateActiveTabId() @@ -427,11 +406,7 @@ chrome.windows.onFocusChanged.addListener(async (windowId: number) => { const windowsToClose = await WindowStackManager.getWindowsToClose(windowId) // Schedule popup windows to close with configured delay. - // Run this before the settings fetch below so popup auto-close isn't - // delayed behind it and doesn't race with overlapping focus-change events. await PopupAutoClose.scheduleClose(windowsToClose) - - await clearSelectionTextUnlessKeepOpen() }) chrome.windows.onRemoved.addListener(async (windowId: number) => { @@ -704,17 +679,28 @@ chrome.commands.onCommand.addListener(async (commandName) => { } const tab = await getCurrentTab() + const enableSendTab = + tab?.id && + !tab.url?.startsWith("chrome") && + !tab.url?.includes("chromewebstore.google.com") - // Use the selection only if it was recorded in the active tab. Tabs - // without the content script (e.g. chrome://) can't record their - // selection, so the stored one may belong to another tab. - const selection = await Storage.get( - SESSION_STORAGE_KEY.SELECTION_TEXT, - ) - const selectionText = - selection?.tabId != null && selection.tabId === tab?.id - ? selection.text - : "" + // Ask the active tab for its latest selection. Tabs without the content + // script (e.g. chrome://) are treated as having no selection. + let selectionText = "" + if (enableSendTab) { + const selection = await Ipc.sendTab< + unknown, + TabSelection | Error | undefined + >(tab.id ?? 0, TabCommand.getSelection) + if (selection != null && !(selection instanceof Error)) { + // A selection made before the window lost focus may be older than + // the clipboard, as the user may have copied other text elsewhere. + const stale = + selection.focusLost && + !settings.startupMethod?.keepMenuOpenOnFocusChange + if (!stale) selectionText = selection.text + } + } // This is the only place to decide whether to use the clipboard text as // the selected text. Actions trust this decision and don't re-check. @@ -732,10 +718,6 @@ chrome.commands.onCommand.addListener(async (commandName) => { selectionFromClipboard = true } } - const enableSendTab = - tab?.id && - !tab.url?.startsWith("chrome") && - !tab.url?.includes("chromewebstore.google.com") let ret: unknown if (enableSendTab) { diff --git a/packages/extension/src/services/__tests__/ipc.test.ts b/packages/extension/src/services/__tests__/ipc.test.ts index 61de4a750..6ab809ea7 100644 --- a/packages/extension/src/services/__tests__/ipc.test.ts +++ b/packages/extension/src/services/__tests__/ipc.test.ts @@ -169,34 +169,3 @@ describe("Ipc.send", () => { expect(vi.getTimerCount()).toBe(0) }) }) - -describe("Ipc.getTabId", () => { - beforeEach(() => { - vi.clearAllMocks() - vi.spyOn(console, "error").mockImplementation(() => {}) - }) - - afterEach(() => { - vi.restoreAllMocks() - }) - - it("IPC-08: 正常系: タブIDを返す", async () => { - vi.mocked(chrome.runtime.sendMessage).mockResolvedValue(10 as never) - - expect(await Ipc.getTabId()).toBe(10) - }) - - it("IPC-09: 正常系: タブに属さない場合（サイドパネル等）は null を返す", async () => { - vi.mocked(chrome.runtime.sendMessage).mockResolvedValue(undefined as never) - - expect(await Ipc.getTabId()).toBeNull() - }) - - it("IPC-10: 異常系: 取得に失敗した場合は例外を投げずに null を返す", async () => { - vi.mocked(chrome.runtime.sendMessage).mockRejectedValue( - new Error("Extension context invalidated") as never, - ) - - expect(await Ipc.getTabId()).toBeNull() - }) -}) diff --git a/packages/extension/src/services/connection.ts b/packages/extension/src/services/connection.ts index 95df0e78a..67434ff64 100644 --- a/packages/extension/src/services/connection.ts +++ b/packages/extension/src/services/connection.ts @@ -20,9 +20,7 @@ Ipc.getTabId().then(async (tabId) => { console.error("Failed to load service worker data:", error) } const serviceWorkerData = ServiceWorkerData.get() - const isConnected = - tabId != null && - (serviceWorkerData?.connectedTabs?.includes(tabId) ?? false) + const isConnected = serviceWorkerData?.connectedTabs?.includes(tabId) ?? false if (!isConnected) { // Setup event listeners for bfcache handling diff --git a/packages/extension/src/services/ipc.ts b/packages/extension/src/services/ipc.ts index dc03d270d..1b815da1e 100644 --- a/packages/extension/src/services/ipc.ts +++ b/packages/extension/src/services/ipc.ts @@ -87,6 +87,7 @@ export enum TabCommand { ping = "ping", connected = "connected", executeAction = "executeAction", + getSelection = "getSelection", clickElement = "clickElement", closeMenu = "closeMenu", showReviewRequest = "showReviewRequest", @@ -104,6 +105,16 @@ export type ConfirmCommandUpdateProps = { title: string } +/** The selection in a tab, returned for TabCommand.getSelection. */ +export type TabSelection = { + text: string + /** + * The window lost focus after the text was selected. The user may have + * copied other text elsewhere in the meantime. + */ + focusLost: boolean +} + export type ClickElementProps = { selector: string } @@ -566,20 +577,8 @@ export const Ipc = { } }, - /** - * Get the ID of the tab this script runs in. - * @returns The tab ID, or null if this script doesn't belong to a tab - * (e.g. side panel) or the ID couldn't be retrieved. - */ - async getTabId(): Promise { - try { - return ( - (await Ipc.send(ServiceWorkerCommand.getTabId)) ?? null - ) - } catch { - // Ipc.send has already logged the error. - return null - } + async getTabId() { + return Ipc.send(ServiceWorkerCommand.getTabId) }, async getActiveTabId() { diff --git a/packages/extension/src/services/storage.ts b/packages/extension/src/services/storage.ts index 98ecefdd9..39a962e48 100644 --- a/packages/extension/src/services/storage.ts +++ b/packages/extension/src/services/storage.ts @@ -5,7 +5,6 @@ import { SESSION_STORAGE_KEY, CMD_PREFIX, KEY, - SelectionTextData, } from "./storage/const" import { BaseStorage, ChangedCallback } from "./storage/index" import { @@ -28,7 +27,7 @@ export { WeeklyBackupManager, } -export type { ChangedCallback, KEY, SelectionTextData } +export type { ChangedCallback, KEY } export const Storage = { // Base storage methods diff --git a/packages/extension/src/services/storage/const.ts b/packages/extension/src/services/storage/const.ts index 8fd5fe648..525378be5 100644 --- a/packages/extension/src/services/storage/const.ts +++ b/packages/extension/src/services/storage/const.ts @@ -24,7 +24,6 @@ export enum LOCAL_STORAGE_KEY { export enum SESSION_STORAGE_KEY { SERVICE_WORKER_DATA = "serviceWorkerData", - SELECTION_TEXT = "selectionText ", SESSION_DATA = "sessionData", MESSAGE_QUEUE = "messageQueue", TMP_CAPTURES = "tmpCaptures", @@ -36,12 +35,6 @@ export enum SESSION_STORAGE_KEY { DEV_TOOLS_ENABLED = "devToolsEnabled", } -export type SelectionTextData = { - text: string - // null if the selection was cleared or the tab is unknown. - tabId: number | null -} - export type KEY = | STORAGE_KEY | LOCAL_STORAGE_KEY diff --git a/packages/extension/src/services/storage/index.ts b/packages/extension/src/services/storage/index.ts index 9621c0281..46b0a2cf5 100644 --- a/packages/extension/src/services/storage/index.ts +++ b/packages/extension/src/services/storage/index.ts @@ -105,7 +105,6 @@ const DEFAULTS = { [SESSION_STORAGE_KEY.PA_CONTEXT]: {}, [SESSION_STORAGE_KEY.PA_RECORDER_OPTION]: {}, [SESSION_STORAGE_KEY.TMP_CAPTURES]: {}, - [SESSION_STORAGE_KEY.SELECTION_TEXT]: { text: "", tabId: null }, [SESSION_STORAGE_KEY.PA_SIDE_PANEL_PENDING]: null, [SESSION_STORAGE_KEY.DEV_TOOLS_ENABLED]: false, } as const From 2f374db026398420d1d864532db6c5aa023aeb55 Mon Sep 17 00:00:00 2001 From: ujiro99 Date: Mon, 19 Oct 2026 13:23:55 +0900 Subject: [PATCH 06/14] refactor: require selectionFromClipboard in the parameters of actions The executor fills in the default, so actions no longer repeat it. Co-Authored-By: Claude Opus 5.5 --- .../src/action/__tests__/aiPrompt.test.ts | 21 +++++++++++++++++++ .../src/action/__tests__/pageAction.test.ts | 8 +++++++ packages/extension/src/action/aiPrompt.ts | 6 +++--- .../extension/src/action/backgroundTab.ts | 6 +++--- packages/extension/src/action/pageAction.ts | 8 +++---- packages/extension/src/action/popup.ts | 6 +++--- packages/extension/src/action/sidePanel.ts | 6 +++--- packages/extension/src/action/tab.ts | 6 +++--- packages/extension/src/action/window.ts | 6 +++--- packages/extension/src/types/index.ts | 5 +++++ 10 files changed, 56 insertions(+), 22 deletions(-) diff --git a/packages/extension/src/action/__tests__/aiPrompt.test.ts b/packages/extension/src/action/__tests__/aiPrompt.test.ts index 473d1a78f..9afb07b08 100644 --- a/packages/extension/src/action/__tests__/aiPrompt.test.ts +++ b/packages/extension/src/action/__tests__/aiPrompt.test.ts @@ -122,6 +122,7 @@ describe("AiPrompt.execute", () => { await AiPrompt.execute({ selectionText: "hello world", + selectionFromClipboard: false, command: { ...baseCommand, aiPromptOption: { @@ -152,6 +153,7 @@ describe("AiPrompt.execute", () => { await AiPrompt.execute({ selectionText: "hello world", + selectionFromClipboard: false, command: { ...baseCommand, aiPromptOption: { @@ -174,6 +176,7 @@ describe("AiPrompt.execute", () => { await AiPrompt.execute({ selectionText: "hello world", + selectionFromClipboard: false, command: { ...baseCommand, aiPromptOption: { @@ -202,6 +205,7 @@ describe("AiPrompt.execute", () => { await AiPrompt.execute({ selectionText: "hello world", + selectionFromClipboard: false, command: { ...baseCommand, aiPromptOption: { @@ -235,6 +239,7 @@ describe("AiPrompt.execute", () => { await AiPrompt.execute({ selectionText: "hello world", + selectionFromClipboard: false, command: { ...baseCommand, aiPromptOption: { @@ -290,6 +295,7 @@ describe("AiPrompt.execute", () => { try { await AiPrompt.execute({ selectionText: "hello world", + selectionFromClipboard: false, command: { ...baseCommand, aiPromptOption: { @@ -321,6 +327,7 @@ describe("AiPrompt.execute", () => { await AiPrompt.execute({ selectionText: "hello world", + selectionFromClipboard: false, command: { ...baseCommand, aiPromptOption: { @@ -356,6 +363,7 @@ describe("AiPrompt.execute", () => { await AiPrompt.execute({ selectionText: "hello", + selectionFromClipboard: false, command: { ...baseCommand, aiPromptOption: { @@ -383,6 +391,7 @@ describe("AiPrompt.execute", () => { await AiPrompt.execute({ selectionText: "hello", + selectionFromClipboard: false, command: baseCommand as any, position: { x: 0, y: 0 }, }) @@ -397,6 +406,7 @@ describe("AiPrompt.execute", () => { await AiPrompt.execute({ selectionText: "hello", + selectionFromClipboard: false, command: baseCommand as any, position: { x: 0, y: 0 }, }) @@ -411,6 +421,7 @@ describe("AiPrompt.execute", () => { await AiPrompt.execute({ selectionText: "hello", + selectionFromClipboard: false, command: { ...baseCommand, aiPromptOption: { @@ -447,6 +458,7 @@ describe("AiPrompt.execute", () => { await AiPrompt.execute({ selectionText: "hello", + selectionFromClipboard: false, command: { ...baseCommand, aiPromptOption: { @@ -471,6 +483,7 @@ describe("AiPrompt.execute", () => { await AiPrompt.execute({ selectionText: "hello", + selectionFromClipboard: false, command: baseCommand as any, position: { x: 0, y: 0 }, }) @@ -484,6 +497,7 @@ describe("AiPrompt.execute", () => { await AiPrompt.execute({ selectionText: "hello world", + selectionFromClipboard: false, command: baseCommand as any, // prompt: "Explain: {{SelectedText}}" position: { x: 0, y: 0 }, }) @@ -497,6 +511,7 @@ describe("AiPrompt.execute", () => { await AiPrompt.execute({ selectionText: "hello", + selectionFromClipboard: false, command: { ...baseCommand, aiPromptOption: { @@ -627,6 +642,7 @@ describe("AiPrompt.execute", () => { await AiPrompt.execute({ selectionText: "hello", + selectionFromClipboard: false, command: { ...baseCommand, aiPromptOption: { @@ -648,6 +664,7 @@ describe("AiPrompt.execute", () => { await AiPrompt.execute({ selectionText: "hello", + selectionFromClipboard: false, command: { ...baseCommand, aiPromptOption: { @@ -668,6 +685,7 @@ describe("AiPrompt.execute", () => { await AiPrompt.execute({ selectionText: "hello", + selectionFromClipboard: false, command: { ...baseCommand, aiPromptOption: { @@ -696,6 +714,7 @@ describe("AiPrompt.execute", () => { await AiPrompt.execute({ selectionText: "https://example.com/page", + selectionFromClipboard: false, command: { ...baseCommand, aiPromptOption: { @@ -718,6 +737,7 @@ describe("AiPrompt.execute", () => { await AiPrompt.execute({ selectionText: "hello", + selectionFromClipboard: false, command: { ...baseCommand, aiPromptOption: { @@ -743,6 +763,7 @@ describe("AiPrompt.execute", () => { await AiPrompt.execute({ selectionText: "https://example.com/page", + selectionFromClipboard: false, command: { ...baseCommand, aiPromptOption: { diff --git a/packages/extension/src/action/__tests__/pageAction.test.ts b/packages/extension/src/action/__tests__/pageAction.test.ts index d2bac16f4..45b1cc21c 100644 --- a/packages/extension/src/action/__tests__/pageAction.test.ts +++ b/packages/extension/src/action/__tests__/pageAction.test.ts @@ -59,6 +59,7 @@ describe("PageAction.execute", () => { await PageAction.execute({ command: baseCommand as any, selectionText: "Selected content", + selectionFromClipboard: false, position: { x: 50, y: 50 }, }) @@ -93,6 +94,7 @@ describe("PageAction.execute", () => { await PageAction.execute({ command: commandWithClipboardVariable as any, selectionText: "", + selectionFromClipboard: false, position: { x: 50, y: 50 }, }) @@ -131,6 +133,7 @@ describe("PageAction.execute", () => { await PageAction.execute({ command: commandWithoutVariableStep as any, selectionText: "hello", + selectionFromClipboard: false, position: { x: 50, y: 50 }, }) @@ -170,6 +173,7 @@ describe("PageAction.execute", () => { await PageAction.execute({ command: legacyCommand as any, selectionText: "sample", + selectionFromClipboard: false, position: { x: 10, y: 10 }, }) @@ -199,6 +203,7 @@ describe("PageAction.execute", () => { await PageAction.execute({ command: sidePanelCommand as any, selectionText: "side panel content", + selectionFromClipboard: false, position: { x: 50, y: 50 }, pageUrl: "https://origin.example.com", }) @@ -239,6 +244,7 @@ describe("PageAction.execute", () => { await PageAction.execute({ command: sidePanelCommand as any, selectionText: "context menu invocation", + selectionFromClipboard: false, position: null, }) @@ -268,6 +274,7 @@ describe("PageAction.execute", () => { await PageAction.execute({ command: sidePanelCommand as any, selectionText: "", + selectionFromClipboard: false, position: { x: 50, y: 50 }, }) @@ -305,6 +312,7 @@ describe("PageAction.execute", () => { await PageAction.execute({ command: sidePanelLegacyCommand as any, selectionText: "legacy text", + selectionFromClipboard: false, position: null, }) diff --git a/packages/extension/src/action/aiPrompt.ts b/packages/extension/src/action/aiPrompt.ts index 0bb8c2d4d..ef3b0d43c 100644 --- a/packages/extension/src/action/aiPrompt.ts +++ b/packages/extension/src/action/aiPrompt.ts @@ -23,7 +23,7 @@ import { } from "@/const" import { resolvePopupSize } from "@/services/option/defaultSettings" import type { - ExecuteCommandParams, + ActionExecuteParams, PageActionStep, UrlParam, AiService, @@ -472,7 +472,7 @@ export const AiPrompt = { useSecondary, selectionFromClipboard, pageUrl, - }: ExecuteCommandParams) { + }: ActionExecuteParams) { if (!isAiPromptType(command)) { console.error("command is not for AiPrompt.") return @@ -504,7 +504,7 @@ export const AiPrompt = { } = analyzePromptRequirements( aiPromptOption, service, - selectionFromClipboard ?? false, + selectionFromClipboard, ) const { steps, urlParam, serviceUrl } = useQueryUrl diff --git a/packages/extension/src/action/backgroundTab.ts b/packages/extension/src/action/backgroundTab.ts index 85344c72e..c02e1a2d3 100644 --- a/packages/extension/src/action/backgroundTab.ts +++ b/packages/extension/src/action/backgroundTab.ts @@ -1,7 +1,7 @@ import { Ipc, ServiceWorkerCommand } from "@/services/ipc" import { isValidString } from "@/lib/utils" import { SPACE_ENCODING } from "@/const" -import type { ExecuteCommandParams } from "@/types" +import type { ActionExecuteParams } from "@/types" import type { OpenTabProps } from "@/services/chrome" export const BackgroundTab = { @@ -10,7 +10,7 @@ export const BackgroundTab = { command, selectionFromClipboard, pageUrl, - }: ExecuteCommandParams) { + }: ActionExecuteParams) { if (!isValidString(command.searchUrl)) { console.error("searchUrl is not valid.") return @@ -21,7 +21,7 @@ export const BackgroundTab = { searchUrl: command.searchUrl, spaceEncoding: command.spaceEncoding ?? SPACE_ENCODING.PLUS, selectionText, - selectionFromClipboard: selectionFromClipboard ?? false, + selectionFromClipboard, pageUrl: pageUrl ?? "", }, active: false, diff --git a/packages/extension/src/action/pageAction.ts b/packages/extension/src/action/pageAction.ts index 0e56d9321..fce40cf4b 100644 --- a/packages/extension/src/action/pageAction.ts +++ b/packages/extension/src/action/pageAction.ts @@ -9,7 +9,7 @@ import { getWindowPosition } from "@/services/screen" import { isValidString, isPageActionCommand } from "@/lib/utils" import { PAGE_ACTION_OPEN_MODE, PAGE_ACTION_EVENT } from "@/const" import { resolvePopupSize } from "@/services/option/defaultSettings" -import type { ExecuteCommandParams, UrlParam } from "@/types" +import type { ActionExecuteParams, UrlParam } from "@/types" import type { OpenAndRunProps } from "@/services/pageAction/serviceWorker" import { INSERT, templateReferencesInsert } from "@/services/pageAction" @@ -26,7 +26,7 @@ export const PageAction = { selectionFromClipboard, userVariables, pageUrl, - }: ExecuteCommandParams & PageActionParams) { + }: ActionExecuteParams & PageActionParams) { if (!isPageActionCommand(command)) { console.error("command is not for PageAction.") return @@ -63,7 +63,7 @@ export const PageAction = { selectedText: selectionText, srcUrl: pageUrl ?? "", clipboardText: "", - selectionFromClipboard: selectionFromClipboard ?? false, + selectionFromClipboard, clipboardForSteps, userVariables: effectiveUserVariables, } @@ -90,7 +90,7 @@ export const PageAction = { const url: UrlParam = { searchUrl: command.pageActionOption.startUrl, selectionText, - selectionFromClipboard: selectionFromClipboard ?? false, + selectionFromClipboard, clipboardForSteps, pageUrl: pageUrl ?? "", } diff --git a/packages/extension/src/action/popup.ts b/packages/extension/src/action/popup.ts index 8adc1f49f..3f2f79352 100644 --- a/packages/extension/src/action/popup.ts +++ b/packages/extension/src/action/popup.ts @@ -3,7 +3,7 @@ import { isValidString } from "@/lib/utils" import { getWindowPosition } from "@/services/screen" import { POPUP_TYPE, SPACE_ENCODING } from "@/const" import { resolvePopupSize } from "@/services/option/defaultSettings" -import type { ExecuteCommandParams } from "@/types" +import type { ActionExecuteParams } from "@/types" import type { OpenPopupProps } from "@/services/chrome" export const Popup = { @@ -13,7 +13,7 @@ export const Popup = { position, selectionFromClipboard, pageUrl, - }: ExecuteCommandParams) { + }: ActionExecuteParams) { if (!isValidString(command.searchUrl)) { console.error("searchUrl is not valid.") return @@ -31,7 +31,7 @@ export const Popup = { searchUrl: command.searchUrl, spaceEncoding: command.spaceEncoding ?? SPACE_ENCODING.PLUS, selectionText, - selectionFromClipboard: selectionFromClipboard ?? false, + selectionFromClipboard, pageUrl: pageUrl ?? "", }, top: Math.floor(windowPosition.top + position.y), diff --git a/packages/extension/src/action/sidePanel.ts b/packages/extension/src/action/sidePanel.ts index a5af2b098..c76337e7b 100644 --- a/packages/extension/src/action/sidePanel.ts +++ b/packages/extension/src/action/sidePanel.ts @@ -1,6 +1,6 @@ import { isValidString, toUrl } from "@/lib/utils" import { SPACE_ENCODING } from "@/const" -import type { ExecuteCommandParams, ShowToastParam } from "@/types" +import type { ActionExecuteParams, ShowToastParam } from "@/types" import type { OpenSidePanelProps } from "@/services/chrome" import { Ipc, ServiceWorkerCommand, TabCommand } from "@/services/ipc" import { t } from "@/services/i18n" @@ -11,7 +11,7 @@ export const SidePanel = { command, selectionFromClipboard, pageUrl, - }: ExecuteCommandParams) { + }: ActionExecuteParams) { if (!isValidString(command.searchUrl)) { console.error("searchUrl is not valid.") return @@ -39,7 +39,7 @@ export const SidePanel = { searchUrl: command.searchUrl, spaceEncoding: command.spaceEncoding ?? SPACE_ENCODING.PLUS, selectionText, - selectionFromClipboard: selectionFromClipboard ?? false, + selectionFromClipboard, pageUrl: pageUrl ?? "", }, clipboardText, diff --git a/packages/extension/src/action/tab.ts b/packages/extension/src/action/tab.ts index d74a7f0fb..16aae56dd 100644 --- a/packages/extension/src/action/tab.ts +++ b/packages/extension/src/action/tab.ts @@ -1,7 +1,7 @@ import { Ipc, ServiceWorkerCommand } from "@/services/ipc" import { isValidString } from "@/lib/utils" import { SPACE_ENCODING } from "@/const" -import type { ExecuteCommandParams } from "@/types" +import type { ActionExecuteParams } from "@/types" import type { OpenTabProps } from "@/services/chrome" export const Tab = { @@ -10,7 +10,7 @@ export const Tab = { command, selectionFromClipboard, pageUrl, - }: ExecuteCommandParams) { + }: ActionExecuteParams) { if (!isValidString(command.searchUrl)) { console.error("searchUrl is not valid.") return @@ -21,7 +21,7 @@ export const Tab = { searchUrl: command.searchUrl, spaceEncoding: command.spaceEncoding ?? SPACE_ENCODING.PLUS, selectionText, - selectionFromClipboard: selectionFromClipboard ?? false, + selectionFromClipboard, pageUrl: pageUrl ?? "", }, active: true, diff --git a/packages/extension/src/action/window.ts b/packages/extension/src/action/window.ts index 30eb3f47c..01fe453e2 100644 --- a/packages/extension/src/action/window.ts +++ b/packages/extension/src/action/window.ts @@ -4,7 +4,7 @@ import { getWindowPosition } from "@/services/screen" import { POPUP_TYPE, SPACE_ENCODING } from "@/const" import { resolvePopupSize } from "@/services/option/defaultSettings" import type { OpenPopupProps } from "@/services/chrome" -import type { ExecuteCommandParams } from "@/types" +import type { ActionExecuteParams } from "@/types" export const Window = { async execute({ @@ -13,7 +13,7 @@ export const Window = { position, selectionFromClipboard, pageUrl, - }: ExecuteCommandParams) { + }: ActionExecuteParams) { if (!isValidString(command.searchUrl)) { console.error("searchUrl is not valid.") return @@ -31,7 +31,7 @@ export const Window = { searchUrl: command.searchUrl, spaceEncoding: command.spaceEncoding ?? SPACE_ENCODING.PLUS, selectionText, - selectionFromClipboard: selectionFromClipboard ?? false, + selectionFromClipboard, pageUrl: pageUrl ?? "", }, top: Math.floor(windowPosition.top + position.y), diff --git a/packages/extension/src/types/index.ts b/packages/extension/src/types/index.ts index 9b8abfaa5..a0f0f3b8a 100644 --- a/packages/extension/src/types/index.ts +++ b/packages/extension/src/types/index.ts @@ -259,6 +259,11 @@ export type ExecuteCommandParams = { pageUrl?: string } +/** Parameters each action receives. The executor fills in the defaults. */ +export type ActionExecuteParams = ExecuteCommandParams & { + selectionFromClipboard: boolean +} + export type ActionTypes = PAGE_ACTION_EVENT | PAGE_ACTION_CONTROL export type PageActionStep = { id: string From 0b395840c8a5b5252e46272c7b6013333c4963be Mon Sep 17 00:00:00 2001 From: ujiro99 Date: Mon, 19 Oct 2026 17:34:19 +0900 Subject: [PATCH 07/14] fix: ignore the selection when a shortcut is run while another app has focus A global shortcut can be pressed while another app has focus, and the active tab of the last focused window still has the selection made before the user switched apps. Co-Authored-By: Claude Opus 5.5 --- .../src/__tests__/service_worker.test.ts | 27 +++++++++++++++++++ packages/extension/src/service_worker.ts | 9 ++++++- 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/packages/extension/src/__tests__/service_worker.test.ts b/packages/extension/src/__tests__/service_worker.test.ts index dd2dae1ea..aa2a7d5c9 100644 --- a/packages/extension/src/__tests__/service_worker.test.ts +++ b/packages/extension/src/__tests__/service_worker.test.ts @@ -281,12 +281,14 @@ describe("Service Worker Migration", () => { tabSelection, noSelectionBehavior = SHORTCUT_NO_SELECTION_BEHAVIOR.DO_NOTHING, keepMenuOpenOnFocusChange = false, + chromeFocused = true, executeActionResult = new Error("Tab execution failed"), }: { tab: { id: number; url: string; windowId: number } tabSelection?: unknown noSelectionBehavior?: SHORTCUT_NO_SELECTION_BEHAVIOR keepMenuOpenOnFocusChange?: boolean + chromeFocused?: boolean executeActionResult?: unknown }) => { const initialSettings = createTestSettings() as any @@ -301,6 +303,10 @@ describe("Service Worker Migration", () => { windowExists: vi.fn(), })) + ;(chrome.windows as any).getLastFocused = vi + .fn() + .mockResolvedValue({ id: tab.windowId, focused: chromeFocused }) + // Mock the current window used to center the popup ;(chrome.windows as any).getCurrent = vi .fn() @@ -513,6 +519,27 @@ describe("Service Worker Migration", () => { }), ) }) + + it("MG-01-k: should treat the selection as empty when another app has focus", async () => { + // e.g. a global shortcut pressed right after copying text in another app. + const { commandListener, mockIpcSendTab } = await setupOnCommand({ + tab: { id: 1, url: "https://example.com", windowId: 1 }, + tabSelection: { text: "selected", focusLost: false }, + noSelectionBehavior: SHORTCUT_NO_SELECTION_BEHAVIOR.USE_CLIPBOARD, + keepMenuOpenOnFocusChange: true, + chromeFocused: false, + executeActionResult: true, + }) + + await runShortcut(commandListener) + + expect(mockIpcSendTab).not.toHaveBeenCalledWith(1, "getSelection") + expect(mockIpcSendTab).toHaveBeenCalledWith(1, "executeAction", { + command: expect.objectContaining({ id: "test-command-123" }), + selectionText: "", + selectionFromClipboard: true, + }) + }) }) describe("Popup Auto-Close Delay", () => { diff --git a/packages/extension/src/service_worker.ts b/packages/extension/src/service_worker.ts index d36acb843..6740da0ec 100644 --- a/packages/extension/src/service_worker.ts +++ b/packages/extension/src/service_worker.ts @@ -684,10 +684,17 @@ chrome.commands.onCommand.addListener(async (commandName) => { !tab.url?.startsWith("chrome") && !tab.url?.includes("chromewebstore.google.com") + // A global shortcut can be pressed while another app has focus. Then the + // selection left in the page is ignored, as the user may have copied + // other text in that app. getLastFocused fails when no window is open. + const lastFocusedWindow = await chrome.windows + .getLastFocused() + .catch(() => undefined) + // Ask the active tab for its latest selection. Tabs without the content // script (e.g. chrome://) are treated as having no selection. let selectionText = "" - if (enableSendTab) { + if (enableSendTab && lastFocusedWindow?.focused) { const selection = await Ipc.sendTab< unknown, TabSelection | Error | undefined From 53d7be3f7d32153ef6ef85a7628736d10df7b49d Mon Sep 17 00:00:00 2001 From: ujiro99 Date: Mon, 19 Oct 2026 17:46:11 +0900 Subject: [PATCH 08/14] fix: ignore the selection after focus loss regardless of keepMenuOpenOnFocusChange The setting only keeps the menu open across focus changes. Keeping the selection for the shortcut let a selection older than the clipboard be used. Co-Authored-By: Claude Opus 5.5 --- .../src/__tests__/service_worker.test.ts | 8 ++++---- packages/extension/src/service_worker.ts | 15 ++++++++------- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/packages/extension/src/__tests__/service_worker.test.ts b/packages/extension/src/__tests__/service_worker.test.ts index aa2a7d5c9..6408f04ba 100644 --- a/packages/extension/src/__tests__/service_worker.test.ts +++ b/packages/extension/src/__tests__/service_worker.test.ts @@ -484,7 +484,8 @@ describe("Service Worker Migration", () => { }) }) - it("MG-01-i: should keep the selection after the window lost focus when keepMenuOpenOnFocusChange is on", async () => { + it("MG-01-i: should ignore the selection after the window lost focus even when keepMenuOpenOnFocusChange is on", async () => { + // The setting only keeps the menu open. It doesn't keep the selection. const { commandListener, mockIpcSendTab } = await setupOnCommand({ tab: { id: 1, url: "https://example.com", windowId: 1 }, tabSelection: { text: "selected", focusLost: true }, @@ -497,8 +498,8 @@ describe("Service Worker Migration", () => { expect(mockIpcSendTab).toHaveBeenCalledWith(1, "executeAction", { command: expect.objectContaining({ id: "test-command-123" }), - selectionText: "selected", - selectionFromClipboard: false, + selectionText: "", + selectionFromClipboard: true, }) }) @@ -526,7 +527,6 @@ describe("Service Worker Migration", () => { tab: { id: 1, url: "https://example.com", windowId: 1 }, tabSelection: { text: "selected", focusLost: false }, noSelectionBehavior: SHORTCUT_NO_SELECTION_BEHAVIOR.USE_CLIPBOARD, - keepMenuOpenOnFocusChange: true, chromeFocused: false, executeActionResult: true, }) diff --git a/packages/extension/src/service_worker.ts b/packages/extension/src/service_worker.ts index 6740da0ec..d25021a04 100644 --- a/packages/extension/src/service_worker.ts +++ b/packages/extension/src/service_worker.ts @@ -699,13 +699,14 @@ chrome.commands.onCommand.addListener(async (commandName) => { unknown, TabSelection | Error | undefined >(tab.id ?? 0, TabCommand.getSelection) - if (selection != null && !(selection instanceof Error)) { - // A selection made before the window lost focus may be older than - // the clipboard, as the user may have copied other text elsewhere. - const stale = - selection.focusLost && - !settings.startupMethod?.keepMenuOpenOnFocusChange - if (!stale) selectionText = selection.text + // A selection made before the window lost focus may be older than the + // clipboard, as the user may have copied other text elsewhere. + if ( + selection != null && + !(selection instanceof Error) && + !selection.focusLost + ) { + selectionText = selection.text } } From 414cd958eb3c2d56a92df37846194fb088bda740 Mon Sep 17 00:00:00 2001 From: ujiro99 Date: Mon, 19 Oct 2026 18:18:17 +0900 Subject: [PATCH 09/14] fix: use the selection shown in the page after returning to Chrome Ignoring the selection after the window lost focus didn't match what the user sees in the page. The selection is now ignored only when another app has focus at the time the shortcut is run. Co-Authored-By: Claude Opus 5.5 --- .../src/__tests__/service_worker.test.ts | 53 +++---------------- .../src/components/menu/InvisibleItem.tsx | 4 +- .../src/providers/SelectContextProvider.tsx | 15 ++---- .../__tests__/SelectContextProvider.test.tsx | 27 +--------- packages/extension/src/service_worker.ts | 23 +++----- packages/extension/src/services/ipc.ts | 10 ---- 6 files changed, 21 insertions(+), 111 deletions(-) diff --git a/packages/extension/src/__tests__/service_worker.test.ts b/packages/extension/src/__tests__/service_worker.test.ts index 6408f04ba..f2f330d4c 100644 --- a/packages/extension/src/__tests__/service_worker.test.ts +++ b/packages/extension/src/__tests__/service_worker.test.ts @@ -280,21 +280,18 @@ describe("Service Worker Migration", () => { tab, tabSelection, noSelectionBehavior = SHORTCUT_NO_SELECTION_BEHAVIOR.DO_NOTHING, - keepMenuOpenOnFocusChange = false, chromeFocused = true, executeActionResult = new Error("Tab execution failed"), }: { tab: { id: number; url: string; windowId: number } tabSelection?: unknown noSelectionBehavior?: SHORTCUT_NO_SELECTION_BEHAVIOR - keepMenuOpenOnFocusChange?: boolean chromeFocused?: boolean executeActionResult?: unknown }) => { const initialSettings = createTestSettings() as any initialSettings.shortcuts.shortcuts[0].noSelectionBehavior = noSelectionBehavior - initialSettings.startupMethod = { keepMenuOpenOnFocusChange } mockEnhancedSettings.get.mockResolvedValue(initialSettings) vi.doMock("@/services/chrome", () => ({ @@ -373,7 +370,7 @@ describe("Service Worker Migration", () => { const { commandListener, mockExecute } = await setupOnCommand({ tab: { id: 1, url: "https://example.com", windowId: 1 }, - tabSelection: { text: "test selection text", focusLost: false }, + tabSelection: "test selection text", }) // Call the listener with our test shortcut command @@ -431,7 +428,7 @@ describe("Service Worker Migration", () => { const { commandListener, mockExecute, mockIpcSendTab } = await setupOnCommand({ tab: { id: 1, url: "https://example.com", windowId: 1 }, - tabSelection: { text: "", focusLost: false }, + tabSelection: "", noSelectionBehavior: SHORTCUT_NO_SELECTION_BEHAVIOR.DO_NOTHING, }) @@ -449,7 +446,7 @@ describe("Service Worker Migration", () => { const { commandListener, mockExecute, mockIpcSendTab } = await setupOnCommand({ tab: { id: 1, url: "https://example.com", windowId: 1 }, - tabSelection: { text: "selected", focusLost: false }, + tabSelection: "selected", noSelectionBehavior: SHORTCUT_NO_SELECTION_BEHAVIOR.USE_CLIPBOARD, executeActionResult: true, }) @@ -465,45 +462,7 @@ describe("Service Worker Migration", () => { expect(mockExecute).not.toHaveBeenCalled() }) - it("MG-01-h: should use the clipboard when the window lost focus after the text was selected", async () => { - // The text is still selected in the page, but the user may have copied - // other text in another app. - const { commandListener, mockIpcSendTab } = await setupOnCommand({ - tab: { id: 1, url: "https://example.com", windowId: 1 }, - tabSelection: { text: "selected", focusLost: true }, - noSelectionBehavior: SHORTCUT_NO_SELECTION_BEHAVIOR.USE_CLIPBOARD, - executeActionResult: true, - }) - - await runShortcut(commandListener) - - expect(mockIpcSendTab).toHaveBeenCalledWith(1, "executeAction", { - command: expect.objectContaining({ id: "test-command-123" }), - selectionText: "", - selectionFromClipboard: true, - }) - }) - - it("MG-01-i: should ignore the selection after the window lost focus even when keepMenuOpenOnFocusChange is on", async () => { - // The setting only keeps the menu open. It doesn't keep the selection. - const { commandListener, mockIpcSendTab } = await setupOnCommand({ - tab: { id: 1, url: "https://example.com", windowId: 1 }, - tabSelection: { text: "selected", focusLost: true }, - noSelectionBehavior: SHORTCUT_NO_SELECTION_BEHAVIOR.USE_CLIPBOARD, - keepMenuOpenOnFocusChange: true, - executeActionResult: true, - }) - - await runShortcut(commandListener) - - expect(mockIpcSendTab).toHaveBeenCalledWith(1, "executeAction", { - command: expect.objectContaining({ id: "test-command-123" }), - selectionText: "", - selectionFromClipboard: true, - }) - }) - - it("MG-01-j: should treat the selection as empty when the tab doesn't respond", async () => { + it("MG-01-h: should treat the selection as empty when the tab doesn't respond", async () => { // e.g. the content script isn't injected yet. const { commandListener, mockExecute } = await setupOnCommand({ tab: { id: 1, url: "https://example.com", windowId: 1 }, @@ -521,11 +480,11 @@ describe("Service Worker Migration", () => { ) }) - it("MG-01-k: should treat the selection as empty when another app has focus", async () => { + it("MG-01-i: should treat the selection as empty when another app has focus", async () => { // e.g. a global shortcut pressed right after copying text in another app. const { commandListener, mockIpcSendTab } = await setupOnCommand({ tab: { id: 1, url: "https://example.com", windowId: 1 }, - tabSelection: { text: "selected", focusLost: false }, + tabSelection: "selected", noSelectionBehavior: SHORTCUT_NO_SELECTION_BEHAVIOR.USE_CLIPBOARD, chromeFocused: false, executeActionResult: true, diff --git a/packages/extension/src/components/menu/InvisibleItem.tsx b/packages/extension/src/components/menu/InvisibleItem.tsx index 0a424d407..f4223fb9e 100644 --- a/packages/extension/src/components/menu/InvisibleItem.tsx +++ b/packages/extension/src/components/menu/InvisibleItem.tsx @@ -66,8 +66,8 @@ export function InvisibleItem(props: InvisibleItemProps): React.ReactNode { executeCommand({ command: action.command as SelectionCommand, position, - // Prefer the selection decided by the sender, which ignores a selection - // made before the window lost focus. + // Prefer the selection decided by the sender, which ignores the + // selection when another app has focus. selectionText: action.selectionText ?? selectionText, target, selectionFromClipboard: action.selectionFromClipboard, diff --git a/packages/extension/src/providers/SelectContextProvider.tsx b/packages/extension/src/providers/SelectContextProvider.tsx index b77131220..2e73aaaf8 100644 --- a/packages/extension/src/providers/SelectContextProvider.tsx +++ b/packages/extension/src/providers/SelectContextProvider.tsx @@ -1,5 +1,5 @@ import { useState, ReactNode, useEffect, useCallback, useRef } from "react" -import { Ipc, TabCommand, type TabSelection } from "@/services/ipc" +import { Ipc, TabCommand } from "@/services/ipc" import { getSelectionText } from "@/services/dom" import { ContextType, selectContext } from "@/hooks/useSelectContext" @@ -11,11 +11,11 @@ export const SelectContextProvider = ({ const [selectionText, _setSelectionText] = useState("") const [target, setTarget] = useState(null) const [detectSelectionEnabled, setDetectSelectionEnabled] = useState(true) - const selectionRef = useRef({ text: "", focusLost: false }) + const selectionRef = useRef("") const setSelectionText = useCallback((text: string) => { _setSelectionText(text) - selectionRef.current = { text, focusLost: false } + selectionRef.current = text }, []) useEffect(() => { @@ -31,21 +31,12 @@ export const SelectContextProvider = ({ }, [detectSelectionEnabled, setSelectionText]) useEffect(() => { - // The selection stays in the page after the window loses focus, so the - // shortcut handler in the service worker needs to know it to decide - // whether to use the selection or the clipboard. - const onBlur = () => { - selectionRef.current = { ...selectionRef.current, focusLost: true } - } - window.addEventListener("blur", onBlur) - Ipc.addListener(TabCommand.getSelection, (_param, _sender, response) => { response(selectionRef.current) return true }) return () => { - window.removeEventListener("blur", onBlur) Ipc.removeListener(TabCommand.getSelection) } }, []) diff --git a/packages/extension/src/providers/__tests__/SelectContextProvider.test.tsx b/packages/extension/src/providers/__tests__/SelectContextProvider.test.tsx index 1f658a880..0ef68a524 100644 --- a/packages/extension/src/providers/__tests__/SelectContextProvider.test.tsx +++ b/packages/extension/src/providers/__tests__/SelectContextProvider.test.tsx @@ -49,33 +49,10 @@ describe("SelectContextProvider", () => { selectText("selected") - expect(requestSelection()).toEqual({ text: "selected", focusLost: false }) + expect(requestSelection()).toBe("selected") }) - it("SC-02: 正常系: 選択後にウィンドウのフォーカスが外れたことを返す", () => { - render({null}) - - selectText("selected") - act(() => { - window.dispatchEvent(new Event("blur")) - }) - - expect(requestSelection()).toEqual({ text: "selected", focusLost: true }) - }) - - it("SC-03: 正常系: フォーカスが外れた後に選択し直すと、フォーカス喪失をリセットする", () => { - render({null}) - - selectText("old") - act(() => { - window.dispatchEvent(new Event("blur")) - }) - selectText("new") - - expect(requestSelection()).toEqual({ text: "new", focusLost: false }) - }) - - it("SC-04: 正常系: アンマウント時にリスナーを解除する", () => { + it("SC-02: 正常系: アンマウント時にリスナーを解除する", () => { const { unmount } = render( {null}, ) diff --git a/packages/extension/src/service_worker.ts b/packages/extension/src/service_worker.ts index d25021a04..510b8b006 100644 --- a/packages/extension/src/service_worker.ts +++ b/packages/extension/src/service_worker.ts @@ -14,7 +14,7 @@ import { TabCommand, CONNECTION_APP, } from "@/services/ipc" -import type { IpcCallback, TabSelection } from "@/services/ipc" +import type { IpcCallback } from "@/services/ipc" import { Settings } from "@/services/settings/settings" import { enhancedSettings } from "@/services/settings/enhancedSettings" import { CACHE_SECTIONS } from "@/services/settings/settingsCache" @@ -686,7 +686,8 @@ chrome.commands.onCommand.addListener(async (commandName) => { // A global shortcut can be pressed while another app has focus. Then the // selection left in the page is ignored, as the user may have copied - // other text in that app. getLastFocused fails when no window is open. + // other text in that app. After returning to Chrome, the selection shown + // in the page is used. getLastFocused fails when no window is open. const lastFocusedWindow = await chrome.windows .getLastFocused() .catch(() => undefined) @@ -695,19 +696,11 @@ chrome.commands.onCommand.addListener(async (commandName) => { // script (e.g. chrome://) are treated as having no selection. let selectionText = "" if (enableSendTab && lastFocusedWindow?.focused) { - const selection = await Ipc.sendTab< - unknown, - TabSelection | Error | undefined - >(tab.id ?? 0, TabCommand.getSelection) - // A selection made before the window lost focus may be older than the - // clipboard, as the user may have copied other text elsewhere. - if ( - selection != null && - !(selection instanceof Error) && - !selection.focusLost - ) { - selectionText = selection.text - } + const selection = await Ipc.sendTab( + tab.id ?? 0, + TabCommand.getSelection, + ) + if (typeof selection === "string") selectionText = selection } // This is the only place to decide whether to use the clipboard text as diff --git a/packages/extension/src/services/ipc.ts b/packages/extension/src/services/ipc.ts index 1b815da1e..d30e6d4f8 100644 --- a/packages/extension/src/services/ipc.ts +++ b/packages/extension/src/services/ipc.ts @@ -105,16 +105,6 @@ export type ConfirmCommandUpdateProps = { title: string } -/** The selection in a tab, returned for TabCommand.getSelection. */ -export type TabSelection = { - text: string - /** - * The window lost focus after the text was selected. The user may have - * copied other text elsewhere in the meantime. - */ - focusLost: boolean -} - export type ClickElementProps = { selector: string } From 44cd8960e8e4bea12657556f397a777e1c47d89e Mon Sep 17 00:00:00 2001 From: ujiro99 Date: Tue, 20 Oct 2026 11:22:10 +0900 Subject: [PATCH 10/14] =?UTF-8?q?test:=20=E3=82=B7=E3=83=A7=E3=83=BC?= =?UTF-8?q?=E3=83=88=E3=82=AB=E3=83=83=E3=83=88=E5=AE=9F=E8=A1=8C=E3=81=AE?= =?UTF-8?q?E2E=E3=83=86=E3=82=B9=E3=83=88=E3=82=92=E8=BF=BD=E5=8A=A0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/test/e2e-test-spec.md | 23 ++++ packages/extension/e2e/shortcut.spec.ts | 150 +++++++++++++++++++++++ packages/extension/src/service_worker.ts | 11 +- 3 files changed, 182 insertions(+), 2 deletions(-) create mode 100644 packages/extension/e2e/shortcut.spec.ts diff --git a/docs/test/e2e-test-spec.md b/docs/test/e2e-test-spec.md index 25bb21573..caa98e484 100644 --- a/docs/test/e2e-test-spec.md +++ b/docs/test/e2e-test-spec.md @@ -63,6 +63,12 @@ | E2E-90 | ハブ | PageActionコマンド | インストール | 実装済 | | E2E-91 | ハブ | 追加 | - | 実装済 | | E2E-92 | ハブ | 削除 | - | 実装済 | +| E2E-100 | ショートカット | 検索・選択文字列 | onCommand以降 | 実装済 | +| E2E-101 | ショートカット | 日本語・空白・記号 | onCommand以降 | 実装済 | +| E2E-102 | ショートカット | クリップボード | onCommand以降 | 実装済 | +| E2E-103 | ショートカット | DO_NOTHING | onCommand以降 | 実装済 | +| E2E-104 | ショートカット | Content Scriptなし | onCommand以降 | 実装済 | +| E2E-105 | ショートカット | 登録状態 | onCommand以降 | 実装済 | --- @@ -1048,3 +1054,20 @@ **期待動作** - 削除したコマンドに対応する Hub ページのダウンロードボタンが復活する（「追加済み」表示が解除される） + +### ショートカット（shortcut.spec.ts） + +`chrome.commands.onCommand` に登録する `handleShortcutCommand` 以降を検証する。 +E2EビルドでのみService Workerのグローバルへ関数を公開し、Playwrightの +`Worker.evaluate()` から呼び出す。IPCや外部Webページ向けの受付経路は追加しない。 +本番ビルドでは公開処理が除去される。OSのキー入力・Chromeのイベント発火、 +サイドパネルは対象外。各テストでOptionsPageから設定をインポートする。 + +| テストID | 手順 | 期待動作 | +| -------- | ------------------------------------------------------ | -------------------------------------------------------------------------------- | +| E2E-100 | TestPageで見出しを選択しslot_1を実行 | 割り当てた検索コマンドの結果が開き、選択文字列が検索パラメータに渡る | +| E2E-101 | 日本語・空白・記号を選択しslot_1を実行 | URLエンコードされた文字列が正しく渡る | +| E2E-102 | 未選択状態でクリップボードへ文字列を設定しslot_1を実行 | クリップボードの文字列で検索する | +| E2E-103 | 未選択状態でDO_NOTHINGのslot_2を実行し処理完了を待つ | タブ数・ID・URLが変化しない | +| E2E-104 | クリップボードを設定しchrome://versionでslot_1を実行 | Content Scriptを使わずService Workerから検索する | +| E2E-105 | Service Workerでchrome.commands.getAll()を実行 | slot_1〜slot_3の登録を確認する。キー文字列の値はOSや競合に依存するため固定しない | diff --git a/packages/extension/e2e/shortcut.spec.ts b/packages/extension/e2e/shortcut.spec.ts new file mode 100644 index 000000000..f8d252a02 --- /dev/null +++ b/packages/extension/e2e/shortcut.spec.ts @@ -0,0 +1,150 @@ +import type { BrowserContext } from "@playwright/test" +import { test, expect } from "./fixtures" +import { OptionsPage } from "./pages/OptionsPage" +import { TestPage } from "./pages/TestPage" +import { readFileSync } from "node:fs" + +const testSettings = JSON.parse( + readFileSync(new URL("./data/test-settings.json", import.meta.url), "utf-8"), +) + +async function runShortcut(context: BrowserContext) { + const worker = context.serviceWorkers()[0] + await worker.evaluate(async () => { + const workerGlobal = globalThis as typeof globalThis & { + __e2eHandleShortcutCommand: (commandName: string) => Promise + } + await workerGlobal.__e2eHandleShortcutCommand("slot_1") + }) +} + +async function expectSearch(context: BrowserContext, text: string) { + const resultPromise = context.waitForEvent("page") + await runShortcut(context) + const result = await resultPromise + await expect(result).toHaveURL( + `https://ujiro99.github.io/selection-command/en/test?k=${encodeURIComponent(text).replaceAll("%20", "+")}`, + ) +} + +test.beforeEach(async ({ context, extensionId, getCommands }) => { + const options = new OptionsPage(context, extensionId, getCommands) + await options.importSettings({ + ...testSettings, + shortcuts: { + shortcuts: [ + { + id: "slot_1", + commandId: "13b4e831-7fac-4b72-a1ae-c82655b3819e", + noSelectionBehavior: "useClipboard", + }, + { + id: "slot_2", + commandId: "13b4e831-7fac-4b72-a1ae-c82655b3819e", + noSelectionBehavior: "doNothing", + }, + ], + }, + }) + await options.close() +}) + +test("E2E-100: slot_1で検索コマンドが実行され選択テキストが渡る", async ({ + context, + page, +}) => { + const testPage = new TestPage(page) + await testPage.open() + await page.bringToFront() + await testPage.selectText("h2") + const selection = await page.evaluate(() => window.getSelection()!.toString()) + expect(selection).not.toBe("") + await expectSearch(context, selection) +}) + +test("E2E-101: 選択テキストの日本語・空白・記号が検索URLへ渡る", async ({ + context, + page, +}) => { + const testPage = new TestPage(page) + await testPage.open() + await page.bringToFront() + const text = "日本語 space & + ?" + await page + .locator("h2") + .first() + .evaluate((element, text) => { + element.textContent = text + const range = document.createRange() + range.selectNodeContents(element) + window.getSelection()!.removeAllRanges() + window.getSelection()!.addRange(range) + }, text) + await expectSearch(context, text) +}) + +test("E2E-102: 未選択時にクリップボードから検索する", async ({ + context, + page, +}) => { + await context.grantPermissions(["clipboard-read", "clipboard-write"]) + await new TestPage(page).open() + await page.bringToFront() + const text = "clipboard 日本語 & test" + await page.evaluate(async (text) => { + window.getSelection()!.removeAllRanges() + await navigator.clipboard.writeText(text) + }, text) + await expectSearch(context, text) +}) + +test("E2E-103: DO_NOTHING設定では未選択時に実行しない", async ({ + context, + page, +}) => { + await new TestPage(page).open() + await page.bringToFront() + await page.evaluate(() => window.getSelection()!.removeAllRanges()) + const worker = context.serviceWorkers()[0] + const before = await worker.evaluate(() => chrome.tabs.query({})) + await worker.evaluate(async () => { + const workerGlobal = globalThis as typeof globalThis & { + __e2eHandleShortcutCommand: (commandName: string) => Promise + } + await workerGlobal.__e2eHandleShortcutCommand("slot_2") + }) + const after = await worker.evaluate(() => chrome.tabs.query({})) + expect(after.map(({ id, url }) => ({ id, url }))).toEqual( + before.map(({ id, url }) => ({ id, url })), + ) +}) + +test("E2E-104: Content Scriptのないchrome://ページではService Workerから実行する", async ({ + context, + page, +}) => { + await context.grantPermissions(["clipboard-read", "clipboard-write"]) + await new TestPage(page).open() + await page.bringToFront() + const text = "fallback clipboard" + await page.evaluate((text) => navigator.clipboard.writeText(text), text) + await page.goto("chrome://version") + await expectSearch(context, text) +}) + +test("E2E-105: chrome.commands.getAllでslot_1〜slot_3が登録されている", async ({ + context, +}) => { + const commands = await context + .serviceWorkers()[0] + .evaluate(() => chrome.commands.getAll()) + for (const name of ["slot_1", "slot_2", "slot_3"]) { + expect(commands).toContainEqual( + expect.objectContaining({ + name, + description: expect.any(String), + shortcut: expect.any(String), + }), + ) + } +}) diff --git a/packages/extension/src/service_worker.ts b/packages/extension/src/service_worker.ts index 510b8b006..602f4b7d9 100644 --- a/packages/extension/src/service_worker.ts +++ b/packages/extension/src/service_worker.ts @@ -659,7 +659,7 @@ Settings.addChangedListener(() => ContextMenu.init()) // ) // Add command processing -chrome.commands.onCommand.addListener(async (commandName) => { +async function handleShortcutCommand(commandName: string) { try { // Get settings const settings = await enhancedSettings.get() @@ -760,7 +760,14 @@ chrome.commands.onCommand.addListener(async (commandName) => { } catch (error) { console.error("Failed to execute shortcut command:", error) } -}) +} + +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 }) +} // Side panel is an optional capability (e.g. not available on Opera), so // each event is feature-detected individually. From 938ff082994d2f107fc94ee3dcb8d1b58260ed09 Mon Sep 17 00:00:00 2001 From: ujiro99 Date: Tue, 20 Oct 2026 12:30:41 +0900 Subject: [PATCH 11/14] =?UTF-8?q?fix:=20=E3=82=AF=E3=83=AA=E3=83=83?= =?UTF-8?q?=E3=83=97=E3=83=9C=E3=83=BC=E3=83=89=E8=AA=AD=E3=81=BF=E5=8F=96?= =?UTF-8?q?=E3=82=8A=E3=82=92=E9=9D=9E=E8=A1=A8=E7=A4=BA=E5=8C=96=E3=81=97?= =?UTF-8?q?PageAction=E3=81=AE=E6=96=87=E5=AD=97=E5=85=A5=E5=8A=9B?= =?UTF-8?q?=E3=82=92=E4=BF=AE=E6=AD=A3?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/test/e2e-test-spec.md | 29 ++ packages/extension/e2e/shortcut.spec.ts | 388 +++++++++++++----- packages/extension/manifest.json | 1 + .../extension/src/offscreen_clipboard.html | 11 + packages/extension/src/offscreen_clipboard.ts | 32 ++ .../services/__tests__/chrome.openTab.test.ts | 102 +++++ .../__tests__/offscreenClipboard.test.ts | 85 ++++ packages/extension/src/services/chrome.ts | 75 ++-- .../src/services/offscreenClipboard.ts | 47 +++ .../__tests__/dispatcher.input.test.ts | 44 ++ .../src/services/pageAction/dispatcher.ts | 3 +- packages/extension/vite.config.ts | 1 + 12 files changed, 673 insertions(+), 145 deletions(-) create mode 100644 packages/extension/src/offscreen_clipboard.html create mode 100644 packages/extension/src/offscreen_clipboard.ts create mode 100644 packages/extension/src/services/__tests__/chrome.openTab.test.ts create mode 100644 packages/extension/src/services/__tests__/offscreenClipboard.test.ts create mode 100644 packages/extension/src/services/offscreenClipboard.ts create mode 100644 packages/extension/src/services/pageAction/__tests__/dispatcher.input.test.ts diff --git a/docs/test/e2e-test-spec.md b/docs/test/e2e-test-spec.md index caa98e484..0826539dd 100644 --- a/docs/test/e2e-test-spec.md +++ b/docs/test/e2e-test-spec.md @@ -69,6 +69,11 @@ | E2E-103 | ショートカット | DO_NOTHING | onCommand以降 | 実装済 | | E2E-104 | ショートカット | Content Scriptなし | onCommand以降 | 実装済 | | E2E-105 | ショートカット | 登録状態 | onCommand以降 | 実装済 | +| E2E-106 | ショートカット | PageAction / Popup | Clipboard / 未選択 | 実装済 | +| E2E-107 | ショートカット | PageAction / BackgroundTab | Clipboard / 未選択 | 実装済 | +| E2E-108 | ショートカット | PageAction / Popup | SelectedText / 選択あり | 実装済 | +| E2E-109 | ショートカット | PageAction / Popup | SelectedText / 未選択 | 実装済 | +| E2E-110 | ショートカット | PageAction / BackgroundTab | SelectedText / 未選択 | 実装済 | --- @@ -1071,3 +1076,27 @@ E2EビルドでのみService Workerのグローバルへ関数を公開し、Pla | E2E-103 | 未選択状態でDO_NOTHINGのslot_2を実行し処理完了を待つ | タブ数・ID・URLが変化しない | | E2E-104 | クリップボードを設定しchrome://versionでslot_1を実行 | Content Scriptを使わずService Workerから検索する | | E2E-105 | Service Workerでchrome.commands.getAll()を実行 | slot_1〜slot_3の登録を確認する。キー文字列の値はOSや競合に依存するため固定しない | + +#### PageActionの変数展開（E2E-106〜110） + +各ケースで入力ステップを含むPageActionをOptionsPageからインポートし、 +`slot_1` に割り当てる。未選択時の設定は `USE_CLIPBOARD` とする。 +TestPageを開き、実際のクリップボードに日本語・記号・改行と、配列を含む +TypeScriptコードを設定する。`[KeyA]`、`{Enter}`、`[]`、`[[`、`{{` も含め、 +キー操作として解釈されず、textareaに文字列のまま入力されることを検証する。 +ショートカット実行後、入力欄の値が `Input: ` と期待する文字列に一致するまで待機する。 +ウィンドウ種別がポップアップ／通常ウィンドウであることも検証する。 +テスト側で対象ページをアクティブ化する操作は行わない。 + +バックグラウンドタブでは `readClipboard()` がOffscreen Documentで読み取り、 +結果タブを非アクティブで作成する。E2E-107／110ではウィンドウが新規作成されないこと、 +結果タブの非アクティブ状態と元ページのフォーカス維持も検証する。 +Offscreen API非対応環境では従来の一時ウィンドウ方式を使用する。 + +| テストID | 選択状態 | 入力ステップ | 表示方法 | 期待する入力値の変数部分 | +| -------- | -------- | ------------------ | -------------------- | ------------------------------------------ | +| E2E-106 | 未選択 | `{{Clipboard}}` | ポップアップ | クリップボードの内容 | +| E2E-107 | 未選択 | `{{Clipboard}}` | バックグラウンドタブ | クリップボードの内容 | +| E2E-108 | 選択あり | `{{SelectedText}}` | ポップアップ | 選択テキスト（クリップボードとは異なる値） | +| E2E-109 | 未選択 | `{{SelectedText}}` | ポップアップ | クリップボードの内容 | +| E2E-110 | 未選択 | `{{SelectedText}}` | バックグラウンドタブ | クリップボードの内容 | diff --git a/packages/extension/e2e/shortcut.spec.ts b/packages/extension/e2e/shortcut.spec.ts index f8d252a02..6b4401b95 100644 --- a/packages/extension/e2e/shortcut.spec.ts +++ b/packages/extension/e2e/shortcut.spec.ts @@ -27,124 +27,296 @@ async function expectSearch(context: BrowserContext, text: string) { ) } -test.beforeEach(async ({ context, extensionId, getCommands }) => { - const options = new OptionsPage(context, extensionId, getCommands) - await options.importSettings({ - ...testSettings, - shortcuts: { - shortcuts: [ - { - id: "slot_1", - commandId: "13b4e831-7fac-4b72-a1ae-c82655b3819e", - noSelectionBehavior: "useClipboard", - }, - { - id: "slot_2", - commandId: "13b4e831-7fac-4b72-a1ae-c82655b3819e", - noSelectionBehavior: "doNothing", - }, - ], - }, +test.describe("検索ショートカット", () => { + test.beforeEach(async ({ context, extensionId, getCommands }) => { + const options = new OptionsPage(context, extensionId, getCommands) + await options.importSettings({ + ...testSettings, + shortcuts: { + shortcuts: [ + { + id: "slot_1", + commandId: "13b4e831-7fac-4b72-a1ae-c82655b3819e", + noSelectionBehavior: "useClipboard", + }, + { + id: "slot_2", + commandId: "13b4e831-7fac-4b72-a1ae-c82655b3819e", + noSelectionBehavior: "doNothing", + }, + ], + }, + }) + await options.close() }) - await options.close() -}) -test("E2E-100: slot_1で検索コマンドが実行され選択テキストが渡る", async ({ - context, - page, -}) => { - const testPage = new TestPage(page) - await testPage.open() - await page.bringToFront() - await testPage.selectText("h2") - const selection = await page.evaluate(() => window.getSelection()!.toString()) - expect(selection).not.toBe("") - await expectSearch(context, selection) -}) + test("E2E-100: slot_1で検索コマンドが実行され選択テキストが渡る", async ({ + context, + page, + }) => { + const testPage = new TestPage(page) + await testPage.open() + await page.bringToFront() + await testPage.selectText("h2") + const selection = await page.evaluate(() => + window.getSelection()!.toString(), + ) + expect(selection).not.toBe("") + await expectSearch(context, selection) + }) + + test("E2E-101: 選択テキストの日本語・空白・記号が検索URLへ渡る", async ({ + context, + page, + }) => { + const testPage = new TestPage(page) + await testPage.open() + await page.bringToFront() + const text = "日本語 space & + ?" + await page + .locator("h2") + .first() + .evaluate((element, text) => { + element.textContent = text + const range = document.createRange() + range.selectNodeContents(element) + window.getSelection()!.removeAllRanges() + window.getSelection()!.addRange(range) + }, text) + await expectSearch(context, text) + }) -test("E2E-101: 選択テキストの日本語・空白・記号が検索URLへ渡る", async ({ - context, - page, -}) => { - const testPage = new TestPage(page) - await testPage.open() - await page.bringToFront() - const text = "日本語 space & + ?" - await page - .locator("h2") - .first() - .evaluate((element, text) => { - element.textContent = text - const range = document.createRange() - range.selectNodeContents(element) + test("E2E-102: 未選択時にクリップボードから検索する", async ({ + context, + page, + }) => { + await context.grantPermissions(["clipboard-read", "clipboard-write"]) + await new TestPage(page).open() + await page.bringToFront() + const text = "clipboard 日本語 & test" + await page.evaluate(async (text) => { window.getSelection()!.removeAllRanges() - window.getSelection()!.addRange(range) + await navigator.clipboard.writeText(text) }, text) - await expectSearch(context, text) -}) + await expectSearch(context, text) + }) -test("E2E-102: 未選択時にクリップボードから検索する", async ({ - context, - page, -}) => { - await context.grantPermissions(["clipboard-read", "clipboard-write"]) - await new TestPage(page).open() - await page.bringToFront() - const text = "clipboard 日本語 & test" - await page.evaluate(async (text) => { - window.getSelection()!.removeAllRanges() - await navigator.clipboard.writeText(text) - }, text) - await expectSearch(context, text) -}) + test("E2E-103: DO_NOTHING設定では未選択時に実行しない", async ({ + context, + page, + }) => { + await new TestPage(page).open() + await page.bringToFront() + await page.evaluate(() => window.getSelection()!.removeAllRanges()) + const worker = context.serviceWorkers()[0] + const before = await worker.evaluate(() => chrome.tabs.query({})) + await worker.evaluate(async () => { + const workerGlobal = globalThis as typeof globalThis & { + __e2eHandleShortcutCommand: (commandName: string) => Promise + } + await workerGlobal.__e2eHandleShortcutCommand("slot_2") + }) + const after = await worker.evaluate(() => chrome.tabs.query({})) + expect(after.map(({ id, url }) => ({ id, url }))).toEqual( + before.map(({ id, url }) => ({ id, url })), + ) + }) -test("E2E-103: DO_NOTHING設定では未選択時に実行しない", async ({ - context, - page, -}) => { - await new TestPage(page).open() - await page.bringToFront() - await page.evaluate(() => window.getSelection()!.removeAllRanges()) - const worker = context.serviceWorkers()[0] - const before = await worker.evaluate(() => chrome.tabs.query({})) - await worker.evaluate(async () => { - const workerGlobal = globalThis as typeof globalThis & { - __e2eHandleShortcutCommand: (commandName: string) => Promise + test("E2E-104: Content Scriptのないchrome://ページではService Workerから実行する", async ({ + context, + page, + }) => { + await context.grantPermissions(["clipboard-read", "clipboard-write"]) + await new TestPage(page).open() + await page.bringToFront() + const text = "fallback clipboard" + await page.evaluate((text) => navigator.clipboard.writeText(text), text) + await page.goto("chrome://version") + await expectSearch(context, text) + }) + + test("E2E-105: chrome.commands.getAllでslot_1〜slot_3が登録されている", async ({ + context, + }) => { + const commands = await context + .serviceWorkers()[0] + .evaluate(() => chrome.commands.getAll()) + for (const name of ["slot_1", "slot_2", "slot_3"]) { + expect(commands).toContainEqual( + expect.objectContaining({ + name, + description: expect.any(String), + shortcut: expect.any(String), + }), + ) } - await workerGlobal.__e2eHandleShortcutCommand("slot_2") }) - const after = await worker.evaluate(() => chrome.tabs.query({})) - expect(after.map(({ id, url }) => ({ id, url }))).toEqual( - before.map(({ id, url }) => ({ id, url })), - ) }) -test("E2E-104: Content Scriptのないchrome://ページではService Workerから実行する", async ({ - context, - page, -}) => { - await context.grantPermissions(["clipboard-read", "clipboard-write"]) - await new TestPage(page).open() - await page.bringToFront() - const text = "fallback clipboard" - await page.evaluate((text) => navigator.clipboard.writeText(text), text) - await page.goto("chrome://version") - await expectSearch(context, text) -}) +const pageActionCases = [ + { + id: "E2E-106", + openMode: "popup", + variable: "Clipboard", + hasSelection: false, + }, + { + id: "E2E-107", + openMode: "backgroundTab", + variable: "Clipboard", + hasSelection: false, + }, + { + id: "E2E-108", + openMode: "popup", + variable: "SelectedText", + hasSelection: true, + }, + { + id: "E2E-109", + openMode: "popup", + variable: "SelectedText", + hasSelection: false, + }, + { + id: "E2E-110", + openMode: "backgroundTab", + variable: "SelectedText", + hasSelection: false, + }, +] as const + +for (const { id, openMode, variable, hasSelection } of pageActionCases) { + test(`${id}: PageActionショートカット / ${openMode} / {{${variable}}} / ${hasSelection ? "選択あり" : "未選択"}`, async ({ + context, + extensionId, + getCommands, + page, + }) => { + const targetUrl = "https://ujiro99.github.io/selection-command/en/test" + const commandId = "shortcut-page-action" + const options = new OptionsPage(context, extensionId, getCommands) + await options.importSettings({ + ...testSettings, + commands: [ + ...testSettings.commands, + { + id: commandId, + title: "Shortcut PageAction", + openMode: "pageAction", + popupOption: { width: 600, height: 700 }, + pageActionOption: { + startUrl: `${targetUrl}?shortcutPageAction=${id}`, + openMode, + steps: [ + { + id: "start", + param: { type: "start", label: "Start", url: targetUrl }, + }, + { + id: "input", + param: { + type: "input", + label: "Textarea", + selector: "//textarea", + selectorType: "xpath", + value: `Input: {{${variable}}}`, + }, + }, + { id: "end", param: { type: "end", label: "End" } }, + ], + }, + }, + ], + shortcuts: { + shortcuts: [ + { id: "slot_1", commandId, noSelectionBehavior: "useClipboard" }, + ], + }, + }) + await options.close() + + await context.grantPermissions(["clipboard-read", "clipboard-write"]) + const testPage = new TestPage(page) + await testPage.open() + await page.bringToFront() + const clipboardText = `clipboard 日本語 & ${id} + const contexts = await chrome.runtime.getContexts({ + contextTypes: [chrome.runtime.ContextType.OFFSCREEN_DOCUMENT], + documentUrls: [documentUrl], + }) +[KeyA] {Enter} [] [[ {{` + await page.evaluate(async (text) => { + window.getSelection()!.removeAllRanges() + await navigator.clipboard.writeText(text) + }, clipboardText) -test("E2E-105: chrome.commands.getAllでslot_1〜slot_3が登録されている", async ({ - context, -}) => { - const commands = await context - .serviceWorkers()[0] - .evaluate(() => chrome.commands.getAll()) - for (const name of ["slot_1", "slot_2", "slot_3"]) { - expect(commands).toContainEqual( - expect.objectContaining({ - name, - description: expect.any(String), - shortcut: expect.any(String), - }), + let expectedText = clipboardText + if (hasSelection) { + await testPage.selectText("h2") + expectedText = await page.evaluate(() => + window.getSelection()!.toString(), + ) + expect(expectedText).not.toBe("") + expect(expectedText).not.toBe(clipboardText) + } else { + expect(await page.evaluate(() => window.getSelection()!.toString())).toBe( + "", + ) + } + + if (openMode === "backgroundTab") { + await context.serviceWorkers()[0].evaluate(() => { + const workerGlobal = globalThis as typeof globalThis & { + createdWindowCount: number + } + workerGlobal.createdWindowCount = 0 + chrome.windows.onCreated.addListener( + () => workerGlobal.createdWindowCount++, + ) + }) + } + + // Popup mode may first load the clipboard page before the destination. + const resultPromise = context.waitForEvent("page", { + predicate: async (candidate) => { + try { + await candidate.waitForURL(`${targetUrl}?shortcutPageAction=${id}`) + return true + } catch { + return false + } + }, + }) + await runShortcut(context) + const result = await resultPromise + await expect(result).toHaveURL(`${targetUrl}?shortcutPageAction=${id}`) + await expect(result.locator("textarea")).toHaveValue( + `Input: ${expectedText}`, + { timeout: 10_000 }, ) - } -}) + + const resultTab = await context + .serviceWorkers()[0] + .evaluate(async (url) => { + const [tab] = await chrome.tabs.query({ url }) + return { + active: tab.active, + windowType: (await chrome.windows.get(tab.windowId)).type, + } + }, result.url()) + expect(resultTab.windowType).toBe(openMode === "popup" ? "popup" : "normal") + if (openMode === "backgroundTab") { + const createdWindowCount = await context + .serviceWorkers()[0] + .evaluate( + () => + (globalThis as typeof globalThis & { createdWindowCount: number }) + .createdWindowCount, + ) + expect(createdWindowCount).toBe(0) + expect(resultTab.active).toBe(false) + expect(await page.evaluate(() => document.hasFocus())).toBe(true) + } + }) +} diff --git a/packages/extension/manifest.json b/packages/extension/manifest.json index 2ff12311d..678556e11 100644 --- a/packages/extension/manifest.json +++ b/packages/extension/manifest.json @@ -51,6 +51,7 @@ "contextMenus", "declarativeNetRequest", "clipboardRead", + "offscreen", "system.display", "sidePanel" ], diff --git a/packages/extension/src/offscreen_clipboard.html b/packages/extension/src/offscreen_clipboard.html new file mode 100644 index 000000000..6e1be4502 --- /dev/null +++ b/packages/extension/src/offscreen_clipboard.html @@ -0,0 +1,11 @@ + + + + + Selection Command Clipboard + + + + + + diff --git a/packages/extension/src/offscreen_clipboard.ts b/packages/extension/src/offscreen_clipboard.ts new file mode 100644 index 000000000..1db0aef54 --- /dev/null +++ b/packages/extension/src/offscreen_clipboard.ts @@ -0,0 +1,32 @@ +import type { ClipboardResult } from "./services/ipc" + +chrome.runtime.onMessage.addListener((message, sender, respond) => { + if ( + sender.id !== chrome.runtime.id || + sender.tab || + message?.target !== "offscreen-clipboard" || + message.command !== "read" + ) { + return false + } + + const textarea = document.getElementById("clipboard") as HTMLTextAreaElement + try { + textarea.value = "" + textarea.focus() + textarea.select() + // Async Clipboard requires document focus, which offscreen documents lack. + if (!document.execCommand("paste")) { + throw new Error("Failed to paste clipboard text") + } + respond({ data: textarea.value } satisfies ClipboardResult) + } catch (error) { + respond({ + data: "", + err: error instanceof Error ? error.message : String(error), + } satisfies ClipboardResult) + } finally { + textarea.value = "" + } + return false +}) diff --git a/packages/extension/src/services/__tests__/chrome.openTab.test.ts b/packages/extension/src/services/__tests__/chrome.openTab.test.ts new file mode 100644 index 000000000..df487358f --- /dev/null +++ b/packages/extension/src/services/__tests__/chrome.openTab.test.ts @@ -0,0 +1,102 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest" +import { openTab } from "../chrome" +import { Ipc, ServiceWorkerCommand } from "../ipc" + +const sourceTab = { id: 10, windowId: 1, index: 2 } as chrome.tabs.Tab +const resultTab = { id: 20, windowId: 1 } as chrome.tabs.Tab +const url = { + searchUrl: "https://example.com/?q=%s", + selectionText: "", + selectionFromClipboard: true, +} + +function provideClipboard(data: string, err?: string) { + vi.mocked(chrome.runtime.onConnect.addListener).mockImplementation( + (listener) => { + const port = { + sender: { tab: { id: 30 } }, + onMessage: { + addListener: (onMessage: (message: unknown) => void) => { + onMessage({ + command: ServiceWorkerCommand.setClipboard, + data: { data, err }, + }) + }, + removeListener: vi.fn(), + }, + } as unknown as chrome.runtime.Port + listener(port) + }, + ) +} + +describe("openTab", () => { + afterEach(() => vi.unstubAllGlobals()) + beforeEach(() => { + vi.clearAllMocks() + vi.mocked(chrome.tabs.query).mockImplementation(() => + Promise.resolve([sourceTab]), + ) + vi.mocked(chrome.tabs.create).mockImplementation(() => + Promise.resolve(resultTab), + ) + vi.stubGlobal("chrome", { + ...chrome, + windows: { + ...chrome.windows, + create: vi.fn().mockResolvedValue({ id: 3, tabs: [{ id: 30 }] }), + remove: vi.fn().mockResolvedValue(undefined), + }, + }) + provideClipboard("clipboard text") + }) + + it("COT-01: 正常系: クリップボードを一時ウィンドウで読み非アクティブタブに渡す", async () => { + const result = await openTab({ url, active: false }) + + expect(chrome.windows.create).toHaveBeenCalledOnce() + expect(chrome.windows.remove).toHaveBeenCalledWith(3) + expect(chrome.tabs.create).toHaveBeenCalledExactlyOnceWith({ + url: "https://example.com/?q=clipboard+text", + active: false, + windowId: 1, + index: 3, + }) + expect(chrome.tabs.update).not.toHaveBeenCalled() + expect(result).toEqual({ tabId: 20, clipboardText: "clipboard text" }) + }) + + it("COT-02: 正常系: クリップボード不要なら一時ウィンドウを開かない", async () => { + await openTab({ + url: { ...url, selectionText: "selected", selectionFromClipboard: false }, + active: false, + }) + + expect(chrome.windows.create).not.toHaveBeenCalled() + expect(chrome.tabs.create).toHaveBeenCalledWith( + expect.objectContaining({ + url: "https://example.com/?q=selected", + active: false, + }), + ) + }) + + it("COT-03: 異常系: 読み取り失敗時も非アクティブで開きエラーを通知する", async () => { + provideClipboard("", "Permission denied") + const connection = vi.spyOn(Ipc, "ensureConnection").mockResolvedValue() + const sendTab = vi.spyOn(Ipc, "sendTab").mockResolvedValue(undefined) + try { + const result = await openTab({ url, active: false }) + expect(chrome.tabs.create).toHaveBeenCalledWith( + expect.objectContaining({ active: false }), + ) + expect(connection).toHaveBeenCalledWith(20) + expect(sendTab).toHaveBeenCalledWith(20, "showToast", expect.any(Object)) + expect(chrome.windows.remove).toHaveBeenCalledWith(3) + expect(result).toEqual({ tabId: 20, clipboardText: "" }) + } finally { + connection.mockRestore() + sendTab.mockRestore() + } + }) +}) diff --git a/packages/extension/src/services/__tests__/offscreenClipboard.test.ts b/packages/extension/src/services/__tests__/offscreenClipboard.test.ts new file mode 100644 index 000000000..b8826ffc9 --- /dev/null +++ b/packages/extension/src/services/__tests__/offscreenClipboard.test.ts @@ -0,0 +1,85 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest" +import { readOffscreenClipboard } from "../offscreenClipboard" + +const createDocument = vi.fn() +const closeDocument = vi.fn() +const getContexts = vi.fn() +const sendMessage = vi.fn() + +describe("readOffscreenClipboard", () => { + beforeEach(() => { + vi.resetAllMocks() + createDocument.mockResolvedValue(undefined) + closeDocument.mockResolvedValue(undefined) + getContexts.mockResolvedValue([]) + sendMessage.mockResolvedValue({ data: "日本語 clipboard" }) + vi.stubGlobal("chrome", { + ...chrome, + runtime: { + ...chrome.runtime, + getURL: (path: string) => `chrome-extension://test/${path}`, + getContexts, + ContextType: { OFFSCREEN_DOCUMENT: "OFFSCREEN_DOCUMENT" }, + sendMessage, + }, + offscreen: { + createDocument, + closeDocument, + Reason: { CLIPBOARD: "CLIPBOARD" }, + }, + }) + }) + afterEach(() => vi.unstubAllGlobals()) + + it("OC-01: 正常系: 非表示ドキュメントで読み取り後に閉じる", async () => { + await expect(readOffscreenClipboard()).resolves.toEqual({ + data: "日本語 clipboard", + }) + expect(createDocument).toHaveBeenCalledWith( + expect.objectContaining({ + url: "src/offscreen_clipboard.html", + reasons: ["CLIPBOARD"], + }), + ) + expect(closeDocument).toHaveBeenCalledOnce() + }) + + it("OC-02: 正常系: 再起動後に残ったドキュメントを再利用する", async () => { + getContexts.mockResolvedValue([{}]) + await readOffscreenClipboard() + expect(createDocument).not.toHaveBeenCalled() + expect(closeDocument).toHaveBeenCalledOnce() + }) + + it("OC-03: 異常系: 読み取り失敗時も閉じてエラーを返す", async () => { + sendMessage.mockRejectedValue(new Error("Read failed")) + await expect(readOffscreenClipboard()).resolves.toEqual({ + data: "", + err: "Read failed", + }) + expect(closeDocument).toHaveBeenCalledOnce() + }) + + it("OC-04: 正常系: 同時要求では前のドキュメントを閉じてから次を作る", async () => { + const events: string[] = [] + createDocument.mockImplementation(async () => { + events.push("create") + }) + sendMessage.mockImplementation(async () => { + events.push("read") + return { data: "text" } + }) + closeDocument.mockImplementation(async () => { + events.push("close") + }) + await Promise.all([readOffscreenClipboard(), readOffscreenClipboard()]) + expect(events).toEqual([ + "create", + "read", + "close", + "create", + "read", + "close", + ]) + }) +}) diff --git a/packages/extension/src/services/chrome.ts b/packages/extension/src/services/chrome.ts index 1b3661109..5911468c7 100644 --- a/packages/extension/src/services/chrome.ts +++ b/packages/extension/src/services/chrome.ts @@ -1,3 +1,4 @@ +import { readOffscreenClipboard } from "./offscreenClipboard" import { sleep, toUrl, isOverBytes, needsClipboard } from "@/lib/utils" import type { ScreenSize } from "@/services/dom" import type { ShowToastParam, UrlParam, WindowLayer } from "@/types" @@ -405,18 +406,20 @@ const openWindowAndReadClipboard = async ( } } -/** - * Open a temporary window to read clipboard content and return the result. - * @returns {Promise<{ clipboardText: string; err?: string }>} The clipboard content and any error message - * - * Note: This function opens a temporary window to read the clipboard content, - * which is necessary due to browser security restrictions. - * The window is closed immediately after reading the clipboard. - */ +/** Read clipboard text without a visible window when Offscreen API is available. */ export const readClipboard = async (): Promise<{ clipboardText: string err?: string }> => { + if ( + typeof chrome.offscreen?.createDocument === "function" && + typeof chrome.runtime.getContexts === "function" + ) { + const result = await readOffscreenClipboard() + return { clipboardText: result.data ?? "", err: result.err } + } + + // Preserve compatibility with browsers without the Offscreen API. const w = await chrome.windows.create({ url: chrome.runtime.getURL("src/clipboard.html"), focused: true, @@ -606,47 +609,47 @@ export const openTab = async (param: OpenTabProps): Promise => { console.warn("Failed to get current tab:", e) } - if (needsClipboard(url)) { - const tab = await chrome.tabs.create({ + const clipboardRequired = needsClipboard(url) + let tab: chrome.tabs.Tab + let result: ClipboardResult = { data: "" } + + if (clipboardRequired && active) { + tab = await chrome.tabs.create({ url: chrome.runtime.getURL("src/clipboard.html"), active: true, windowId: currentTab?.windowId, index: currentTab?.index !== undefined ? currentTab.index + 1 : undefined, }) - - const result = await readClipboardContent(tab.id as number) + result = await readClipboardContent(tab.id as number) await chrome.tabs.update(tab.id as number, { url: toUrl(url, result.data ?? ""), }) - - if (result.err) { - await Ipc.ensureConnection(tab.id as number) - await Ipc.sendTab( - tab.id as number, - TabCommand.showToast, - { - title: t("clipboard_error_title"), - description: t("clipboard_error_description"), - action: t("clipboard_error_action"), - }, - ) - } - - return { - tabId: tab.id as number, - clipboardText: result.data ?? "", - } } else { - const tab = await chrome.tabs.create({ - url: toUrl(url), + if (clipboardRequired) { + // Read separately so the destination tab stays inactive. + const clipboard = await readClipboard() + result = { data: clipboard.clipboardText, err: clipboard.err } + } + tab = await chrome.tabs.create({ + url: toUrl(url, result.data ?? ""), active, windowId: currentTab?.windowId, index: currentTab?.index !== undefined ? currentTab.index + 1 : undefined, }) - return { - tabId: tab.id as number, - clipboardText: "", - } + } + + if (result.err) { + await Ipc.ensureConnection(tab.id as number) + await Ipc.sendTab(tab.id as number, TabCommand.showToast, { + title: t("clipboard_error_title"), + description: t("clipboard_error_description"), + action: t("clipboard_error_action"), + }) + } + + return { + tabId: tab.id as number, + clipboardText: result.data ?? "", } } diff --git a/packages/extension/src/services/offscreenClipboard.ts b/packages/extension/src/services/offscreenClipboard.ts new file mode 100644 index 000000000..ebfdc781e --- /dev/null +++ b/packages/extension/src/services/offscreenClipboard.ts @@ -0,0 +1,47 @@ +import type { ClipboardResult } from "./ipc" + +let pendingRead: Promise = Promise.resolve() + +async function readOnce(): Promise { + const documentUrl = chrome.runtime.getURL("src/offscreen_clipboard.html") + let documentReady = false + try { + const contexts = await chrome.runtime.getContexts({ + contextTypes: [chrome.runtime.ContextType.OFFSCREEN_DOCUMENT], + documentUrls: [documentUrl], + }) + if (contexts.length === 0) { + await chrome.offscreen.createDocument({ + url: "src/offscreen_clipboard.html", + reasons: [chrome.offscreen.Reason.CLIPBOARD], + justification: "Read clipboard text without opening a visible window", + }) + } + documentReady = true + return await chrome.runtime.sendMessage({ + target: "offscreen-clipboard", + command: "read", + }) + } catch (error) { + return { + data: "", + err: error instanceof Error ? error.message : String(error), + } + } finally { + if (documentReady) { + await chrome.offscreen.closeDocument().catch((error) => { + console.warn("Failed to close clipboard document:", error) + }) + } + } +} + +export function readOffscreenClipboard(): Promise { + // Only one offscreen document may exist. Serialize its complete lifecycle. + const read = pendingRead.then(readOnce) + pendingRead = read.then( + () => undefined, + () => undefined, + ) + return read +} diff --git a/packages/extension/src/services/pageAction/__tests__/dispatcher.input.test.ts b/packages/extension/src/services/pageAction/__tests__/dispatcher.input.test.ts new file mode 100644 index 000000000..c31bc8cb3 --- /dev/null +++ b/packages/extension/src/services/pageAction/__tests__/dispatcher.input.test.ts @@ -0,0 +1,44 @@ +import { afterEach, describe, expect, it } from "vitest" +import { PageActionDispatcher } from "../dispatcher" +import { SelectorType, PAGE_ACTION_EVENT } from "@/const" + +const codeText = ` const contexts = await chrome.runtime.getContexts({ + contextTypes: [chrome.runtime.ContextType.OFFSCREEN_DOCUMENT], + documentUrls: [documentUrl], + })` + +const inputCases = [ + { tag: "textarea", text: codeText }, + { + tag: "input", + text: "[KeyA] [Enter] {Enter} {Control>} [] [[ {{ [ 日本語 \\", + }, +] as const + +describe("PageActionDispatcher literal text input", () => { + afterEach(() => document.body.replaceChildren()) + + for (const source of ["Clipboard", "SelectedText", "literal"] as const) { + for (const { tag, text } of inputCases) { + it(`PDI-LITERAL-${source}-${tag}: 正常系: 括弧を含む${source}を${tag}へそのまま入力する`, async () => { + const element = document.createElement(tag) + element.id = "target" + document.body.append(element) + + const result = await PageActionDispatcher.input({ + type: PAGE_ACTION_EVENT.input, + label: "Target", + selector: "#target", + selectorType: SelectorType.css, + value: source === "literal" ? text : `{{${source}}}`, + clipboardText: source === "Clipboard" ? text : "", + selectedText: source === "SelectedText" ? text : "", + srcUrl: "https://example.com/", + }) + + expect(result).toEqual([true]) + expect(element.value).toBe(text) + }) + } + } +}) diff --git a/packages/extension/src/services/pageAction/dispatcher.ts b/packages/extension/src/services/pageAction/dispatcher.ts index 85339ea9d..b1e455590 100644 --- a/packages/extension/src/services/pageAction/dispatcher.ts +++ b/packages/extension/src/services/pageAction/dispatcher.ts @@ -139,7 +139,8 @@ export const PageActionDispatcher = { if (param.clearBefore) clearInput(element) await inputContentEditable(element, value, 10, null) } else { - value = value.replace(/{/g, "{{") // escape + // Treat input text literally instead of user-event keyboard descriptors. + value = value.replaceAll("{", "{{").replaceAll("[", "[[") const isTextControl = element instanceof HTMLInputElement || element instanceof HTMLTextAreaElement diff --git a/packages/extension/vite.config.ts b/packages/extension/vite.config.ts index 1a101b89a..0f65c1e25 100644 --- a/packages/extension/vite.config.ts +++ b/packages/extension/vite.config.ts @@ -172,6 +172,7 @@ export default defineConfig(({ mode }) => { rollupOptions: { input: { clipboard: "src/clipboard.html", + offscreenClipboard: "src/offscreen_clipboard.html", onboarding: "src/onboarding_page.html", }, }, From 4a1fffa65158c04712c0c4a25f43278d34cdf86a Mon Sep 17 00:00:00 2001 From: ujiro99 Date: Tue, 20 Oct 2026 13:08:05 +0900 Subject: [PATCH 12/14] =?UTF-8?q?refactor:=20=E3=82=AF=E3=83=AA=E3=83=83?= =?UTF-8?q?=E3=83=97=E3=83=9C=E3=83=BC=E3=83=89=E8=AA=AD=E3=81=BF=E5=8F=96?= =?UTF-8?q?=E3=82=8A=E3=82=92=E5=86=8D=E5=88=A9=E7=94=A8=E3=81=99=E3=82=8B?= =?UTF-8?q?Offscreen=E3=81=AB=E7=B5=B1=E4=B8=80?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/design/clipboard.md | 36 +++ docs/test/e2e-test-spec.md | 5 +- packages/extension/e2e/shortcut.spec.ts | 45 +++- packages/extension/manifest.json | 1 + packages/extension/src/clipboard.html | 10 - packages/extension/src/clipboard.ts | 47 ---- .../__tests__/chrome.openPopupWindow.test.ts | 108 ++++++++ .../services/__tests__/chrome.openTab.test.ts | 55 ++-- .../__tests__/offscreenClipboard.test.ts | 70 +++-- packages/extension/src/services/chrome.ts | 250 ++++-------------- packages/extension/src/services/ipc.ts | 1 - .../src/services/offscreenClipboard.ts | 11 +- packages/extension/vite.config.ts | 1 - 13 files changed, 302 insertions(+), 338 deletions(-) create mode 100644 docs/design/clipboard.md delete mode 100644 packages/extension/src/clipboard.html delete mode 100644 packages/extension/src/clipboard.ts create mode 100644 packages/extension/src/services/__tests__/chrome.openPopupWindow.test.ts diff --git a/docs/design/clipboard.md b/docs/design/clipboard.md new file mode 100644 index 000000000..eae5b4176 --- /dev/null +++ b/docs/design/clipboard.md @@ -0,0 +1,36 @@ +# クリップボード読み取り + +Chrome 116相当以降を対象とし、Service Workerからの読み取りは +`readClipboard()` → `readOffscreenClipboard()` に統一する。 +`clipboard.html` による表示タブ・ウィンドウでの読み取りは使用しない。 + +## 作成コストと再利用 + +- 最初の読み取りでOffscreen Documentを遅延作成する。 +- 読み取り後もドキュメントを維持し、次回以降は同じドキュメントを再利用する。 +- 読み取りは直列化し、同時要求による二重作成や結果の混在を防ぐ。 +- Service Worker再起動時も `runtime.getContexts()` で既存ドキュメントを検出する。 +- Chromeによってドキュメントが破棄された場合は、次の要求で再作成する。 +- textareaの内容は読み取り直後に消去し、クリップボード本文を保持しない。 + +Offscreen Documentを表示先のタブ・ウィンドウへ転用するAPIはないため、 +従来の「読み取り画面を表示先として転用する」最適化はそのまま移植できない。 +代わりに、読み取り側のドキュメントを要求間で再利用する設計とする。 + +従来は、表示先タブ・ウィンドウで読み取った後、その画面を目的URLへ遷移させていた。 +新方式では読み取り用のドキュメントを繰り返し再利用し、表示先は完成したURLで +一度だけ作成する。初回は非表示ドキュメントの作成待ちが必要だが、次回以降は +読み取りごとの作成・破棄と、表示先での中間ページの読み込みを避けられる。 +ブラウザによるレンダラープロセスの割り当ては制御しないため、 +初回を含めて常に高速になるとは保証しない。 + +## 読み取りAPI + +Offscreen Documentはフォーカスできない。検証環境では +`navigator.clipboard.readText()` は存在するものの、 +`Document is not focused` で失敗したため、`clipboardRead` 権限と +textareaへの `document.execCommand("paste")` を使用する。 +APIを将来変更する際は、非フォーカス状態での読み取りが成立することを確認する。 + +- [Offscreen API](https://developer.chrome.com/docs/extensions/reference/api/offscreen) +- [runtime.getContexts](https://developer.chrome.com/docs/extensions/reference/api/runtime#method-getContexts) diff --git a/docs/test/e2e-test-spec.md b/docs/test/e2e-test-spec.md index 0826539dd..832b66ffc 100644 --- a/docs/test/e2e-test-spec.md +++ b/docs/test/e2e-test-spec.md @@ -1091,7 +1091,10 @@ TypeScriptコードを設定する。`[KeyA]`、`{Enter}`、`[]`、`[[`、`{{` バックグラウンドタブでは `readClipboard()` がOffscreen Documentで読み取り、 結果タブを非アクティブで作成する。E2E-107／110ではウィンドウが新規作成されないこと、 結果タブの非アクティブ状態と元ページのフォーカス維持も検証する。 -Offscreen API非対応環境では従来の一時ウィンドウ方式を使用する。 +全表示モードでOffscreenに統一し、Chrome 116相当以降を対応範囲とする。 +E2E-107ではクリップボードを書き換えて再実行し、最新の入力値と +Offscreen Documentの `documentId` が同一であること（再利用）も検証する。 +設計の詳細は [クリップボード読み取り](../design/clipboard.md) を参照。 | テストID | 選択状態 | 入力ステップ | 表示方法 | 期待する入力値の変数部分 | | -------- | -------- | ------------------ | -------------------- | ------------------------------------------ | diff --git a/packages/extension/e2e/shortcut.spec.ts b/packages/extension/e2e/shortcut.spec.ts index 6b4401b95..dc994f5e1 100644 --- a/packages/extension/e2e/shortcut.spec.ts +++ b/packages/extension/e2e/shortcut.spec.ts @@ -277,17 +277,7 @@ for (const { id, openMode, variable, hasSelection } of pageActionCases) { }) } - // Popup mode may first load the clipboard page before the destination. - const resultPromise = context.waitForEvent("page", { - predicate: async (candidate) => { - try { - await candidate.waitForURL(`${targetUrl}?shortcutPageAction=${id}`) - return true - } catch { - return false - } - }, - }) + const resultPromise = context.waitForEvent("page") await runShortcut(context) const result = await resultPromise await expect(result).toHaveURL(`${targetUrl}?shortcutPageAction=${id}`) @@ -317,6 +307,39 @@ for (const { id, openMode, variable, hasSelection } of pageActionCases) { expect(createdWindowCount).toBe(0) expect(resultTab.active).toBe(false) expect(await page.evaluate(() => document.hasFocus())).toBe(true) + if (id === "E2E-107") { + const worker = context.serviceWorkers()[0] + const getDocumentId = () => + worker.evaluate(async () => { + const contexts = await chrome.runtime.getContexts({ + contextTypes: ["OFFSCREEN_DOCUMENT"], + }) + return contexts[0]?.documentId + }) + const documentId = await getDocumentId() + expect(documentId).toBeTruthy() + const nextText = "Updated clipboard [second] 日本語" + await page.evaluate( + (text) => navigator.clipboard.writeText(text), + nextText, + ) + const nextResultPromise = context.waitForEvent("page") + await runShortcut(context) + const nextResult = await nextResultPromise + await expect(nextResult.locator("textarea")).toHaveValue( + `Input: ${nextText}`, + { timeout: 10_000 }, + ) + expect(await getDocumentId()).toBe(documentId) + expect(await page.evaluate(() => document.hasFocus())).toBe(true) + expect( + await worker.evaluate( + () => + (globalThis as typeof globalThis & { createdWindowCount: number }) + .createdWindowCount, + ), + ).toBe(0) + } } }) } diff --git a/packages/extension/manifest.json b/packages/extension/manifest.json index 678556e11..308026a95 100644 --- a/packages/extension/manifest.json +++ b/packages/extension/manifest.json @@ -1,5 +1,6 @@ { "manifest_version": 3, + "minimum_chrome_version": "116", "name": "__MSG_extName__", "description": "__MSG_extDescription__", "version": "1.3.2", diff --git a/packages/extension/src/clipboard.html b/packages/extension/src/clipboard.html deleted file mode 100644 index 1a4dd1449..000000000 --- a/packages/extension/src/clipboard.html +++ /dev/null @@ -1,10 +0,0 @@ - - - - - Selection Command - - - - - diff --git a/packages/extension/src/clipboard.ts b/packages/extension/src/clipboard.ts deleted file mode 100644 index 4d1628e05..000000000 --- a/packages/extension/src/clipboard.ts +++ /dev/null @@ -1,47 +0,0 @@ -import { ClipboardResult, ServiceWorkerCommand } from "@/services/ipc" - -/** - * Read text from clipboard with retry mechanism - * @param maxRetries Maximum number of retry attempts (default: 3) - * @param delayMs Delay between retries in milliseconds (default: 100) - * @returns Promise Clipboard content or empty string if all attempts fail - */ -const readClipboardWithRetry = async ( - maxRetries = 3, - delayMs = 100, -): Promise => { - for (let i = 0; i < maxRetries; i++) { - try { - return { data: await navigator.clipboard.readText() } - } catch (error) { - console.warn( - `Retry to read clipboard (attempt ${i + 1}/${maxRetries}):`, - error, - ) - if (i === maxRetries - 1) { - return { data: undefined, err: `Retry limit exceeded. [${maxRetries}]` } - } - await new Promise((resolve) => setTimeout(resolve, delayMs)) - } - } - return { data: undefined, err: "Out of retries." } -} - -const port = chrome.runtime.connect({ name: "clipboard" }) -port.onDisconnect.addListener(function (port) { - console.warn("onDisconnect", port.name) -}) - -readClipboardWithRetry() - .then((ret: ClipboardResult) => { - port.postMessage({ command: ServiceWorkerCommand.setClipboard, data: ret }) - }) - .catch((error: Error) => { - port.postMessage({ - command: ServiceWorkerCommand.setClipboard, - data: { data: undefined, err: error.message }, - }) - }) - .finally(() => { - port.disconnect() - }) diff --git a/packages/extension/src/services/__tests__/chrome.openPopupWindow.test.ts b/packages/extension/src/services/__tests__/chrome.openPopupWindow.test.ts new file mode 100644 index 000000000..b3f3458b0 --- /dev/null +++ b/packages/extension/src/services/__tests__/chrome.openPopupWindow.test.ts @@ -0,0 +1,108 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest" +import { openPopupWindow } from "../chrome" +import { readOffscreenClipboard } from "../offscreenClipboard" +import { Ipc } from "../ipc" +import { POPUP_TYPE, WINDOW_STATE } from "@/const" + +vi.mock("../offscreenClipboard", () => ({ readOffscreenClipboard: vi.fn() })) +vi.mock("../screen", () => ({ + getScreenSize: vi + .fn() + .mockResolvedValue({ left: 0, top: 0, width: 1280, height: 800 }), +})) +vi.mock("../windowStackManager", () => ({ + WindowStackManager: { addWindows: vi.fn().mockResolvedValue(undefined) }, +})) + +const params = { + commandId: "test", + url: { + searchUrl: "https://example.com/?q=%s", + selectionText: "", + selectionFromClipboard: true, + }, + type: POPUP_TYPE.POPUP, + top: 10, + left: 20, + width: 600, + height: 700, +} + +describe("openPopupWindow", () => { + beforeEach(() => { + vi.clearAllMocks() + vi.mocked(readOffscreenClipboard).mockResolvedValue({ + data: "clipboard text", + }) + vi.stubGlobal("chrome", { + ...chrome, + windows: { + ...chrome.windows, + getCurrent: vi.fn().mockResolvedValue({ id: 1, incognito: true }), + create: vi.fn().mockResolvedValue({ id: 2, tabs: [{ id: 20 }] }), + update: vi.fn().mockResolvedValue(undefined), + }, + declarativeNetRequest: { + ...chrome.declarativeNetRequest, + RuleActionType: { MODIFY_HEADERS: "modifyHeaders" }, + HeaderOperation: { REMOVE: "remove", SET: "set" }, + ResourceType: { MAIN_FRAME: "main_frame" }, + getSessionRules: vi.fn().mockResolvedValue([]), + updateSessionRules: vi.fn().mockResolvedValue(undefined), + }, + }) + }) + afterEach(() => vi.unstubAllGlobals()) + + it("COP-01: 正常系: 完成したURLで一度だけ作成し位置・サイズ・シークレット設定を保持する", async () => { + expect(await openPopupWindow(params)).toEqual({ + tabId: 20, + clipboardText: "clipboard text", + }) + expect(chrome.windows.create).toHaveBeenCalledExactlyOnceWith({ + url: "https://example.com/?q=clipboard+text", + type: "popup", + top: 10, + left: 20, + width: 600, + height: 700, + state: undefined, + incognito: true, + }) + expect(chrome.tabs.update).not.toHaveBeenCalled() + }) + + it("COP-02: 正常系: fullscreenでは座標とサイズを渡さず作成後にも状態を適用する", async () => { + await openPopupWindow({ ...params, windowState: WINDOW_STATE.FULLSCREEN }) + expect(chrome.windows.create).toHaveBeenCalledWith( + expect.objectContaining({ + state: "fullscreen", + top: undefined, + left: undefined, + width: undefined, + height: undefined, + }), + ) + expect(chrome.windows.update).toHaveBeenCalledWith(2, { + state: "fullscreen", + }) + }) + + it("COP-03: 異常系: 読み取り失敗時にも表示先を作成してエラーを通知する", async () => { + vi.mocked(readOffscreenClipboard).mockResolvedValue({ + data: "", + err: "Read failed", + }) + const connection = vi.spyOn(Ipc, "ensureConnection").mockResolvedValue() + const sendTab = vi.spyOn(Ipc, "sendTab").mockResolvedValue(undefined) + try { + await openPopupWindow(params) + expect(chrome.windows.create).toHaveBeenCalledOnce() + expect(connection).toHaveBeenCalledWith(20) + expect(sendTab).toHaveBeenCalledWith(20, "showToast", expect.any(Object)) + } finally { + connection.mockRestore() + sendTab.mockRestore() + } + }) +}) diff --git a/packages/extension/src/services/__tests__/chrome.openTab.test.ts b/packages/extension/src/services/__tests__/chrome.openTab.test.ts index df487358f..4c9ec333a 100644 --- a/packages/extension/src/services/__tests__/chrome.openTab.test.ts +++ b/packages/extension/src/services/__tests__/chrome.openTab.test.ts @@ -1,6 +1,9 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest" import { openTab } from "../chrome" -import { Ipc, ServiceWorkerCommand } from "../ipc" +import { Ipc } from "../ipc" +import { readOffscreenClipboard } from "../offscreenClipboard" + +vi.mock("../offscreenClipboard", () => ({ readOffscreenClipboard: vi.fn() })) const sourceTab = { id: 10, windowId: 1, index: 2 } as chrome.tabs.Tab const resultTab = { id: 20, windowId: 1 } as chrome.tabs.Tab @@ -10,26 +13,6 @@ const url = { selectionFromClipboard: true, } -function provideClipboard(data: string, err?: string) { - vi.mocked(chrome.runtime.onConnect.addListener).mockImplementation( - (listener) => { - const port = { - sender: { tab: { id: 30 } }, - onMessage: { - addListener: (onMessage: (message: unknown) => void) => { - onMessage({ - command: ServiceWorkerCommand.setClipboard, - data: { data, err }, - }) - }, - removeListener: vi.fn(), - }, - } as unknown as chrome.runtime.Port - listener(port) - }, - ) -} - describe("openTab", () => { afterEach(() => vi.unstubAllGlobals()) beforeEach(() => { @@ -48,14 +31,16 @@ describe("openTab", () => { remove: vi.fn().mockResolvedValue(undefined), }, }) - provideClipboard("clipboard text") + vi.mocked(readOffscreenClipboard).mockResolvedValue({ + data: "clipboard text", + }) }) - it("COT-01: 正常系: クリップボードを一時ウィンドウで読み非アクティブタブに渡す", async () => { + it("COT-01: 正常系: Offscreenで読み非アクティブタブを直接作成する", async () => { const result = await openTab({ url, active: false }) - expect(chrome.windows.create).toHaveBeenCalledOnce() - expect(chrome.windows.remove).toHaveBeenCalledWith(3) + expect(readOffscreenClipboard).toHaveBeenCalledOnce() + expect(chrome.windows.create).not.toHaveBeenCalled() expect(chrome.tabs.create).toHaveBeenCalledExactlyOnceWith({ url: "https://example.com/?q=clipboard+text", active: false, @@ -66,12 +51,25 @@ describe("openTab", () => { expect(result).toEqual({ tabId: 20, clipboardText: "clipboard text" }) }) + it("COT-01-a: 正常系: アクティブタブも完成したURLで一度だけ作成する", async () => { + await openTab({ url, active: true }) + expect(readOffscreenClipboard).toHaveBeenCalledOnce() + expect(chrome.tabs.create).toHaveBeenCalledExactlyOnceWith( + expect.objectContaining({ + url: "https://example.com/?q=clipboard+text", + active: true, + }), + ) + expect(chrome.tabs.update).not.toHaveBeenCalled() + }) + it("COT-02: 正常系: クリップボード不要なら一時ウィンドウを開かない", async () => { await openTab({ url: { ...url, selectionText: "selected", selectionFromClipboard: false }, active: false, }) + expect(readOffscreenClipboard).not.toHaveBeenCalled() expect(chrome.windows.create).not.toHaveBeenCalled() expect(chrome.tabs.create).toHaveBeenCalledWith( expect.objectContaining({ @@ -82,7 +80,10 @@ describe("openTab", () => { }) it("COT-03: 異常系: 読み取り失敗時も非アクティブで開きエラーを通知する", async () => { - provideClipboard("", "Permission denied") + vi.mocked(readOffscreenClipboard).mockResolvedValue({ + data: "", + err: "Permission denied", + }) const connection = vi.spyOn(Ipc, "ensureConnection").mockResolvedValue() const sendTab = vi.spyOn(Ipc, "sendTab").mockResolvedValue(undefined) try { @@ -92,7 +93,7 @@ describe("openTab", () => { ) expect(connection).toHaveBeenCalledWith(20) expect(sendTab).toHaveBeenCalledWith(20, "showToast", expect.any(Object)) - expect(chrome.windows.remove).toHaveBeenCalledWith(3) + expect(chrome.windows.create).not.toHaveBeenCalled() expect(result).toEqual({ tabId: 20, clipboardText: "" }) } finally { connection.mockRestore() diff --git a/packages/extension/src/services/__tests__/offscreenClipboard.test.ts b/packages/extension/src/services/__tests__/offscreenClipboard.test.ts index b8826ffc9..8ece136aa 100644 --- a/packages/extension/src/services/__tests__/offscreenClipboard.test.ts +++ b/packages/extension/src/services/__tests__/offscreenClipboard.test.ts @@ -2,7 +2,6 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest" import { readOffscreenClipboard } from "../offscreenClipboard" const createDocument = vi.fn() -const closeDocument = vi.fn() const getContexts = vi.fn() const sendMessage = vi.fn() @@ -10,8 +9,11 @@ describe("readOffscreenClipboard", () => { beforeEach(() => { vi.resetAllMocks() createDocument.mockResolvedValue(undefined) - closeDocument.mockResolvedValue(undefined) - getContexts.mockResolvedValue([]) + let documentExists = false + getContexts.mockImplementation(async () => (documentExists ? [{}] : [])) + createDocument.mockImplementation(async () => { + documentExists = true + }) sendMessage.mockResolvedValue({ data: "日本語 clipboard" }) vi.stubGlobal("chrome", { ...chrome, @@ -24,14 +26,13 @@ describe("readOffscreenClipboard", () => { }, offscreen: { createDocument, - closeDocument, Reason: { CLIPBOARD: "CLIPBOARD" }, }, }) }) afterEach(() => vi.unstubAllGlobals()) - it("OC-01: 正常系: 非表示ドキュメントで読み取り後に閉じる", async () => { + it("OC-01: 正常系: 非表示ドキュメントを再利用し毎回最新の文字列を返す", async () => { await expect(readOffscreenClipboard()).resolves.toEqual({ data: "日本語 clipboard", }) @@ -41,45 +42,58 @@ describe("readOffscreenClipboard", () => { reasons: ["CLIPBOARD"], }), ) - expect(closeDocument).toHaveBeenCalledOnce() + sendMessage.mockResolvedValue({ data: "updated" }) + await expect(readOffscreenClipboard()).resolves.toEqual({ data: "updated" }) + expect(createDocument).toHaveBeenCalledOnce() + expect(sendMessage).toHaveBeenCalledTimes(2) }) it("OC-02: 正常系: 再起動後に残ったドキュメントを再利用する", async () => { getContexts.mockResolvedValue([{}]) await readOffscreenClipboard() expect(createDocument).not.toHaveBeenCalled() - expect(closeDocument).toHaveBeenCalledOnce() }) - it("OC-03: 異常系: 読み取り失敗時も閉じてエラーを返す", async () => { + it("OC-03: 異常系: 読み取り失敗後も次の要求で読み取れる", async () => { sendMessage.mockRejectedValue(new Error("Read failed")) await expect(readOffscreenClipboard()).resolves.toEqual({ data: "", err: "Read failed", }) - expect(closeDocument).toHaveBeenCalledOnce() + sendMessage.mockResolvedValue({ data: "recovered" }) + await expect(readOffscreenClipboard()).resolves.toEqual({ + data: "recovered", + }) + expect(createDocument).toHaveBeenCalledOnce() }) - it("OC-04: 正常系: 同時要求では前のドキュメントを閉じてから次を作る", async () => { - const events: string[] = [] - createDocument.mockImplementation(async () => { - events.push("create") - }) - sendMessage.mockImplementation(async () => { - events.push("read") - return { data: "text" } + it("OC-04: 正常系: 同時要求でも一度だけ作成して異なる値を順に返す", async () => { + sendMessage + .mockResolvedValueOnce({ data: "first" }) + .mockResolvedValueOnce({ data: "second" }) + const results = await Promise.all([ + readOffscreenClipboard(), + readOffscreenClipboard(), + ]) + expect(results).toEqual([{ data: "first" }, { data: "second" }]) + expect(createDocument).toHaveBeenCalledOnce() + }) + + it("OC-05: 正常系: ドキュメントが破棄されたら再作成する", async () => { + await readOffscreenClipboard() + getContexts.mockResolvedValueOnce([]) + await readOffscreenClipboard() + expect(createDocument).toHaveBeenCalledTimes(2) + }) + + it("OC-06: 異常系: 作成失敗後の要求で再試行できる", async () => { + createDocument.mockRejectedValueOnce(new Error("Creation failed")) + await expect(readOffscreenClipboard()).resolves.toEqual({ + data: "", + err: "Creation failed", }) - closeDocument.mockImplementation(async () => { - events.push("close") + await expect(readOffscreenClipboard()).resolves.toEqual({ + data: "日本語 clipboard", }) - await Promise.all([readOffscreenClipboard(), readOffscreenClipboard()]) - expect(events).toEqual([ - "create", - "read", - "close", - "create", - "read", - "close", - ]) }) }) diff --git a/packages/extension/src/services/chrome.ts b/packages/extension/src/services/chrome.ts index 5911468c7..98c124ada 100644 --- a/packages/extension/src/services/chrome.ts +++ b/packages/extension/src/services/chrome.ts @@ -6,11 +6,7 @@ import { POPUP_OFFSET, POPUP_TYPE, WINDOW_STATE } from "@/const" import { PopupOption } from "@/services/option/defaultSettings" import { ServiceWorkerData } from "@/services/serviceWorkerData" import { WindowStackManager } from "@/services/windowStackManager" -import { - ServiceWorkerCommand, - ClipboardResult, - TabCommand, -} from "@/services/ipc" +import { TabCommand } from "@/services/ipc" import { getScreenSize } from "@/services/screen" import { Ipc } from "@/services/ipc" import { t } from "@/services/i18n" @@ -136,17 +132,6 @@ export type OpenTabProps = { active: boolean } -type ReadClipboardParam = Omit & { - incognito: boolean - state?: "fullscreen" | "maximized" -} - -type ReadClipboardResult = { - clipboardText: string - window: chrome.windows.Window - err?: string -} - type OpenResult = { tabId: number | undefined clipboardText: string @@ -340,106 +325,13 @@ const updateRules = async (tabIds: number[]) => { }) } -/** - * Read clipboard content from a tab - * @param {number} tabId - The ID of the tab to read clipboard from - * @returns {Promise} The clipboard content - */ -const readClipboardContent = async ( - tabId: number, -): Promise => { - try { - const result = await new Promise((resolve) => { - const onConnect = (port: chrome.runtime.Port) => { - if (port.sender?.tab?.id !== tabId) { - return - } - const onMessage = (msg: { command: string; data: ClipboardResult }) => { - if (msg.command === ServiceWorkerCommand.setClipboard) { - port.onMessage.removeListener(onMessage) - chrome.runtime.onConnect.removeListener(onConnect) - resolve(msg.data) - } - } - port.onMessage.addListener(onMessage) - } - chrome.runtime.onConnect.addListener(onConnect) - if (chrome.runtime.lastError) { - throw new Error(chrome.runtime.lastError.message) - } - }) - - return result - } catch (e) { - console.error(e) - return { data: "", err: e instanceof Error ? e.message : "Unknown error" } - } -} - -const openWindowAndReadClipboard = async ( - param: ReadClipboardParam, -): Promise => { - const usesWindowState = - param.state === "fullscreen" || param.state === "maximized" - const w = await chrome.windows.create({ - url: chrome.runtime.getURL("src/clipboard.html"), - focused: true, - type: param.type, - width: usesWindowState ? undefined : param.width, - height: usesWindowState ? undefined : param.height, - left: usesWindowState ? undefined : param.left, - top: usesWindowState ? undefined : param.top, - state: param.state, - incognito: param.incognito, - }) - - const tab = w?.tabs?.[0] - if (!tab?.id || !w?.id) { - throw new Error("Failed to create clipboard window") - } - const result = await readClipboardContent(tab.id) - - return { - window: w, - clipboardText: result.data ?? "", - err: result.err, - } -} - -/** Read clipboard text without a visible window when Offscreen API is available. */ +/** Read clipboard text through the shared offscreen document. */ export const readClipboard = async (): Promise<{ clipboardText: string err?: string }> => { - if ( - typeof chrome.offscreen?.createDocument === "function" && - typeof chrome.runtime.getContexts === "function" - ) { - const result = await readOffscreenClipboard() - return { clipboardText: result.data ?? "", err: result.err } - } - - // Preserve compatibility with browsers without the Offscreen API. - const w = await chrome.windows.create({ - url: chrome.runtime.getURL("src/clipboard.html"), - focused: true, - type: "popup", - width: 1, - height: 1, - left: 0, - top: 0, - state: "normal", - }) - const tab = w?.tabs?.[0] - if (!tab?.id || !w?.id) { - throw new Error("Failed to create clipboard window") - } - const result = await readClipboardContent(tab.id) - await closeWindow(w.id, "readClipboard close window") - return { - clipboardText: result.data ?? "", - err: result.err, - } + const result = await readOffscreenClipboard() + return { clipboardText: result.data ?? "", err: result.err } } export const openPopupWindow = async ( @@ -474,68 +366,39 @@ export const openPopupWindow = async ( ? "maximized" : undefined - let window: chrome.windows.Window - let clipboardText = "" - - if (needsClipboard(url)) { - const result = await openWindowAndReadClipboard({ - commandId: param.commandId, - type, - width, - height, - top: at, - left: al, - state: windowState, - incognito: current.incognito, - }) - window = result.window - clipboardText = result.clipboardText - - try { - await chrome.tabs.update(window.tabs?.[0].id as number, { - url: toUrl(url, clipboardText), - }) - if (isFullscreen) { - // On macOS, even if you open with state: "fullscreen", - // it may not actually go fullscreen, so switch to fullscreen after opening. - await chrome.windows.update(window.id!, { state: "fullscreen" }) - } - } catch (e) { - console.error(e) + const clipboard = needsClipboard(url) + ? await readClipboard() + : { clipboardText: "", err: undefined } + const window = (await chrome.windows.create({ + url: toUrl(url, clipboard.clipboardText), + type, + width: usesWindowState ? undefined : width, + height: usesWindowState ? undefined : height, + top: usesWindowState ? undefined : at, + left: usesWindowState ? undefined : al, + state: windowState, + incognito: current.incognito, + }))! + try { + if (isFullscreen) { + // macOS may require reapplying fullscreen after window creation. + await chrome.windows.update(window.id!, { state: "fullscreen" }) } + } catch (e) { + console.error(e) + } - if (result.err) { - await Ipc.ensureConnection(window.tabs?.[0].id as number) - await Ipc.sendTab( - window.tabs?.[0].id as number, - TabCommand.showToast, - { - title: t("clipboard_error_title"), - description: t("clipboard_error_description"), - action: t("clipboard_error_action"), - }, - ) - } - } else { - window = (await chrome.windows.create({ - url: toUrl(url), - type, - width: usesWindowState ? undefined : width, - height: usesWindowState ? undefined : height, - top: usesWindowState ? undefined : at, - left: usesWindowState ? undefined : al, - state: windowState, - incognito: current.incognito, - }))! - try { - if (isFullscreen) { - // On macOS, even if you open with state: "fullscreen", - // it may not actually go fullscreen, so switch to fullscreen after opening. - await chrome.windows.update(window.id!, { state: "fullscreen" }) - } - } catch (e) { - console.error(e) - } + if (clipboard.err) { + await Ipc.ensureConnection(window.tabs?.[0].id as number) + await Ipc.sendTab( + window.tabs?.[0].id as number, + TabCommand.showToast, + { + title: t("clipboard_error_title"), + description: t("clipboard_error_description"), + action: t("clipboard_error_action"), + }, + ) } await updateServiceWorkerData([window], param.commandId, current.id, type) @@ -545,7 +408,7 @@ export const openPopupWindow = async ( return { tabId: window.tabs?.[0]?.id, - clipboardText, + clipboardText: clipboard.clipboardText, } } @@ -609,36 +472,17 @@ export const openTab = async (param: OpenTabProps): Promise => { console.warn("Failed to get current tab:", e) } - const clipboardRequired = needsClipboard(url) - let tab: chrome.tabs.Tab - let result: ClipboardResult = { data: "" } - - if (clipboardRequired && active) { - tab = await chrome.tabs.create({ - url: chrome.runtime.getURL("src/clipboard.html"), - active: true, - windowId: currentTab?.windowId, - index: currentTab?.index !== undefined ? currentTab.index + 1 : undefined, - }) - result = await readClipboardContent(tab.id as number) - await chrome.tabs.update(tab.id as number, { - url: toUrl(url, result.data ?? ""), - }) - } else { - if (clipboardRequired) { - // Read separately so the destination tab stays inactive. - const clipboard = await readClipboard() - result = { data: clipboard.clipboardText, err: clipboard.err } - } - tab = await chrome.tabs.create({ - url: toUrl(url, result.data ?? ""), - active, - windowId: currentTab?.windowId, - index: currentTab?.index !== undefined ? currentTab.index + 1 : undefined, - }) - } + const clipboard = needsClipboard(url) + ? await readClipboard() + : { clipboardText: "", err: undefined } + const tab = await chrome.tabs.create({ + url: toUrl(url, clipboard.clipboardText), + active, + windowId: currentTab?.windowId, + index: currentTab?.index !== undefined ? currentTab.index + 1 : undefined, + }) - if (result.err) { + if (clipboard.err) { await Ipc.ensureConnection(tab.id as number) await Ipc.sendTab(tab.id as number, TabCommand.showToast, { title: t("clipboard_error_title"), @@ -649,7 +493,7 @@ export const openTab = async (param: OpenTabProps): Promise => { return { tabId: tab.id as number, - clipboardText: result.data ?? "", + clipboardText: clipboard.clipboardText, } } diff --git a/packages/extension/src/services/ipc.ts b/packages/extension/src/services/ipc.ts index d30e6d4f8..8cb5e4145 100644 --- a/packages/extension/src/services/ipc.ts +++ b/packages/extension/src/services/ipc.ts @@ -62,7 +62,6 @@ export enum ServiceWorkerCommand { toggleStar = "toggleStar", getTabId = "getTabId", getActiveTabId = "getActiveTabId", - setClipboard = "setClipboard", closeTab = "closeTab", resolveIconColors = "resolveIconColors", // Hub diff --git a/packages/extension/src/services/offscreenClipboard.ts b/packages/extension/src/services/offscreenClipboard.ts index ebfdc781e..c0f04e855 100644 --- a/packages/extension/src/services/offscreenClipboard.ts +++ b/packages/extension/src/services/offscreenClipboard.ts @@ -4,12 +4,12 @@ let pendingRead: Promise = Promise.resolve() async function readOnce(): Promise { const documentUrl = chrome.runtime.getURL("src/offscreen_clipboard.html") - let documentReady = false try { const contexts = await chrome.runtime.getContexts({ contextTypes: [chrome.runtime.ContextType.OFFSCREEN_DOCUMENT], documentUrls: [documentUrl], }) + // Recheck after worker restarts or if Chrome discarded the document. if (contexts.length === 0) { await chrome.offscreen.createDocument({ url: "src/offscreen_clipboard.html", @@ -17,7 +17,6 @@ async function readOnce(): Promise { justification: "Read clipboard text without opening a visible window", }) } - documentReady = true return await chrome.runtime.sendMessage({ target: "offscreen-clipboard", command: "read", @@ -27,17 +26,11 @@ async function readOnce(): Promise { data: "", err: error instanceof Error ? error.message : String(error), } - } finally { - if (documentReady) { - await chrome.offscreen.closeDocument().catch((error) => { - console.warn("Failed to close clipboard document:", error) - }) - } } } export function readOffscreenClipboard(): Promise { - // Only one offscreen document may exist. Serialize its complete lifecycle. + // Serialize reads and creation; keep the document for subsequent requests. const read = pendingRead.then(readOnce) pendingRead = read.then( () => undefined, diff --git a/packages/extension/vite.config.ts b/packages/extension/vite.config.ts index 0f65c1e25..78846ea11 100644 --- a/packages/extension/vite.config.ts +++ b/packages/extension/vite.config.ts @@ -171,7 +171,6 @@ export default defineConfig(({ mode }) => { emptyOutDir: !isWatchMode, // prevent deleting the dist folder when running in watch mode rollupOptions: { input: { - clipboard: "src/clipboard.html", offscreenClipboard: "src/offscreen_clipboard.html", onboarding: "src/onboarding_page.html", }, From 632670ab472d2ddf5a0e8e877910a1294f458d9f Mon Sep 17 00:00:00 2001 From: ujiro99 Date: Tue, 20 Oct 2026 13:25:52 +0900 Subject: [PATCH 13/14] =?UTF-8?q?fix:=20Offscreen=E3=82=AF=E3=83=AA?= =?UTF-8?q?=E3=83=83=E3=83=97=E3=83=9C=E3=83=BC=E3=83=89=E8=AA=AD=E3=81=BF?= =?UTF-8?q?=E5=8F=96=E3=82=8A=E3=81=AE=E5=A4=B1=E6=95=97=E6=99=82=E5=87=A6?= =?UTF-8?q?=E7=90=86=E3=82=92=E5=BC=B7=E5=8C=96?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 読み取り失敗時に最大3回まで再試行する - Offscreenからの不正な応答をエラーとして扱う - textareaにフォーカスできない場合はエラーにする - 作成時の単一ドキュメント制約エラーでは既存ドキュメントを使う - バックグラウンドタブで開く場合は実行元タブへエラーを通知する - エラートーストとクリップボード読み取りの重複処理を共通化する Co-Authored-By: Claude Opus 5.5 --- docs/design/clipboard.md | 10 +++ packages/extension/src/offscreen_clipboard.ts | 4 + .../services/__tests__/chrome.openTab.test.ts | 23 +++++- .../__tests__/offscreenClipboard.test.ts | 39 ++++++++- packages/extension/src/services/chrome.ts | 46 +++++------ .../src/services/offscreenClipboard.ts | 80 +++++++++++++------ .../src/services/pageAction/serviceWorker.ts | 9 +-- 7 files changed, 152 insertions(+), 59 deletions(-) diff --git a/docs/design/clipboard.md b/docs/design/clipboard.md index eae5b4176..38d69afc4 100644 --- a/docs/design/clipboard.md +++ b/docs/design/clipboard.md @@ -12,6 +12,16 @@ Chrome 116相当以降を対象とし、Service Workerからの読み取りは - Service Worker再起動時も `runtime.getContexts()` で既存ドキュメントを検出する。 - Chromeによってドキュメントが破棄された場合は、次の要求で再作成する。 - textareaの内容は読み取り直後に消去し、クリップボード本文を保持しない。 +- 作成時に他のコンテキストが先に作成していた場合（単一ドキュメント制約のエラー）は、 + 既存ドキュメントをそのまま使用する。 + +## 失敗時の扱い + +- 読み取りに失敗した場合は50ms間隔で最大3回まで再試行する。 +- Offscreen Documentからの応答が `ClipboardResult` の形式でない場合はエラーとして扱う。 +- textareaにフォーカスできない場合は、空文字を成功として返さずエラーとする。 +- エラー時のトーストは、ユーザーが見ているタブへ表示する。 + バックグラウンドタブで開く場合は、表示先ではなく実行元のタブへ通知する。 Offscreen Documentを表示先のタブ・ウィンドウへ転用するAPIはないため、 従来の「読み取り画面を表示先として転用する」最適化はそのまま移植できない。 diff --git a/packages/extension/src/offscreen_clipboard.ts b/packages/extension/src/offscreen_clipboard.ts index 1db0aef54..1a6a7fc9a 100644 --- a/packages/extension/src/offscreen_clipboard.ts +++ b/packages/extension/src/offscreen_clipboard.ts @@ -15,6 +15,10 @@ chrome.runtime.onMessage.addListener((message, sender, respond) => { textarea.value = "" textarea.focus() textarea.select() + // Without focus, paste may report success while inserting nothing. + if (document.activeElement !== textarea) { + throw new Error("Failed to focus the clipboard textarea") + } // Async Clipboard requires document focus, which offscreen documents lack. if (!document.execCommand("paste")) { throw new Error("Failed to paste clipboard text") diff --git a/packages/extension/src/services/__tests__/chrome.openTab.test.ts b/packages/extension/src/services/__tests__/chrome.openTab.test.ts index 4c9ec333a..de10216f4 100644 --- a/packages/extension/src/services/__tests__/chrome.openTab.test.ts +++ b/packages/extension/src/services/__tests__/chrome.openTab.test.ts @@ -79,7 +79,7 @@ describe("openTab", () => { ) }) - it("COT-03: 異常系: 読み取り失敗時も非アクティブで開きエラーを通知する", async () => { + it("COT-03: 異常系: 読み取り失敗時は非アクティブで開き元のタブへエラーを通知する", async () => { vi.mocked(readOffscreenClipboard).mockResolvedValue({ data: "", err: "Permission denied", @@ -91,8 +91,8 @@ describe("openTab", () => { expect(chrome.tabs.create).toHaveBeenCalledWith( expect.objectContaining({ active: false }), ) - expect(connection).toHaveBeenCalledWith(20) - expect(sendTab).toHaveBeenCalledWith(20, "showToast", expect.any(Object)) + expect(connection).toHaveBeenCalledWith(10) + expect(sendTab).toHaveBeenCalledWith(10, "showToast", expect.any(Object)) expect(chrome.windows.create).not.toHaveBeenCalled() expect(result).toEqual({ tabId: 20, clipboardText: "" }) } finally { @@ -100,4 +100,21 @@ describe("openTab", () => { sendTab.mockRestore() } }) + + it("COT-04: 異常系: アクティブタブで開いた場合は新しいタブへエラーを通知する", async () => { + vi.mocked(readOffscreenClipboard).mockResolvedValue({ + data: "", + err: "Permission denied", + }) + const connection = vi.spyOn(Ipc, "ensureConnection").mockResolvedValue() + const sendTab = vi.spyOn(Ipc, "sendTab").mockResolvedValue(undefined) + try { + await openTab({ url, active: true }) + expect(connection).toHaveBeenCalledWith(20) + expect(sendTab).toHaveBeenCalledWith(20, "showToast", expect.any(Object)) + } finally { + connection.mockRestore() + sendTab.mockRestore() + } + }) }) diff --git a/packages/extension/src/services/__tests__/offscreenClipboard.test.ts b/packages/extension/src/services/__tests__/offscreenClipboard.test.ts index 8ece136aa..fbaf9d80d 100644 --- a/packages/extension/src/services/__tests__/offscreenClipboard.test.ts +++ b/packages/extension/src/services/__tests__/offscreenClipboard.test.ts @@ -86,14 +86,49 @@ describe("readOffscreenClipboard", () => { expect(createDocument).toHaveBeenCalledTimes(2) }) - it("OC-06: 異常系: 作成失敗後の要求で再試行できる", async () => { + it("OC-06: 異常系: 作成に一時失敗しても再試行して読み取る", async () => { createDocument.mockRejectedValueOnce(new Error("Creation failed")) + await expect(readOffscreenClipboard()).resolves.toEqual({ + data: "日本語 clipboard", + }) + expect(createDocument).toHaveBeenCalledTimes(2) + }) + + it("OC-07: 異常系: 一時的な読み取り失敗は再試行して読み取る", async () => { + sendMessage + .mockResolvedValueOnce({ data: "", err: "Busy" }) + .mockResolvedValueOnce({ data: "retried" }) + await expect(readOffscreenClipboard()).resolves.toEqual({ + data: "retried", + }) + expect(sendMessage).toHaveBeenCalledTimes(2) + }) + + it("OC-08: 異常系: 再試行上限に達したら最後のエラーを返す", async () => { + sendMessage.mockRejectedValue(new Error("Read failed")) await expect(readOffscreenClipboard()).resolves.toEqual({ data: "", - err: "Creation failed", + err: "Read failed", }) + expect(sendMessage).toHaveBeenCalledTimes(3) + }) + + it("OC-09: 異常系: 不正な応答はエラーとして扱う", async () => { + sendMessage.mockResolvedValue(undefined) + await expect(readOffscreenClipboard()).resolves.toEqual({ + data: "", + err: "Invalid response from offscreen document", + }) + }) + + it("OC-10: 異常系: 作成時の競合で既存ドキュメントがあればそのまま読み取る", async () => { + createDocument.mockRejectedValueOnce( + new Error("Only a single offscreen document may be created."), + ) await expect(readOffscreenClipboard()).resolves.toEqual({ data: "日本語 clipboard", }) + expect(createDocument).toHaveBeenCalledOnce() + expect(sendMessage).toHaveBeenCalledOnce() }) }) diff --git a/packages/extension/src/services/chrome.ts b/packages/extension/src/services/chrome.ts index 98c124ada..5f764efa1 100644 --- a/packages/extension/src/services/chrome.ts +++ b/packages/extension/src/services/chrome.ts @@ -1,4 +1,3 @@ -import { readOffscreenClipboard } from "./offscreenClipboard" import { sleep, toUrl, isOverBytes, needsClipboard } from "@/lib/utils" import type { ScreenSize } from "@/services/dom" import type { ShowToastParam, UrlParam, WindowLayer } from "@/types" @@ -8,6 +7,7 @@ import { ServiceWorkerData } from "@/services/serviceWorkerData" import { WindowStackManager } from "@/services/windowStackManager" import { TabCommand } from "@/services/ipc" import { getScreenSize } from "@/services/screen" +import { readOffscreenClipboard } from "@/services/offscreenClipboard" import { Ipc } from "@/services/ipc" import { t } from "@/services/i18n" import { @@ -334,6 +334,22 @@ export const readClipboard = async (): Promise<{ return { clipboardText: result.data ?? "", err: result.err } } +/** Read clipboard text only when the URL refers to the clipboard. */ +const readClipboardIfNeeded = async ( + url: string | UrlParam, +): Promise<{ clipboardText: string; err?: string }> => + needsClipboard(url) ? readClipboard() : { clipboardText: "" } + +/** Notify the user in the given tab that reading the clipboard failed. */ +export const showClipboardErrorToast = async (tabId: number): Promise => { + await Ipc.ensureConnection(tabId) + await Ipc.sendTab(tabId, TabCommand.showToast, { + title: t("clipboard_error_title"), + description: t("clipboard_error_description"), + action: t("clipboard_error_action"), + }) +} + export const openPopupWindow = async ( param: OpenPopupProps, ): Promise => { @@ -366,9 +382,7 @@ export const openPopupWindow = async ( ? "maximized" : undefined - const clipboard = needsClipboard(url) - ? await readClipboard() - : { clipboardText: "", err: undefined } + const clipboard = await readClipboardIfNeeded(url) const window = (await chrome.windows.create({ url: toUrl(url, clipboard.clipboardText), type, @@ -389,16 +403,7 @@ export const openPopupWindow = async ( } if (clipboard.err) { - await Ipc.ensureConnection(window.tabs?.[0].id as number) - await Ipc.sendTab( - window.tabs?.[0].id as number, - TabCommand.showToast, - { - title: t("clipboard_error_title"), - description: t("clipboard_error_description"), - action: t("clipboard_error_action"), - }, - ) + await showClipboardErrorToast(window.tabs?.[0].id as number) } await updateServiceWorkerData([window], param.commandId, current.id, type) @@ -472,9 +477,7 @@ export const openTab = async (param: OpenTabProps): Promise => { console.warn("Failed to get current tab:", e) } - const clipboard = needsClipboard(url) - ? await readClipboard() - : { clipboardText: "", err: undefined } + const clipboard = await readClipboardIfNeeded(url) const tab = await chrome.tabs.create({ url: toUrl(url, clipboard.clipboardText), active, @@ -483,12 +486,9 @@ export const openTab = async (param: OpenTabProps): Promise => { }) if (clipboard.err) { - await Ipc.ensureConnection(tab.id as number) - await Ipc.sendTab(tab.id as number, TabCommand.showToast, { - title: t("clipboard_error_title"), - description: t("clipboard_error_description"), - action: t("clipboard_error_action"), - }) + // A background tab is not visible, so notify in the tab the user is on. + const toastTabId = active ? tab.id : (currentTab?.id ?? tab.id) + await showClipboardErrorToast(toastTabId as number) } return { diff --git a/packages/extension/src/services/offscreenClipboard.ts b/packages/extension/src/services/offscreenClipboard.ts index c0f04e855..096265f8e 100644 --- a/packages/extension/src/services/offscreenClipboard.ts +++ b/packages/extension/src/services/offscreenClipboard.ts @@ -1,40 +1,72 @@ import type { ClipboardResult } from "./ipc" +import { sleep } from "@/lib/utils" -let pendingRead: Promise = Promise.resolve() +const DOCUMENT_PATH = "src/offscreen_clipboard.html" +const MAX_ATTEMPTS = 3 +const RETRY_DELAY_MS = 50 -async function readOnce(): Promise { - const documentUrl = chrome.runtime.getURL("src/offscreen_clipboard.html") +let pendingRead: Promise = Promise.resolve() + +const toErrorMessage = (error: unknown): string => + error instanceof Error ? error.message : String(error) + +const isClipboardResult = (value: unknown): value is ClipboardResult => + typeof value === "object" && + value !== null && + typeof (value as ClipboardResult).data === "string" + +async function ensureDocument(): Promise { + const contexts = await chrome.runtime.getContexts({ + contextTypes: [chrome.runtime.ContextType.OFFSCREEN_DOCUMENT], + documentUrls: [chrome.runtime.getURL(DOCUMENT_PATH)], + }) + // Recheck after worker restarts or if Chrome discarded the document. + if (contexts.length > 0) return try { - const contexts = await chrome.runtime.getContexts({ - contextTypes: [chrome.runtime.ContextType.OFFSCREEN_DOCUMENT], - documentUrls: [documentUrl], + await chrome.offscreen.createDocument({ + url: DOCUMENT_PATH, + reasons: [chrome.offscreen.Reason.CLIPBOARD], + justification: "Read clipboard text without opening a visible window", }) - // Recheck after worker restarts or if Chrome discarded the document. - if (contexts.length === 0) { - await chrome.offscreen.createDocument({ - url: "src/offscreen_clipboard.html", - reasons: [chrome.offscreen.Reason.CLIPBOARD], - justification: "Read clipboard text without opening a visible window", - }) - } - return await chrome.runtime.sendMessage({ + } catch (error) { + // Another context may have created it between the check and the creation. + if (!/single offscreen document/i.test(toErrorMessage(error))) throw error + } +} + +async function readOnce(): Promise { + try { + await ensureDocument() + const result: unknown = await chrome.runtime.sendMessage({ target: "offscreen-clipboard", command: "read", }) - } catch (error) { - return { - data: "", - err: error instanceof Error ? error.message : String(error), + if (!isClipboardResult(result)) { + return { data: "", err: "Invalid response from offscreen document" } } + return result + } catch (error) { + return { data: "", err: toErrorMessage(error) } + } +} + +async function readWithRetry(): Promise { + let result: ClipboardResult = { data: "" } + for (let i = 0; i < MAX_ATTEMPTS; i++) { + result = await readOnce() + if (!result.err) return result + console.warn( + `Retry to read clipboard (attempt ${i + 1}/${MAX_ATTEMPTS}):`, + result.err, + ) + if (i < MAX_ATTEMPTS - 1) await sleep(RETRY_DELAY_MS) } + return result } export function readOffscreenClipboard(): Promise { // Serialize reads and creation; keep the document for subsequent requests. - const read = pendingRead.then(readOnce) - pendingRead = read.then( - () => undefined, - () => undefined, - ) + const read = pendingRead.then(readWithRetry) + pendingRead = read.catch(() => undefined) return read } diff --git a/packages/extension/src/services/pageAction/serviceWorker.ts b/packages/extension/src/services/pageAction/serviceWorker.ts index 778abbe58..3383abea4 100644 --- a/packages/extension/src/services/pageAction/serviceWorker.ts +++ b/packages/extension/src/services/pageAction/serviceWorker.ts @@ -15,6 +15,7 @@ import { getCurrentTab, OpenPopupProps, readClipboard, + showClipboardErrorToast, } from "@/services/chrome" import { incrementCommandExecutionCount } from "@/services/commandMetrics" import { @@ -40,9 +41,7 @@ import type { PageActionContext, PopupOption, DeepPartial, - ShowToastParam, } from "@/types" -import { t } from "@/services/i18n" import { ServiceWorkerData } from "@/services/serviceWorkerData" ServiceWorkerData.init() @@ -386,11 +385,7 @@ export const openAndRun = ( const clipboard = await readClipboard() clipboardText = clipboard.clipboardText if (clipboard.err) { - await Ipc.sendTab(tabId, TabCommand.showToast, { - title: t("clipboard_error_title"), - description: t("clipboard_error_description"), - action: t("clipboard_error_action"), - }) + await showClipboardErrorToast(tabId) } } } else { From 261cde6ddbb851ed4a043535876f065c984d02fd Mon Sep 17 00:00:00 2001 From: ujiro99 Date: Tue, 20 Oct 2026 13:33:59 +0900 Subject: [PATCH 14/14] =?UTF-8?q?test:=20E2E=E3=83=86=E3=82=B9=E3=83=88?= =?UTF-8?q?=E5=90=8D=E3=81=A8=E3=82=B3=E3=83=A1=E3=83=B3=E3=83=88=E3=82=92?= =?UTF-8?q?=E8=8B=B1=E8=AA=9E=E3=81=AB=E7=B5=B1=E4=B8=80?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 5.5 --- packages/extension/e2e/link-preview.spec.ts | 2 +- packages/extension/e2e/shortcut.spec.ts | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/extension/e2e/link-preview.spec.ts b/packages/extension/e2e/link-preview.spec.ts index 237dc1024..177dc688d 100644 --- a/packages/extension/e2e/link-preview.spec.ts +++ b/packages/extension/e2e/link-preview.spec.ts @@ -30,7 +30,7 @@ test.describe("Link Preview", () => { const href = await link.getAttribute("href") expect(href).toBeTruthy() - // React 側が ready になってからネイティブ Shift+click + // Native Shift+click after the React side is ready const [previewPage] = await Promise.all([ context.waitForEvent("page", { timeout: 5000 }), await link.click({ diff --git a/packages/extension/e2e/shortcut.spec.ts b/packages/extension/e2e/shortcut.spec.ts index dc994f5e1..c347578f8 100644 --- a/packages/extension/e2e/shortcut.spec.ts +++ b/packages/extension/e2e/shortcut.spec.ts @@ -27,7 +27,7 @@ async function expectSearch(context: BrowserContext, text: string) { ) } -test.describe("検索ショートカット", () => { +test.describe("Search shortcuts", () => { test.beforeEach(async ({ context, extensionId, getCommands }) => { const options = new OptionsPage(context, extensionId, getCommands) await options.importSettings({ @@ -50,7 +50,7 @@ test.describe("検索ショートカット", () => { await options.close() }) - test("E2E-100: slot_1で検索コマンドが実行され選択テキストが渡る", async ({ + test("E2E-100: slot_1 runs the search command with the selected text", async ({ context, page, }) => { @@ -65,7 +65,7 @@ test.describe("検索ショートカット", () => { await expectSearch(context, selection) }) - test("E2E-101: 選択テキストの日本語・空白・記号が検索URLへ渡る", async ({ + test("E2E-101: Japanese, spaces, and symbols in the selected text are passed to the search URL", async ({ context, page, }) => { @@ -86,7 +86,7 @@ test.describe("検索ショートカット", () => { await expectSearch(context, text) }) - test("E2E-102: 未選択時にクリップボードから検索する", async ({ + test("E2E-102: Searches with the clipboard when nothing is selected", async ({ context, page, }) => { @@ -101,7 +101,7 @@ test.describe("検索ショートカット", () => { await expectSearch(context, text) }) - test("E2E-103: DO_NOTHING設定では未選択時に実行しない", async ({ + test("E2E-103: Does nothing when nothing is selected with the DO_NOTHING setting", async ({ context, page, }) => { @@ -122,7 +122,7 @@ test.describe("検索ショートカット", () => { ) }) - test("E2E-104: Content Scriptのないchrome://ページではService Workerから実行する", async ({ + test("E2E-104: Runs from the Service Worker on chrome:// pages without a content script", async ({ context, page, }) => { @@ -135,7 +135,7 @@ test.describe("検索ショートカット", () => { await expectSearch(context, text) }) - test("E2E-105: chrome.commands.getAllでslot_1〜slot_3が登録されている", async ({ + test("E2E-105: slot_1 to slot_3 are registered in chrome.commands.getAll", async ({ context, }) => { const commands = await context @@ -187,7 +187,7 @@ const pageActionCases = [ ] as const for (const { id, openMode, variable, hasSelection } of pageActionCases) { - test(`${id}: PageActionショートカット / ${openMode} / {{${variable}}} / ${hasSelection ? "選択あり" : "未選択"}`, async ({ + test(`${id}: PageAction shortcut / ${openMode} / {{${variable}}} / ${hasSelection ? "with selection" : "no selection"}`, async ({ context, extensionId, getCommands,