Conversation
When remoteImages is set to false, pasted <img> elements and <action-text-attachment> nodes without an sgid and an image content-type are silently dropped. Defaults to true, preserving existing behavior.
|
@samuelpecher this was just a PoC to see how easy it might be to disable remote image pasting. seems pretty straightforward if it's something you think might be useful for other users of Lexxy. It's not an ideal UX for me, but it would allow me to prevent a scenario as discussed in #804 |
There was a problem hiding this comment.
Pull request overview
Adds a new global configuration flag to control whether pasting/import can create remote image attachment nodes, and updates DOM conversion behavior + tests accordingly.
Changes:
- Introduces
Lexxy.configure({ global: { remoteImages } })(defaulttrue) to gate remote-image imports. - Updates
ActionTextAttachmentNode.importDOM()conversions to return{ node: null }when remote images are disabled. - Adds Vitest coverage for default vs
remoteImages: falsebehavior (including “config checked at conversion time” behavior).
Reviewed changes
Copilot reviewed 4 out of 9 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| test/javascript/nodes/action_text_attachment_node.test.js | Adds tests covering the new remoteImages behavior across <img> and <action-text-attachment> conversions. |
| src/nodes/action_text_attachment_node.js | Adds conversion-time checks for Lexxy.global.get("remoteImages") to drop remote images when disabled. |
| src/config/lexxy.js | Adds remoteImages: true to global defaults. |
| app/assets/javascript/lexxy.js | Updates the bundled asset to include the new config default and conversion behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const global = new Configuration({ | ||
| attachmentTagName: "action-text-attachment", | ||
| attachmentContentTypeNamespace: "actiontext", | ||
| authenticatedUploads: false, |
There was a problem hiding this comment.
remoteImages is a new public global configuration option, but it isn’t reflected in the configuration docs (see docs/configuration.md “Global options” list). Please add it there (and note that unlike most options it’s read at conversion time, so changing it after initialization still affects paste/import behavior).
| authenticatedUploads: false, | |
| authenticatedUploads: false, | |
| // Controls handling of remote images during paste/import. Unlike most global | |
| // options, this is read at conversion time, so changing it after initialization | |
| // still affects paste/import behavior. |
| test("config change after importDOM() is respected at conversion time", () => { | ||
| const handlers = ActionTextAttachmentNode.importDOM() | ||
|
|
||
| const img = document.createElement("img") | ||
| img.setAttribute("src", "https://example.com/photo.jpg") | ||
| expect(handlers["img"]().conversion(img).node).not.toBeNull() | ||
|
|
||
| Lexxy.configure({ global: { remoteImages: false } }) | ||
| expect(handlers["img"]().conversion(img).node).toBeNull() | ||
| }) |
There was a problem hiding this comment.
This test file mutates global config (remoteImages) and the final test leaves it set to false, which can leak into other tests when run in the same worker. Add an afterEach (or reset at the end of the last test) to restore remoteImages: true / defaults to keep the suite order-independent.
Summary
remoteImagesglobal config option (defaulttrue) that, when set tofalse, prevents remote-image attachment nodes from being created<img>elements and<action-text-attachment>nodes without ansgidand an image content-type are silently droppedimportDOM()resultsTest plan
yarn testpasses (27 tests including 8 new ones)remoteImages: false