Guard navigator.clipboard in the WebKit clipboard workaround - #334878
Open
Florian Schirmer (joltcoke) wants to merge 1 commit into
Open
Guard navigator.clipboard in the WebKit clipboard workaround#334878Florian Schirmer (joltcoke) wants to merge 1 commit into
Florian Schirmer (joltcoke) wants to merge 1 commit into
Conversation
installWebKitWriteTextWorkaround() called navigator.clipboard.write() without checking that the async clipboard API exists. Outside a secure context it does not, and since the handler is bound to click and keydown on every container, Safari throws on every click and keystroke. Worse, the handler assigns webKitPendingClipboardWritePromise before the throwing line, so writeText() takes its early exit and never reaches fallbackWriteText() - copying silently does nothing although the execCommand fallback would work. Reading the clipboard once and returning early when it is absent leaves that promise untouched, so writeText() falls through to the fallback as intended. Every other clipboard access in this file is already guarded.
Copilot started reviewing on behalf of
Florian Schirmer (joltcoke)
September 7, 2026 09:40
View session
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The guard preserves fallback behavior, with only a non-blocking comment-style nit remaining.
Pull request overview
Guards the WebKit clipboard workaround when navigator.clipboard is unavailable, preserving fallback copying in insecure Safari contexts.
Changes:
- Checks for the Clipboard API before creating a pending write.
- Reuses the guarded clipboard reference.
File summaries
| File | Review |
|---|---|
src/vs/platform/clipboard/browser/clipboardService.ts |
Correctly adds the availability guard. Nit: shorten the three-line method comment to retain only the secure-context constraint. |
Review details
Suppressed comments (1)
src/vs/platform/clipboard/browser/clipboardService.ts:94
- This three-line method-body comment exceeds the one-line limit for inline comments. The control flow already shows that the pending promise remains unset; please keep only the non-obvious secure-context constraint.
// Guard access to navigator.clipboard, it is only present in a secure context.
// Returning early leaves `webKitPendingClipboardWritePromise` unset, so `writeText`
// keeps falling through to `fallbackWriteText` and copying still works.
- Files reviewed: 1/1 changed files
- Comments generated: 0
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
BrowserClipboardService.installWebKitWriteTextWorkaround()callsgetActiveWindow().navigator.clipboard.write(...)without checking that the async clipboard API isthere. Outside a secure context — plain
http://on anything butlocalhost—navigator.clipboardis
undefined, and since the handler is bound toclickandkeydownon every container, Safarithrows on every click and keystroke:
Every other clipboard access in this very file is already guarded; five of them carry the comment
// Guard access to navigator.clipboard with try/catch. This one is the exception.Why it is more than noise
The handler assigns
this.webKitPendingClipboardWritePromise = currentWritePromisebefore thethrowing line.
writeText()then takes its early exit and never reaches the working fallback:So copying inside the editor silently does nothing on Safari over http, even though
fallbackWriteText()would handle it.The change
Read
navigator.clipboardonce and return early when it is absent. That leaveswebKitPendingClipboardWritePromiseuntouched, sowriteText()proceeds tofallbackWriteText()and copying keeps working. Where the API is present nothing changes; the local binding also removes
a repeated
getActiveWindow()lookup.Reported downstream
microsoft/monaco-editor#4496 describes this exactly, with the same reproduction: serve monaco over
http://on a LAN address, open it in Safari, type. Open since 2024-05-05 with three independentconfirmations and no maintainer response — reported against monaco-editor, while the code lives
here.
Reproducing
http://on a non-loopback address, sowindow.isSecureContextis
falseandnavigator.clipboardisundefined