-
Notifications
You must be signed in to change notification settings - Fork 25
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
#7324: Automatically use the focused document to copy to the clipboard #7635
Changes from all commits
943b54e
3018313
a1a47c7
c92d6c8
c93df22
24ea3e4
6c20940
fd515c1
ac2591a
6cf4d0e
6af134c
236efb4
2ee50a9
60c094a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
import { createActivationUrl } from "@/activation/activationLinkUtils"; | ||
|
||
type ActivationLinkProps = { | ||
|
@@ -44,7 +44,7 @@ const ActivationLink: React.FunctionComponent<ActivationLinkProps> = ({ | |
<AsyncButton | ||
variant="info" | ||
onClick={async () => { | ||
await writeTextToClipboard({ text: activationLink }); | ||
await writeToClipboard({ text: activationLink }); | ||
// Don't close the modal - that allows the user to re-copy the link and verify the link works | ||
notify.success("Copied activation link to clipboard"); | ||
}} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,84 @@ | ||
/* | ||
* Copyright (C) 2024 PixieBrix, Inc. | ||
* | ||
* This program is free software: you can redistribute it and/or modify | ||
* it under the terms of the GNU Affero General Public License as published by | ||
* the Free Software Foundation, either version 3 of the License, or | ||
* (at your option) any later version. | ||
* | ||
* This program is distributed in the hope that it will be useful, | ||
* but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
* GNU Affero General Public License for more details. | ||
* | ||
* You should have received a copy of the GNU Affero General Public License | ||
* along with this program. If not, see <http://www.gnu.org/licenses/>. | ||
*/ | ||
|
||
import { lastFocusedTarget } from "@/utils/focusTracker"; | ||
import { | ||
type Sender, | ||
type Target, | ||
type PageTarget, | ||
getMethod, | ||
} from "webext-messenger"; | ||
import { type ClipboardText } from "@/utils/clipboardUtils"; | ||
|
||
// `WRITE_TO_CLIPBOARD` can be handled by multiple contexts, that's why it's here and not in their /api.ts files | ||
const writeToClipboardInTarget = getMethod("WRITE_TO_CLIPBOARD"); | ||
|
||
/** | ||
* Given a `Sender` as defined by the native Chrome Messaging API, | ||
* 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could be extracted |
||
sender: Sender, | ||
): Target | PageTarget | undefined { | ||
const tabId = sender.tab?.id; | ||
const { frameId, url } = sender; | ||
if (tabId != null && frameId != null) { | ||
return { | ||
tabId, | ||
frameId, | ||
}; | ||
} | ||
|
||
const rootExtensionUrl = chrome.runtime.getURL(""); | ||
if (url?.startsWith(rootExtensionUrl)) { | ||
// Tab-less contexts like the sidePanel and dev tools | ||
return { | ||
page: url.replace(rootExtensionUrl, ""), | ||
}; | ||
} | ||
|
||
// Untargetable sender (e.g. an iframe in the sidebar, page editor, etc.) | ||
// https://github.com/pixiebrix/pixiebrix-extension/issues/7565 | ||
} | ||
|
||
// 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( | ||
Comment on lines
+59
to
+62
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Future API alert 🙌 The current code already works in MV3, but presumably that API will avoid the need to track focus and other guesswork. |
||
item: ClipboardText, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
): Promise<boolean> { | ||
const lastFocusedDocument = await lastFocusedTarget.get(); | ||
if (lastFocusedDocument) { | ||
const target = extractTargetFromSender(lastFocusedDocument); | ||
if (target) { | ||
// The target might be any context that calls `markDocumentAsFocusableByUser`. | ||
// Also, just because they were the lastFocusedDocument, it doesn't mean that | ||
// they are still focused at a OS level, so this might return false. | ||
return writeToClipboardInTarget(target, item); | ||
} | ||
} | ||
|
||
// Try just getting the frontmost tab | ||
const [tab] = await browser.tabs.query({ active: true, currentWindow: true }); | ||
if (tab?.id != null) { | ||
return writeToClipboardInTarget({ tabId: tab.id }, item); | ||
} | ||
|
||
// Dead code, there's always at least one tab, but not worth throwing | ||
return false; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,8 +23,7 @@ import { BusinessError, PropError } from "@/errors/businessErrors"; | |
import { | ||
type ContentType, | ||
detectContentType, | ||
writeTextToClipboard, | ||
writeItemsToClipboard, | ||
writeToClipboard, | ||
} from "@/utils/clipboardUtils"; | ||
import { convertDataUrl } from "@/utils/parseDataUrl"; | ||
|
||
|
@@ -106,6 +105,12 @@ export class CopyToClipboard extends EffectABC { | |
throw new BusinessError("Invalid image content", { cause: error }); | ||
} | ||
|
||
if (blob.type !== "image/png") { | ||
throw new BusinessError( | ||
"Only PNG images are supported by the browser clipboard API", | ||
); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Every browser only supports |
||
|
||
if (!("write" in navigator.clipboard)) { | ||
throw new BusinessError( | ||
"Your browser does not support writing images to the clipboard", | ||
|
@@ -116,22 +121,13 @@ export class CopyToClipboard extends EffectABC { | |
logger.warn("Ignoring HTML content for image content"); | ||
} | ||
|
||
await writeItemsToClipboard( | ||
[ | ||
new ClipboardItem({ | ||
[blob.type]: blob, | ||
}), | ||
], | ||
{ | ||
type: "image", | ||
}, | ||
); | ||
await writeToClipboard({ image: blob }); | ||
|
||
break; | ||
} | ||
|
||
case "text": { | ||
await writeTextToClipboard({ | ||
await writeToClipboard({ | ||
text: String(text), | ||
html, | ||
}); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,15 +23,22 @@ | |
*/ | ||
|
||
import { expectContext } from "@/utils/expectContext"; | ||
import { type JsonValue } from "type-fest"; | ||
import { type OmitIndexSignature, type JsonValue } from "type-fest"; | ||
import { type ManualStorageKey } from "@/utils/storageUtils"; | ||
import { once } from "lodash"; | ||
|
||
// 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 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
expectContext( | ||
"background", | ||
"This polyfill doesn’t share data across contexts; only use it in the background page", | ||
); | ||
} | ||
|
||
/** | ||
* MV3-compatible Map-like storage, this helps transition to chrome.storage.session | ||
|
@@ -41,18 +48,18 @@ export class SessionMap<Value extends JsonValue> { | |
constructor( | ||
private readonly key: string, | ||
private readonly url: ImportMeta["url"], | ||
) { | ||
expectContext( | ||
"background", | ||
"This polyfill doesn’t share data across contexts; only use it in the background page", | ||
); | ||
} | ||
) {} | ||
|
||
// Do not call `expectContext` in the constructor so that `SessionMap` can be | ||
// instantiated at the top level and still be imported (but unused) in other contexts. | ||
private readonly validateContext = once(validateContext); | ||
|
||
private getRawStorageKey(secondaryKey: string): ManualStorageKey { | ||
return `${this.key}::${this.url}::${secondaryKey}` as ManualStorageKey; | ||
} | ||
|
||
async get(secondaryKey: string): Promise<Value | undefined> { | ||
this.validateContext(); | ||
const rawStorageKey = this.getRawStorageKey(secondaryKey); | ||
if (!hasSession) { | ||
return storage.get(rawStorageKey) as Value | undefined; | ||
|
@@ -64,6 +71,8 @@ export class SessionMap<Value extends JsonValue> { | |
} | ||
|
||
async set(secondaryKey: string, value: Value): Promise<void> { | ||
this.validateContext(); | ||
|
||
const rawStorageKey = this.getRawStorageKey(secondaryKey); | ||
if (hasSession) { | ||
await browser.storage.session.set({ [rawStorageKey]: value }); | ||
|
@@ -73,6 +82,8 @@ export class SessionMap<Value extends JsonValue> { | |
} | ||
|
||
async delete(secondaryKey: string): Promise<void> { | ||
this.validateContext(); | ||
|
||
const rawStorageKey = this.getRawStorageKey(secondaryKey); | ||
if (hasSession) { | ||
await browser.storage.session.remove(rawStorageKey); | ||
|
@@ -86,7 +97,8 @@ export class SessionMap<Value extends JsonValue> { | |
* MV3-compatible single-value storage. | ||
* This helps transition to chrome.storage.session and provide some type safety. | ||
*/ | ||
export class SessionValue<Value extends JsonValue> { | ||
// "OmitIndexSignature" is because of https://github.com/sindresorhus/type-fest/issues/815 | ||
export class SessionValue<Value extends OmitIndexSignature<JsonValue>> { | ||
private readonly map: SessionMap<Value>; | ||
|
||
constructor(key: string, url: ImportMeta["url"]) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No longer needed somehow