Skip to content
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: Resolves #6570: Fixed broken image links #6590

Merged
merged 3 commits into from
Jul 12, 2022

Conversation

SFulpius
Copy link
Contributor

Resolves #6570.

When using the WYSIWYG editor, full paths to images are encoded using the encodeURI function. Non-ASCII characters contained in the path are changed. By example, é becomes '%C3%A9'. If changes are made using the WYSIWYG editor, image paths must be translated back to joplin resource id when the note is saved. The translation is using regex to find paths to resources in the note and then translate them.

However, there is an issue if the path contains non-ASCII characters. The regex tries to match a path not encoded with encodeURI, thus it will not match resources paths. For instance, it will try to find path containing é character, but the note contains a path with '%C3%A9'. Thus the link will not be translated back. If the note is synchronized to another device, the link will not be valid.

This was fixed by encoding the paths with encodeURI before the matching. Note that I replaced the markdownUtils.escapeLinkUrl which was used to encode spaces. It works because encodeURI also encodes spaces. markdownUtils.escapeLinkUrl also encodes parenthesis, but I don't think it is useful in this case.

packages/lib/models/Note.ts Outdated Show resolved Hide resolved
packages/lib/models/Note.ts Show resolved Hide resolved
packages/lib/models/Note.ts Outdated Show resolved Hide resolved
@laurent22
Copy link
Owner

Thanks for the fix!

@SFulpius SFulpius deleted the fix-resource-links branch July 12, 2022 12:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Android Joplin Fails to Sync Pictures Added on PC
2 participants