#7324: Automatically use the focused document to copy to the clipboard #7635
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #7635 +/- ##
==========================================
- Coverage 72.90% 72.85% -0.05%
==========================================
Files 1233 1234 +1
Lines 38356 38404 +48
Branches 7196 7209 +13
==========================================
+ Hits 27963 27981 +18
- Misses 10393 10423 +30 ☔ View full report in Codecov by Sentry. |
because it's also used to message other targets
| @@ -74,8 +74,6 @@ const knipConfig = { | |||
| "@types/gapi.client.oauth2-v2", | |||
| // Used by Code Editor so format on save matches pre-commit behavior | |||
| "prettier", | |||
| // Used by eslint-local-rules/noCrossBoundaryImports.js | |||
| "multimatch", | |||
There was a problem hiding this comment.
No longer needed somehow
| // TODO: After the MV3 migration, just use chrome.offscreen instead | ||
| // https://developer.chrome.com/blog/Offscreen-Documents-in-Manifest-v3 | ||
| // https://github.com/GoogleChrome/chrome-extensions-samples/tree/73265836c40426c004ac699a6e19b9d56590cdca/functional-samples/cookbook.offscreen-clipboard-write | ||
| export default async function writeToClipboardInFocusedContext( |
There was a problem hiding this comment.
Future API alert 🙌
The current code already works in MV3, but presumably that API will avoid the need to track focus and other guesswork.
fregante
changed the title
| * return a uniquely-idenfying Target usable by the Messenger. | ||
| * It returns `undefined` for tab-less HTTP senders; they must be either in tabs or chrome-extension:// pages. | ||
| */ | ||
| function extractTargetFromSender( |
There was a problem hiding this comment.
Could be extracted
| // https://developer.chrome.com/blog/Offscreen-Documents-in-Manifest-v3 | ||
| // https://github.com/GoogleChrome/chrome-extensions-samples/tree/73265836c40426c004ac699a6e19b9d56590cdca/functional-samples/cookbook.offscreen-clipboard-write | ||
| export default async function writeToClipboardInFocusedContext( | ||
| item: ClipboardText, |
There was a problem hiding this comment.
Since the base64-to-blob parsing/validation is done a bit too early, this currently only supports copying text. Image copying will still use the modal if necessary
| throw new BusinessError( | ||
| "Only PNG images are supported by the browser clipboard API", | ||
| ); | ||
| } |
There was a problem hiding this comment.
Every browser only supports image/text, image/html and image/png at the moment.
| @@ -23,7 +23,7 @@ import { Form, InputGroup } from "react-bootstrap"; | |||
| import { FontAwesomeIcon } from "@fortawesome/react-fontawesome"; | |||
| import { faCopy } from "@fortawesome/free-solid-svg-icons"; | |||
| import AsyncButton from "@/components/AsyncButton"; | |||
| import { writeTextToClipboard } from "@/utils/clipboardUtils"; | |||
| import { writeToClipboard } from "@/utils/clipboardUtils"; | |||
There was a problem hiding this comment.
There is no need for a dedicated method. Content detection can be done in one place. Before this change, text was being passed around as string or as ClipboardItem indiscriminately. This change just makes the conversion at the last moment so that text can be serialized easily.
src/pageEditor/pageEditor.tsx
Outdated
|
|
||
| void initMessengerLogging(); | ||
| void initRuntimeLogging(); | ||
| void initPerformanceMonitoring(); | ||
| watchNavigation(); | ||
| initToaster(); | ||
| markDocumentAsFocusableByUser(); |
There was a problem hiding this comment.
I dropped this because, as one comment suggested, the dev tools don't support it.
Repro
- Click "Copy after 1 second" menu item on page
- Focus the page editor within a second
- See modal dialog appear on page after failure
Log
This is the unified background <-> page editor communication log. Notice how it responds with false after a failure.
| interface MessengerMethods { | ||
| WRITE_TO_CLIPBOARD: typeof nonInteractivelyWriteToClipboard; | ||
| } | ||
| } |
There was a problem hiding this comment.
Explained in the code below
|
|
||
| // Just like chrome.storage.session, this must be "global" | ||
| // eslint-disable-next-line local-rules/persistBackgroundData -- MV2-only | ||
| const storage = new Map<ManualStorageKey, JsonValue>(); | ||
|
|
||
| // eslint-disable-next-line local-rules/persistBackgroundData -- Static | ||
| const hasSession = "session" in chrome.storage; | ||
| function validateContext(): void { |
What does this PR do?
Demo
The demo shows in order:
copycopyfromsidePanel(thesidePanelis focused)copyviasidePanel(thesidePanelis focused)Screen.Recording.4.mov
Checklist
src/tsconfig.strictNullChecks.json(if possible)