Support cross domain/application copy/paste #6087
Conversation
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 10/18/2025, 10:44:53 PM UTC 🔗 Links🎉 Your Storybook is ready for review! |
🎭 Playwright Test Results⏰ Completed at: 10/18/2025, 10:58:15 PM UTC 📈 Summary
📊 Test Reports by Browser
🎉 Click on the links above to view detailed test results for each browser configuration. |
The instanceof check is required for the type inspection to be valid, but in general, if text is being edited (such as the login password field), the text input should always have priority
Bundle Size ReportApp Entry PointsMain application bundles
Category Total: 11.7 MB Core ViewsMajor application views and screens
Category Total: 722 kB UI PanelsSettings and configuration panels
Category Total: 74.8 kB ServicesBusiness logic and services
Category Total: 10 kB UtilitiesHelper functions and utilities
Category Total: 1.07 kB OtherUncategorized bundles
Category Total: 1.12 kB Overall Total Size: 12.5 MB |
AustinMroz
changed the title
dosubot
bot
added
the
size:M
christian-byrne
added
the
claude-review
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| ._deserializeItems(JSON.parse(atob(encodedData)), {}) | ||
| return true | ||
| } catch (err) { | ||
| console.error(err) |
There was a problem hiding this comment.
[quality] high Priority
Issue: Generic error handling that swallows specific error details
Context: Using console.error(err) and returning false makes debugging difficult for users when clipboard operations fail
Suggestion: Log specific error message with context like 'Failed to parse clipboard data:' + err.message or provide user-friendly feedback
| const encodedData = | ||
| dataElement.attributes?.getNamedItem('data-metadata')?.value | ||
| if (!encodedData) return false | ||
| useCanvasStore() |
There was a problem hiding this comment.
[architecture] high Priority
Issue: Cross-cutting concerns violation - direct access to canvas store within utility function
Context: The pasteClipboardItems function directly calls useCanvasStore() which breaks the dependency injection pattern used in Vue composables
Suggestion: Pass canvas instance as parameter: pasteClipboardItems(data: DataTransfer, canvas: LGraphCanvas): boolean
There was a problem hiding this comment.
Comprehensive PR Review
This review is generated by Claude. It may not always be accurate, as with human reviewers. If you believe that any of the comments are invalid or incorrect, please state why for each. For others, please implement the changes in one way or another.
Review Summary
PR: Support cross domain/application copy/paste (#6087)
Impact: 44 additions, 28 deletions across 3 files
Issue Distribution
- Critical: 1
- High: 2
- Medium: 1
- Low: 0 (1 comment could not be posted due to line resolution issue)
Category Breakdown
- Architecture: 1 issue
- Security: 1 issue
- Performance: 0 issues
- Code Quality: 3 issues
Key Findings
Architecture & Design
The PR introduces a cross-domain clipboard functionality but has architectural concerns:
- The new pasteClipboardItems function directly accesses useCanvasStore(), violating dependency injection patterns
- Removal of element targeting validation may affect existing workflows
- Method visibility changes in LGraphCanvas lack documentation
Security Considerations
CRITICAL: The implementation has a potential XSS vulnerability where HTML content from clipboard is directly assigned to innerHTML without sanitization. This could allow malicious scripts to execute if an attacker can control clipboard HTML content.
Performance Impact
No significant performance impact identified. The changes maintain existing patterns for clipboard operations.
Integration Points
- Changes affect cross-application clipboard sharing
- Modifications to canvas event handling could impact existing extensions
- DOM manipulation patterns may need testing across different browser environments
Positive Observations
- Good use of btoa/atob for data encoding
- Proper error handling structure with try/catch blocks
- Maintains backward compatibility with existing clipboard operations
- Uses proper Vue composable patterns for most functionality
References
Next Steps
- CRITICAL: Address the XSS vulnerability in pasteClipboardItems function before merge
- Refactor pasteClipboardItems to use dependency injection
- Add comprehensive tests for cross-domain clipboard functionality
- Consider adding user-facing error messages for clipboard failures
This is a comprehensive automated review. For architectural decisions requiring human judgment, please request additional manual review.
github-actions
bot
removed
the
claude-review
and moved the check into a utility file so that the check is shared between the copy and paste code.
dosubot
bot
added
size:L
2 participants
Browsers place very heavy restrictions on what can be copied and pasted. See:
Further thoughts and TODO
┆Issue is synchronized with this Notion page by Unito