Desktop: Fixes #10733: Fix not-yet-created images lost while editing with the Rich Text Editor (#10734)

2024-12-21 09:38:01 +02:00 · 2024-07-16 11:25:23 -07:00 · 2024-07-16 11:25:23 -07:00 · 624bfd9175
commit 624bfd9175
parent 9ad1249f11
10 changed files with 168 additions and 12 deletions
--- 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);
--- a/packages/app-cli/tests/html_to_md/resource_placeholder.html
+++ b/packages/app-cli/tests/html_to_md/resource_placeholder.html
@ -0,0 +1,48 @@
+<p>Markdown images:</p>
+<ul>
+	<li>
+		With ALT and title:
+		<div
+			class="not-loaded-resource not-loaded-image-resource resource-status-test"
+			data-original-alt="test"
+			data-original-title="testing"
+			data-resource-id="0415d61cc33e47afa6dde45948c3177f"
+		>
+			<img src="data:image/svg+xml;utf8,some-icon-here"/>
+		</div>
+	</li>
+	<li>
+		With neither ALT nor title:
+		<div
+			class="not-loaded-resource not-loaded-image-resource resource-status-error"
+			data-original-alt=""
+			data-original-title=""
+			data-resource-id="0a25d61cc33e57afa6dde45948c3177f"
+		>
+			<img src="data:image/svg+xml;utf8,some-icon-here"/>
+		</div>
+	</li>
+</ul>
+
+<p>HTML images:</p>
+<ul>
+	<li>
+		<div
+			class="not-loaded-resource not-loaded-image-resource resource-status-error"
+			data-original-before=" width=&quot;230&quot;"
+			data-original-after=" style=&quot;border: 32px inset red;&quot;/"
+			data-resource-id="0415d61cc33e47afa6dde45948c3177f"
+		>
+			<img src="data:image/svg+xml;utf8,some-icon-here"/>
+		</div>
+	</li>
+	<li>
+		<div
+			class="not-loaded-resource not-loaded-image-resource resource-status-error"
+			data-original-after="/"
+			data-resource-id="0415d61cc33e47afa6dde45948c3177f"
+		>
+			<img src="data:image/svg+xml;utf8,some-icon-here"/>
+		</div>
+	</li>
+</ul>
--- a/packages/app-cli/tests/html_to_md/resource_placeholder.md
+++ 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:
+
+- <img width="230" src=":/0415d61cc33e47afa6dde45948c3177f" style="border: 32px inset red;"/>
+- <img src=":/0415d61cc33e47afa6dde45948c3177f" />
--- a/packages/app-cli/tests/md_to_html/resource_nonexistent_image.html
+++ b/packages/app-cli/tests/md_to_html/resource_nonexistent_image.html
@ -0,0 +1,15 @@
+<div class="not-loaded-resource not-loaded-image-resource resource-status-notDownloaded" data-resource-id="a1test2a1test2a1test2a1test22345" data-original-alt data-original-title="test" contenteditable="false"><img src="data:image/svg+xml;utf8,
+&Tab;&Tab;&lt;svg width=&quot;1700&quot; height=&quot;1536&quot; xmlns=&quot;http://www.w3.org/2000/svg&quot;&gt;
+&Tab;&Tab;    &lt;path d=&quot;M1280 1344c0-35-29-64-64-64s-64 29-64 64 29 64 64 64 64-29 64-64zm256 0c0-35-29-64-64-64s-64 29-64 64 29 64 64 64 64-29 64-64zm128-224v320c0 53-43 96-96 96H96c-53 0-96-43-96-96v-320c0-53 43-96 96-96h465l135 136c37 36 85 56 136 56s99-20 136-56l136-136h464c53 0 96 43 96 96zm-325-569c10 24 5 52-14 70l-448 448c-12 13-29 19-45 19s-33-6-45-19L339 621c-19-18-24-46-14-70 10-23 33-39 59-39h256V64c0-35 29-64 64-64h256c35 0 64 29 64 64v448h256c26 0 49 16 59 39z&quot;/&gt;
+&Tab;&Tab;&lt;/svg&gt;
+&Tab;"/></div>
+<div class="not-loaded-resource not-loaded-image-resource resource-status-notDownloaded" data-resource-id="a1test2a1test2a1test2a1test22346" data-original-alt="test" data-original-title contenteditable="false"><img src="data:image/svg+xml;utf8,
+&Tab;&Tab;&lt;svg width=&quot;1700&quot; height=&quot;1536&quot; xmlns=&quot;http://www.w3.org/2000/svg&quot;&gt;
+&Tab;&Tab;    &lt;path d=&quot;M1280 1344c0-35-29-64-64-64s-64 29-64 64 29 64 64 64 64-29 64-64zm256 0c0-35-29-64-64-64s-64 29-64 64 29 64 64 64 64-29 64-64zm128-224v320c0 53-43 96-96 96H96c-53 0-96-43-96-96v-320c0-53 43-96 96-96h465l135 136c37 36 85 56 136 56s99-20 136-56l136-136h464c53 0 96 43 96 96zm-325-569c10 24 5 52-14 70l-448 448c-12 13-29 19-45 19s-33-6-45-19L339 621c-19-18-24-46-14-70 10-23 33-39 59-39h256V64c0-35 29-64 64-64h256c35 0 64 29 64 64v448h256c26 0 49 16 59 39z&quot;/&gt;
+&Tab;&Tab;&lt;/svg&gt;
+&Tab;"/></div>
+<div class="not-loaded-resource not-loaded-image-resource resource-status-notDownloaded" data-resource-id="a1test2a1test2a1test2a1test22347" data-original-before=" " data-original-after=" class=&quot;jop-noMdConv&quot;/" contenteditable="false"><img src="data:image/svg+xml;utf8,
+&Tab;&Tab;&lt;svg width=&quot;1700&quot; height=&quot;1536&quot; xmlns=&quot;http://www.w3.org/2000/svg&quot;&gt;
+&Tab;&Tab;    &lt;path d=&quot;M1280 1344c0-35-29-64-64-64s-64 29-64 64 29 64 64 64 64-29 64-64zm256 0c0-35-29-64-64-64s-64 29-64 64 29 64 64 64 64-29 64-64zm128-224v320c0 53-43 96-96 96H96c-53 0-96-43-96-96v-320c0-53 43-96 96-96h465l135 136c37 36 85 56 136 56s99-20 136-56l136-136h464c53 0 96 43 96 96zm-325-569c10 24 5 52-14 70l-448 448c-12 13-29 19-45 19s-33-6-45-19L339 621c-19-18-24-46-14-70 10-23 33-39 59-39h256V64c0-35 29-64 64-64h256c35 0 64 29 64 64v448h256c26 0 49 16 59 39z&quot;/&gt;
+&Tab;&Tab;&lt;/svg&gt;
+&Tab;"/></div>
--- a/packages/app-cli/tests/md_to_html/resource_nonexistent_image.md
+++ b/packages/app-cli/tests/md_to_html/resource_nonexistent_image.md
@ -0,0 +1,3 @@
+![](:/a1test2a1test2a1test2a1test22345 "test")
+![test](:/a1test2a1test2a1test2a1test22346)
+<img src=":/a1test2a1test2a1test2a1test22347"/>
--- 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 <br/>s to render. See
--- 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 `<img ${before} ${attributesHtml(r)} ${after}/>`;
 	return `[Image: ${src}]`;
--- 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 `<img data-from-md ${attributesHtml({ ...r, title: title, alt: token.content })} ${js}/>`;
+			return `<img data-from-md ${attributesHtml({ ...r, title: title, alt })} ${js}/>`;
 		}
 		return defaultRender(tokens, idx, options, env, self);
 	};
--- 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 `<div class="not-loaded-resource resource-status-${status}" data-resource-id="${resourceId}">` + `<img src="data:image/svg+xml;utf8,${htmlentities(icon)}"/>` + '</div>';
+
+		// Preserve information necessary to restore the original markup when converting
+		// from HTML to markdown.
+		const attrs: Record<string, string> = {
+			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 <div>, which breaks the html-to-md conversion.
+		return (
+			`<div ${attributesHtml(attrs)} contenteditable="false">`
+				+ `<img src="data:image/svg+xml;utf8,${htmlentities(icon)}"/>`
+			+ '</div>'
+		);
 	}
 	const mime = resource.mime ? resource.mime.toLowerCase() : '';
 	if (ResourceModel.isSupportedImageMimeType(mime)) {
--- 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 `<img ${attrs.join(' ')}>`;
+    } 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) {