Repository navigation
fix: ショートカット実行時に古い選択テキストを使わないようにし、クリップボード利用の判定を一本化する - #499
Merged
Merged
Conversation
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 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #499 +/- ##
==========================================
+ Coverage 55.15% 55.27% +0.11%
==========================================
Files 294 294
Lines 29904 29934 +30
Branches 3044 3067 +23
==========================================
+ Hits 16494 16546 +52
+ Misses 13410 13388 -22 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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 <noreply@anthropic.com>
ujiro99
changed the title
- 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 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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 <noreply@anthropic.com>
The executor fills in the default, so actions no longer repeat it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…s 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 <noreply@anthropic.com>
…OnFocusChange 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 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
PR説明で保証している iframe 内の選択取得が、現在の実装では満たされていません。
1 open issue
What changed in this PR
概要
Issue #498 に対応し、ショートカット実行時の古い選択テキストの使用を防ぎ、クリップボード利用の判定を統一する変更です。
- 共有ストレージの選択テキストを廃止し、タブへの問い合わせとフォーカス状態で判定します。
- 選択テキストの代替とステップ用のクリップボード利用を分離します。
- 関連テストとテスト設計書を更新します。
懸念点
記載された仕様と実装に不一致が残っています。
改善提案
仕様を満たす取得経路と回帰テストを追加してください。
| File | Description |
|---|---|
| packages/extension/src/types/index.ts | 実行引数とクリップボード用フラグを整理 |
| packages/extension/src/services/storage/index.ts | 選択テキストの初期値を削除 |
| packages/extension/src/services/storage/const.ts | 選択テキストの保存キーを削除 |
| packages/extension/src/services/pageAction/serviceWorker.ts | 用途別フラグで取得・置換 |
| packages/extension/src/services/pageAction/serviceWorker-sidePanel.ts | サイドパネルの選択代替に対応 |
| packages/extension/src/services/pageAction/__tests__/serviceWorker-sidePanel.test.ts | 用途別の置換動作を検証 |
| packages/extension/src/services/pageAction/__tests__/serviceWorker-setup.ts | 新しい判定関数をモック化 |
| packages/extension/src/services/pageAction/__tests__/serviceWorker-execution.test.ts | 取得と選択代替の分離を検証 |
| packages/extension/src/services/ipc.ts | 選択問い合わせと通信型を追加 |
| packages/extension/src/services/contextMenus.ts | 判定済み選択テキストの引数を追加 |
| packages/extension/src/services/chrome.ts | 取得判定を共通関数へ統一 |
| packages/extension/src/service_worker.ts | ショートカットの選択判定を集約 |
| packages/extension/src/providers/SelectContextProvider.tsx | 選択とフォーカス喪失を応答 |
| packages/extension/src/providers/__tests__/SelectContextProvider.test.tsx | 選択応答とフォーカス状態を検証 |
| packages/extension/src/lib/utils.ts | 取得判定とURL置換を整理 |
| packages/extension/src/lib/__tests__/utils.test.ts | 判定関数と置換仕様を検証 |
| packages/extension/src/hooks/useContextMenu.ts | 実行要求をまとめて保持 |
| packages/extension/src/hooks/useCommandExecutor.ts | 新フラグを実行処理へ伝達 |
| packages/extension/src/components/option/editor/SearchUrlAssistDialog.tsx | 新しい実行引数名へ更新 |
| packages/extension/src/components/menu/InvisibleItem.tsx | 送信元の選択判定を優先 |
| packages/extension/src/components/menu/__tests__/InvisibleItem.test.tsx | 選択テキストの受け渡しを検証 |
| packages/extension/src/action/window.ts | 新フラグと引数型へ移行 |
| packages/extension/src/action/tab.ts | 新フラグと引数型へ移行 |
| packages/extension/src/action/sidePanel.ts | 選択の再判定を削除 |
| packages/extension/src/action/serviceWorker.ts | 新フラグを伝達 |
| packages/extension/src/action/popup.ts | 新フラグと引数型へ移行 |
| packages/extension/src/action/pageAction.ts | 選択代替とステップ利用を分離 |
| packages/extension/src/action/index.ts | 新フラグを伝達 |
| packages/extension/src/action/executor.ts | 新フラグの既定値を設定 |
| packages/extension/src/action/backgroundTab.ts | 新フラグと引数型へ移行 |
| packages/extension/src/action/aiPrompt.ts | プロンプトのクリップボード用途を分離 |
| packages/extension/src/action/__tests__/pageAction.test.ts | 用途別フラグへ検証を更新 |
| packages/extension/src/action/__tests__/aiPrompt.test.ts | 新しい取得判定へ検証を更新 |
| packages/extension/src/__tests__/service_worker.test.ts | ショートカットの選択判定を検証 |
| docs/test/serviceWorker-test-design.md | 選択代替のテスト仕様を更新 |
Review effort: Balanced
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 <noreply@anthropic.com>
7 tasks
ujiro99
marked this pull request as ready for review
October 19, 2026 11:54
Owner
Author
パターン組み合わせ
テストケース
|
- 読み取り失敗時に最大3回まで再試行する - Offscreenからの不正な応答をエラーとして扱う - textareaにフォーカスできない場合はエラーにする - 作成時の単一ドキュメント制約エラーでは既存ドキュメントを使う - バックグラウンドタブで開く場合は実行元タブへエラーを通知する - エラートーストとクリップボード読み取りの重複処理を共通化する Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
概要
ショートカットキーで検索コマンドを実行したときに、直近のクリップボードの値ではなく古い選択テキストが使われる不具合を修正します。あわせて、クリップボードを使うかどうかの判定を1か所に集約し、関連するフラグの命名を整理します。
Closes #498
原因
SESSION_STORAGE_KEY.SELECTION_TEXT）で全タブ共有されているが、タブを切り替えてもクリアされない。また、chrome:// など content script が動かないタブでは選択テキストを記録できない。そのため、別タブの古い選択テキストが使われていた。toUrl、ページアクションの Service Worker）にあり、それぞれ別の選択テキストを見ていた。フォーカス変更時に storage がクリアされると onCommand は「選択なし」と判定するが、実行する content script はタブの React state（ページ上の選択）を使うため、判定と実行で選択テキストが食い違っていたkeepMenuOpenOnFocusChangeが ON の場合は、フォーカス変更時に storage がクリアされないため、古い選択テキストがそのまま使われていた変更内容
選択テキストの判定（仕様）
SELECTION_TEXTを廃止し、onCommand ではアクティブタブに選択テキストを問い合わせる（TabCommand.getSelection）chrome.windows.getLastFocused()で判定）selectionTextとselectionFromClipboardを content script に渡し、content script はそれを使う（右クリックメニュー経由の場合は従来どおりタブの state を使う）命名の整理
allowClipboardFallbackselectionFromClipboardUrlParam.useClipboard（選択テキストの代替）UrlParam.selectionFromClipboard%s/{{SelectedText}}をクリップボードの値にするUrlParam.useClipboard（ページアクションの{{Clipboard}}）UrlParam.clipboardForStepsSidePanelPendingAction.useClipboardselectionFromClipboard/clipboardForStepsneedsClipboard(url)で導出するActionExecuteParams（selectionFromClipboard必須）にし、?? falseの重複を削除する（デフォルト値は executor で設定）toUrl・ページアクションの Service Worker から、isEmpty(selectionText)による再判定を削除する挙動が変わる点
{{Clipboard}}を使うページアクションを選択なしで実行したとき、{{SelectedText}}と%sがクリップボードの値に置き換わらなくなる（潜在バグの修正）{{SelectedText}}がクリップボードの値になる（他のモードと統一）テスト
InvisibleItem、SelectContextProvider（選択テキストの応答）、needsClipboard、ページアクション SW（SWD-44-b、SP-13-b）yarn test（1522件）／yarn lint（error 0）／yarn buildが通ることを確認済み確認手順
🤖 Generated with Claude Code