-
-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Desktop: Fixes #10733: Fix not-yet-created images lost while editing with the Rich Text Editor #10734
Desktop: Fixes #10733: Fix not-yet-created images lost while editing with the Rich Text Editor #10734
Conversation
…ces lost in Rich Text Editor
<div class="not-loaded-resource not-loaded-image-resource resource-status-notDownloaded" data-resource-id="a1test2a1test2a1test2a1test22347" data-original-before=" " data-original-after=" class="jop-noMdConv"/" contenteditable="false"><img src="data:image/svg+xml;utf8, | ||
		<svg width="1700" height="1536" xmlns="http://www.w3.org/2000/svg"> |
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.
- To-do (optional/nonblocking?): Debug: Why is
class="jop-noMdConv"
added todata-original-after
?
const htmlBefore = node.getAttribute('data-original-before') || ''; | ||
const htmlAfter = node.getAttribute('data-original-after') || ''; | ||
const isHtml = htmlBefore || htmlAfter; | ||
const resourceId = node.getAttribute('data-resource-id'); | ||
if (isHtml) { | ||
const attrs = [ | ||
htmlBefore.trim(), | ||
`src=":/${resourceId}"`, | ||
htmlAfter.trim(), | ||
].filter(a => !!a); | ||
|
||
return `<img ${attrs.join(' ')}>`; |
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.
Optional: Try to reduce future code duplication: Pass a function through options
to do part of the conversion? Alternatively, add to utils
.
At present, much of this logic will need to be duplicated when fixing #10733 for HTML notes.
Thanks for fixing this!
If we fix this for HTML notes we can refactor at that time |
Summary
This pull request adds support for converting "downloading" icons from HTML back into Markdown in the Rich Text Editor. At present, it's possible to edit notes with these placeholders if the placeholders were generated from images that link to non-existent resources.
This partially fixes #10733.
Important
This only fixes #10733 for Markdown notes. HTML notes still experience the issue.
Possible alternate solution: Disallow editing notes with "not downloaded" placeholders
At present, the Rich Text Editor prevents users from editing notes with linked resources that haven't been downloaded. However, images that point to resources that don't exist allow users to edit notes. Such resources might be created after a user manually deletes an attachment, edits the note in the Markdown editor, or due to a bug similar to the one mentioned here.
Completely disallowing users from editing notes with "not downloaded" placeholders would prevent users from editing notes that include deleted resources.
Testing plan
This pull request includes automated tests.
It has also been tested manually by:
TEST
to the end of the note.TEST
).