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: Fixes #10733: Fix not-yet-created images lost while editing with the Rich Text Editor #10734

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions packages/app-cli/tests/MdToHtml.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
};

Expand All @@ -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);
Expand Down
48 changes: 48 additions & 0 deletions packages/app-cli/tests/html_to_md/resource_placeholder.html
Original file line number Diff line number Diff line change
@@ -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>
9 changes: 9 additions & 0 deletions packages/app-cli/tests/html_to_md/resource_placeholder.md
Original file line number Diff line number Diff line change
@@ -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" />
15 changes: 15 additions & 0 deletions packages/app-cli/tests/md_to_html/resource_nonexistent_image.html
Original file line number Diff line number Diff line change
@@ -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;
Comment on lines +11 to +12
Copy link
Collaborator Author

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=&quot;jop-noMdConv&quot; added to data-original-after?

&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>
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
![](:/a1test2a1test2a1test2a1test22345 "test")
![test](:/a1test2a1test2a1test2a1test22346)
<img src=":/a1test2a1test2a1test2a1test22347"/>
1 change: 1 addition & 0 deletions packages/lib/HtmlToMd.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion packages/renderer/MdToHtml/rules/html_image.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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}]`;
Expand Down
5 changes: 3 additions & 2 deletions packages/renderer/MdToHtml/rules/image.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'];
Expand All @@ -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);
};
Expand Down
33 changes: 31 additions & 2 deletions packages/renderer/utils.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { attributesHtml } from './htmlUtils';
import { ItemIdToUrlHandler, OptionsResourceModel } from './types';

const Entities = require('html-entities').AllHtmlEntities;
Expand Down Expand Up @@ -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);
Expand All @@ -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)) {
Expand Down
55 changes: 50 additions & 5 deletions packages/turndown/src/commonmark-rules.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(' ')}>`;
Comment on lines +156 to +167
Copy link
Collaborator Author

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.

} 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
// ==============================
Expand Down Expand Up @@ -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,
Expand All @@ -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) {
Expand Down
Loading