fix: fall back when sidebar clipboard writes fail#1216
fix: fall back when sidebar clipboard writes fail#1216lawrence3699 wants to merge 1 commit intomodelcontextprotocol:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a resilient clipboard-copy implementation in the Sidebar to fix copy failures (Issue #913) by falling back to document.execCommand("copy"), while preserving existing success/error toasts and adding regression tests.
Changes:
- Introduce a
copyTextToClipboardhelper with anexecCommand("copy")fallback whennavigator.clipboard.writeTextis unavailable or rejected. - Update both Sidebar copy handlers to use the helper while keeping existing UX/toast behavior.
- Add Sidebar tests covering fallback-success and full-failure scenarios.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| client/src/components/Sidebar.tsx | Adds clipboard helper + routes copy actions through it with fallback logic. |
| client/src/components/tests/Sidebar.test.tsx | Adds regression tests for fallback success and full failure of clipboard copying. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| try { | ||
| textarea.focus(); | ||
| textarea.select(); | ||
| copied = document.execCommand("copy"); |
There was a problem hiding this comment.
fallbackCopy calls document.execCommand("copy") without checking that execCommand exists/is callable. In browsers/environments where execCommand is missing, this will throw a TypeError and surface a confusing error message (and bypass the intended "Clipboard copy failed" UX). Consider guarding with typeof document.execCommand === "function" and throwing a consistent Error("Clipboard copy failed") (or similar) when unavailable.
| copied = document.execCommand("copy"); | |
| if (typeof document.execCommand !== "function") { | |
| throw new Error("Clipboard copy failed"); | |
| } | |
| try { | |
| copied = document.execCommand("copy"); | |
| } catch { | |
| throw new Error("Clipboard copy failed"); | |
| } |
| if (!navigator.clipboard?.writeText) { | ||
| fallbackCopy(); | ||
| return; | ||
| } |
There was a problem hiding this comment.
The new branch that handles navigator.clipboard.writeText being unavailable (if (!navigator.clipboard?.writeText)) isn't covered by a regression test. Adding a test case where navigator.clipboard (or writeText) is missing would help ensure the fallback path works in insecure contexts/older browsers.
Fixes #913.
Summary
document.execCommand("copy")whennavigator.clipboard.writeTextis rejected or unavailableTesting