From 624bfd91754e30656ffa00310a203397a13b4b6b Mon Sep 17 00:00:00 2001 From: Henry Heino <46334387+personalizedrefrigerator@users.noreply.github.com> Date: Tue, 16 Jul 2024 11:25:23 -0700 Subject: [PATCH] Desktop: Fixes #10733: Fix not-yet-created images lost while editing with the Rich Text Editor (#10734) --- packages/app-cli/tests/MdToHtml.ts | 9 ++- .../html_to_md/resource_placeholder.html | 48 ++++++++++++++++ .../tests/html_to_md/resource_placeholder.md | 9 +++ .../resource_nonexistent_image.html | 15 +++++ .../md_to_html/resource_nonexistent_image.md | 3 + packages/lib/HtmlToMd.ts | 1 + .../renderer/MdToHtml/rules/html_image.ts | 2 +- packages/renderer/MdToHtml/rules/image.ts | 5 +- packages/renderer/utils.ts | 33 ++++++++++- packages/turndown/src/commonmark-rules.js | 55 +++++++++++++++++-- 10 files changed, 168 insertions(+), 12 deletions(-) create mode 100644 packages/app-cli/tests/html_to_md/resource_placeholder.html create mode 100644 packages/app-cli/tests/html_to_md/resource_placeholder.md create mode 100644 packages/app-cli/tests/md_to_html/resource_nonexistent_image.html create mode 100644 packages/app-cli/tests/md_to_html/resource_nonexistent_image.md diff --git a/packages/app-cli/tests/MdToHtml.ts b/packages/app-cli/tests/MdToHtml.ts index 6cebadf36..a5571bb89 100644 --- a/packages/app-cli/tests/MdToHtml.ts +++ b/packages/app-cli/tests/MdToHtml.ts @@ -2,13 +2,16 @@ import MdToHtml from '@joplin/renderer/MdToHtml'; const { filename } = require('@joplin/lib/path-utils'); import { setupDatabaseAndSynchronizer, switchClient } from '@joplin/lib/testing/test-utils'; import shim from '@joplin/lib/shim'; +import { RenderOptions } from '@joplin/renderer/types'; +import { isResourceUrl, resourceUrlToId } from '@joplin/lib/models/utils/resourceUtils'; const { themeStyle } = require('@joplin/lib/theme'); // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied function newTestMdToHtml(options: any = null) { options = { ResourceModel: { - isResourceUrl: () => false, + isResourceUrl: isResourceUrl, + urlToId: resourceUrlToId, }, fsDriver: shim.fsDriver(), ...options, @@ -39,7 +42,7 @@ describe('MdToHtml', () => { // if (mdFilename !== 'sanitize_9.md') continue; // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied - const mdToHtmlOptions: any = { + const mdToHtmlOptions: RenderOptions = { bodyOnly: true, }; @@ -51,6 +54,8 @@ describe('MdToHtml', () => { }; } else if (mdFilename.startsWith('sourcemap_')) { mdToHtmlOptions.mapsToLine = true; + } else if (mdFilename.startsWith('resource_')) { + mdToHtmlOptions.resources = {}; } const markdown = await shim.fsDriver().readFile(mdFilePath); diff --git a/packages/app-cli/tests/html_to_md/resource_placeholder.html b/packages/app-cli/tests/html_to_md/resource_placeholder.html new file mode 100644 index 000000000..188c13e08 --- /dev/null +++ b/packages/app-cli/tests/html_to_md/resource_placeholder.html @@ -0,0 +1,48 @@ +

Markdown images:

+ + +

HTML images:

+ diff --git a/packages/app-cli/tests/html_to_md/resource_placeholder.md b/packages/app-cli/tests/html_to_md/resource_placeholder.md new file mode 100644 index 000000000..f564c82b3 --- /dev/null +++ b/packages/app-cli/tests/html_to_md/resource_placeholder.md @@ -0,0 +1,9 @@ +Markdown images: + +- With ALT and title:![test](:/0415d61cc33e47afa6dde45948c3177f "testing") +- With neither ALT nor title:![](:/0a25d61cc33e57afa6dde45948c3177f) + +HTML images: + +- +- \ No newline at end of file diff --git a/packages/app-cli/tests/md_to_html/resource_nonexistent_image.html b/packages/app-cli/tests/md_to_html/resource_nonexistent_image.html new file mode 100644 index 000000000..19789acfa --- /dev/null +++ b/packages/app-cli/tests/md_to_html/resource_nonexistent_image.html @@ -0,0 +1,15 @@ +
+
+
\ No newline at end of file diff --git a/packages/app-cli/tests/md_to_html/resource_nonexistent_image.md b/packages/app-cli/tests/md_to_html/resource_nonexistent_image.md new file mode 100644 index 000000000..1b33a3877 --- /dev/null +++ b/packages/app-cli/tests/md_to_html/resource_nonexistent_image.md @@ -0,0 +1,3 @@ +![](:/a1test2a1test2a1test2a1test22345 "test") +![test](:/a1test2a1test2a1test2a1test22346) + diff --git a/packages/lib/HtmlToMd.ts b/packages/lib/HtmlToMd.ts index fad6e2dfa..c43795be8 100644 --- a/packages/lib/HtmlToMd.ts +++ b/packages/lib/HtmlToMd.ts @@ -28,6 +28,7 @@ export default class HtmlToMd { bulletListMarker: '-', emDelimiter: '*', strongDelimiter: '**', + allowResourcePlaceholders: true, // If soft-breaks are enabled, lines need to end with two or more spaces for // trailing
s to render. See diff --git a/packages/renderer/MdToHtml/rules/html_image.ts b/packages/renderer/MdToHtml/rules/html_image.ts index 5a6302ad1..874e2db5d 100644 --- a/packages/renderer/MdToHtml/rules/html_image.ts +++ b/packages/renderer/MdToHtml/rules/html_image.ts @@ -3,7 +3,7 @@ import { attributesHtml } from '../../htmlUtils'; import * as utils from '../../utils'; function renderImageHtml(before: string, src: string, after: string, ruleOptions: RuleOptions) { - const r = utils.imageReplacement(ruleOptions.ResourceModel, src, ruleOptions.resources, ruleOptions.resourceBaseUrl, ruleOptions.itemIdToUrl); + const r = utils.imageReplacement(ruleOptions.ResourceModel, { src, before, after }, ruleOptions.resources, ruleOptions.resourceBaseUrl, ruleOptions.itemIdToUrl); if (typeof r === 'string') return r; if (r) return ``; return `[Image: ${src}]`; diff --git a/packages/renderer/MdToHtml/rules/image.ts b/packages/renderer/MdToHtml/rules/image.ts index d584b806c..3a5cb7189 100644 --- a/packages/renderer/MdToHtml/rules/image.ts +++ b/packages/renderer/MdToHtml/rules/image.ts @@ -17,7 +17,8 @@ function plugin(markdownIt: any, ruleOptions: RuleOptions) { if (!Resource.isResourceUrl(src) || ruleOptions.plainResourceRendering) return defaultRender(tokens, idx, options, env, self); - const r = utils.imageReplacement(ruleOptions.ResourceModel, src, ruleOptions.resources, ruleOptions.resourceBaseUrl, ruleOptions.itemIdToUrl); + const alt = token.content; + const r = utils.imageReplacement(ruleOptions.ResourceModel, { src, alt, title }, ruleOptions.resources, ruleOptions.resourceBaseUrl, ruleOptions.itemIdToUrl); if (typeof r === 'string') return r; if (r) { const id = r['data-resource-id']; @@ -35,7 +36,7 @@ function plugin(markdownIt: any, ruleOptions: RuleOptions) { destroyEditPopupSyntax: ruleOptions.destroyEditPopupSyntax, }, null); - return ``; + return ``; } return defaultRender(tokens, idx, options, env, self); }; diff --git a/packages/renderer/utils.ts b/packages/renderer/utils.ts index 9ad01a162..e54ad89c7 100644 --- a/packages/renderer/utils.ts +++ b/packages/renderer/utils.ts @@ -1,3 +1,4 @@ +import { attributesHtml } from './htmlUtils'; import { ItemIdToUrlHandler, OptionsResourceModel } from './types'; const Entities = require('html-entities').AllHtmlEntities; @@ -123,10 +124,17 @@ export const resourceStatus = function(ResourceModel: OptionsResourceModel, reso return status; }; +type ImageMarkupData = { + src: string; + alt: string; + title: string; +}|{ src: string; before: string; after: string }; + // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied -export const imageReplacement = function(ResourceModel: OptionsResourceModel, src: string, resources: any, resourceBaseUrl: string, itemIdToUrl: ItemIdToUrlHandler = null) { +export const imageReplacement = function(ResourceModel: OptionsResourceModel, markup: ImageMarkupData, resources: any, resourceBaseUrl: string, itemIdToUrl: ItemIdToUrlHandler = null) { if (!ResourceModel || !resources) return null; + const src = markup.src; if (!ResourceModel.isResourceUrl(src)) return null; const resourceId = ResourceModel.urlToId(src); @@ -136,7 +144,28 @@ export const imageReplacement = function(ResourceModel: OptionsResourceModel, sr if (status !== 'ready') { const icon = resourceStatusImage(status); - return `
` + `` + '
'; + + // Preserve information necessary to restore the original markup when converting + // from HTML to markdown. + const attrs: Record = { + class: `not-loaded-resource not-loaded-image-resource resource-status-${status}`, + ['data-resource-id']: resourceId, + }; + if ('alt' in markup) { + attrs['data-original-alt'] = markup.alt; + attrs['data-original-title'] = markup.title; + } else { + attrs['data-original-before'] = markup.before; + attrs['data-original-after'] = markup.after; + } + + // contenteditable="false": Improves support for the Rich Text Editor -- without this, + // users can add content within the
, which breaks the html-to-md conversion. + return ( + `
` + + `` + + '
' + ); } const mime = resource.mime ? resource.mime.toLowerCase() : ''; if (ResourceModel.isSupportedImageMimeType(mime)) { diff --git a/packages/turndown/src/commonmark-rules.js b/packages/turndown/src/commonmark-rules.js index c37b4124d..b2a3249d2 100644 --- a/packages/turndown/src/commonmark-rules.js +++ b/packages/turndown/src/commonmark-rules.js @@ -140,6 +140,43 @@ rules.foregroundColor = { }, } +// Converts placeholders for not-loaded resources. +rules.resourcePlaceholder = { + filter: function (node, options) { + if (!options.allowResourcePlaceholders) return false; + if (!node.classList || !node.classList.contains('not-loaded-resource')) return false; + const isImage = node.classList.contains('not-loaded-image-resource'); + if (!isImage) return false; + + const resourceId = node.getAttribute('data-resource-id'); + return resourceId && resourceId.match(/^[a-z0-9]{32}$/); + }, + + replacement: function (_content, node) { + 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 ``; + } else { + const originalAltText = node.getAttribute('data-original-alt') || ''; + const title = node.getAttribute('data-original-title'); + return imageMarkdownFromAttributes({ + alt: originalAltText, + title, + src: `:/${resourceId}`, + }); + } + } +} + // ============================== // END Joplin format support // ============================== @@ -510,6 +547,14 @@ rules.code = { } } +function imageMarkdownFromAttributes(attributes) { + var alt = attributes.alt || '' + var src = filterLinkHref(attributes.src || '') + var title = attributes.title || '' + var titlePart = title ? ' "' + filterImageTitle(title) + '"' : '' + return src ? '![' + alt.replace(/([[\]])/g, '\\$1') + ']' + '(' + src + titlePart + ')' : '' +} + function imageMarkdownFromNode(node, options = null) { options = Object.assign({}, { preserveImageTagsWithSize: false, @@ -519,11 +564,11 @@ function imageMarkdownFromNode(node, options = null) { return node.outerHTML; } - var alt = node.alt || '' - var src = filterLinkHref(node.getAttribute('src') || '') - var title = node.title || '' - var titlePart = title ? ' "' + filterImageTitle(title) + '"' : '' - return src ? '![' + alt.replace(/([[\]])/g, '\\$1') + ']' + '(' + src + titlePart + ')' : '' + return imageMarkdownFromAttributes({ + alt: node.alt, + src: node.getAttribute('src'), + title: node.title, + }); } function imageUrlFromSource(node) {