fix: allow paste from context menu #1910
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ee04e063c6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (hasPermission && navigator.clipboard && navigator.clipboard.read) { | ||
| try { | ||
| const items = await navigator.clipboard.read(); | ||
| for (const item of items) { | ||
| if (item.types.includes('text/html')) { | ||
| html = await (await item.getType('text/html')).text(); | ||
| } | ||
| if (item.types.includes('text/plain')) { | ||
| text = await (await item.getType('text/plain')).text(); | ||
| } | ||
| } | ||
| } catch { | ||
| try { | ||
| text = await navigator.clipboard.readText(); | ||
| } catch {} | ||
| } | ||
| } |
There was a problem hiding this comment.
In environments where navigator.clipboard.read is not implemented (e.g., Safari), this function returns empty content without attempting navigator.clipboard.readText(). Since the slash-menu paste action now relies solely on readClipboardRaw, paste will become a no-op in those browsers even though plain text reading is supported. Consider falling back to readText() when read is missing (not just when it throws) to avoid a regression in non-HTML clipboard support.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This was fixed(marked as Outdated)
harbournick
left a comment
harbournick
left a comment
There was a problem hiding this comment.
🤖 SD Review Bot
Non-diff Issues
🤖 [Review Bot: Major] Missing test coverage for readClipboardRaw() in clipboardUtils.test.js. The new function has multiple fallback paths (read → readText → empty) and should have tests covering:
- Successful HTML + text read
read()fails, falls back toreadText()read()unavailable, usesreadText()directly- All clipboard access denied (returns empty strings)
See inline comments for line-specific items.
| * Reads raw HTML and text from the system clipboard (for use in paste actions). | ||
| * @returns {Promise<{ html: string, text: string }>} | ||
| */ | ||
| export async function readClipboardRaw() { |
There was a problem hiding this comment.
🤖 [Review Bot: Major] This function duplicates clipboard reading logic from readFromClipboard() (lines 96-120). Both check permissions, try read() for HTML/text, and fall back to readText().
Consider refactoring readFromClipboard to use this function internally:
export async function readFromClipboard(state) {
const { html, text } = await readClipboardRaw();
// ... parse html/text to ProseMirror content
}This reduces maintenance burden and ensures both paths stay in sync.
There was a problem hiding this comment.
trying out this review bot.. ignore if not helpful. seems worth looking at though
Hi @harbournick!
document.execCommand("paste") was deprecated due to security issues. The recommended way to do copy/paste is the one with Clipboard API