🔒 Remove deprecated document.execCommand from useCopyToClipboard - #520
🔒 Remove deprecated document.execCommand from useCopyToClipboard#520is0692vs wants to merge 1 commit into
Conversation
Co-authored-by: is0692vs <135803462+is0692vs@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
Warning Review limit reached
Next review available in: 53 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
📄 Knowledge reviewDosu skipped reviewing this PR because your organization has used its
|
PR Summary by QodoRemove deprecated document.execCommand fallback from useCopyToClipboard
AI Description
Diagram
High-Level Assessment
Files changed (3)
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Code Review by Qodo
Context used✅ Compliance rules (platform):
30 rules 1. Stale copied feedback
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| logger.error("Failed to copy", err); | ||
| } | ||
| } else { | ||
| clipboardError = new Error("Clipboard API not available"); | ||
| } | ||
|
|
||
| // Fallback for older browsers | ||
| const textArea = document.createElement("textarea"); | ||
| textArea.value = text; | ||
| textArea.style.position = "fixed"; | ||
| textArea.style.left = "-9999px"; | ||
| textArea.style.top = "0"; | ||
| textArea.setAttribute("readonly", ""); | ||
| document.body.appendChild(textArea); | ||
|
|
||
| let successful = false; | ||
| let fallbackError: unknown = null; | ||
|
|
||
| try { | ||
| textArea.select(); | ||
| successful = document.execCommand("copy"); | ||
| if (!successful) { | ||
| fallbackError = new Error("document.execCommand('copy') failed"); | ||
| } | ||
| } catch (err) { | ||
| successful = false; | ||
| fallbackError = err; | ||
| } finally { | ||
| document.body.removeChild(textArea); | ||
| } | ||
|
|
||
| if (successful) { | ||
| showCopiedFeedback(); | ||
| } else { | ||
| logger.error("Failed to copy", clipboardError, fallbackError); | ||
| logger.error("Failed to copy", new Error("Clipboard API not available")); | ||
| } |
There was a problem hiding this comment.
1. Stale copied feedback 🐞 Bug ≡ Correctness
When copyToClipboard fails (clipboard missing or writeText rejects), useCopyToClipboard only logs and never clears any existing copied state/timer, so UI may still show “Copied!” from a previous success even though the latest attempt failed (until the old timeout expires). This becomes more likely now that the execCommand fallback has been removed, increasing the number of failure-path executions.
Agent Prompt
## Issue description
`useCopyToClipboard` sets `copied=true` only on success, but on failure it only logs and does not clear `copied` or cancel any existing timer. If a previous successful copy set `copied=true` and the user clicks again within the timeout, a failure can leave the UI incorrectly displaying “Copied!” for the remainder of the prior timer.
## Issue Context
Now that the legacy execCommand fallback is removed, failures are more common (e.g., missing Clipboard API / permission issues), so the stale-success feedback is more visible.
## Fix Focus Areas
- src/hooks/useCopyToClipboard.ts[14-31]
## Suggested fix
- In both failure branches (the `catch` and the `else`), clear any existing timeout and set `copied` to `false` (e.g., via a small helper like `clearCopiedFeedback()` that mirrors the timer cleanup used in `showCopiedFeedback`).
- Optionally, have `copyToClipboard` return a boolean (`true` on success, `false` on failure) to make it harder for callers to assume success when `await` resolves.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
1 participant
🎯 What: The
useCopyToClipboardhook previously included a fallback mechanism usingdocument.execCommand("copy")for older browsers that do not support the modernnavigator.clipboardAPI. This fallback has been completely removed. The hook now exclusively relies onnavigator.clipboard.writeText.document.execCommandAPI is deprecated and known to have security implications and inconsistencies across different browsers. Retaining this deprecated API could introduce potential vulnerabilities or unhandled edge cases, especially since it requires manipulating hidden DOM elements (<textarea>) which could be exploited or cause styling/layout issues.🛡️ Solution: Removed the
document.execCommandfallback logic and the associated DOM manipulation fromuseCopyToClipboard.ts. The hook now safely catches errors ifnavigator.clipboard.writeTextfails or if the API is entirely unavailable, logging the error securely without triggering any legacy copy operations. The corresponding test suites (useCopyToClipboard.test.tsandShareButtons.test.tsx) have been updated to reflect the new behavior and remove alldocument.execCommandmocks.PR created automatically by Jules for task 17996083711857532255 started by @is0692vs
Greptile Summary
useCopyToClipboardから非推奨のdocument.execCommand("copy")と一時的な DOM 操作を削除し、Clipboard API のみに統一する変更です。ShareButtonsのテストが新しい失敗時の挙動に合わせて更新されています。Confidence Score: 5/5
意図されたブラウザー互換性の縮小を除き、マージを妨げる問題は確認できず、安全にマージできると考えます。
非推奨のコピー処理と関連する DOM 操作は一貫して削除され、成功時の状態更新と Clipboard API の失敗時・未提供時のエラー処理が対応するテストでカバーされています。
Important Files Changed
Reviews (1): Last reviewed commit: "refactor: remove document.execCommand fa..." | Re-trigger Greptile
Context used: