-
Notifications
You must be signed in to change notification settings - Fork 37.4k
fix: ensure SVG images are loaded before clipboard copy #263799
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
fix: ensure SVG images are loaded before clipboard copy #263799
Conversation
Fixes intermittent clipboard copy failures for SVG output where users encountered "No blob data to write to clipboard" errors. The issue was caused by a race condition where canvas operations proceeded before the dynamically created Image element finished loading the SVG data URL. - Add ensureImageLoaded helper to wait for image load completion - Handle both already-loaded and loading states - Include error handling and 5-second timeout protection - Maintain backward compatibility with existing image copy functionality
|
is there an issue that can be linked to this? |
|
I’ve registered issue #265020 and linked it to this pull request. |
|
I've updated issue #265020 with details about the visible error in the browser console, which is triggered by the patched function. Please let me know if there's anything I can do to help move the review process forward. |
|
verified the fix, thanks! |
|
|
||
| if (image) { | ||
| const imageToCopy = image; | ||
| //FIX: Ensure image is loaded before proceeding |
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.
// FIX: Normally means there's a todo here? Can we remove that?
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.
Thanks for catching that. I’ve removed the // FIX: and clarified the comments.
| } else { | ||
| img.onload = () => resolve(img); | ||
| img.onerror = () => reject(new Error('Failed to load image')); | ||
| // Add timeout to prevent hanging |
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.
Seems redundant
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.
Thank you for the feedback!
I understand why it might seem redundant, but in practice, the image is not always guaranteed to be fully loaded at this point - especially with dynamically created images. Adding this check prevents race conditions and intermittent clipboard errors, as seen in #265020.
I’ve clarified the comments in the code to explain this rationale.
Please let me know if you have further concerns or if I’ve missed something!
|
@microsoft-github-policy-service agree |
|
Thanks, guys! I really appreciate your support. |
Fixes intermittent clipboard copy failures for SVG output where users
encountered "No blob data to write to clipboard" errors. The issue was
caused by a race condition where canvas operations proceeded before
the dynamically created Image element finished loading the SVG data URL.
Fixes: #265020