self-hosted/joplin
1
0
Fork 0
mirror of https://github.com/laurent22/joplin.git synced 2024-11-24 08:12:24 +02:00
Code Issues Releases Activity

Desktop: Fix images fail to render in the preview pane for HTML notes (#10806)

Browse Source
This commit is contained in:
Henry Heino 2024-08-02 06:47:43 -07:00 committed by GitHub
parent e5c8b4bb66
commit 9dbd481f28
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 102 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
.eslintignore
View File

@ -455,6 +455,7 @@ packages/app-desktop/gui/utils/loadScript.js
packages/app-desktop/gulpfile.js
packages/app-desktop/integration-tests/goToAnything.spec.js
packages/app-desktop/integration-tests/main.spec.js
packages/app-desktop/integration-tests/markdownEditor.spec.js
packages/app-desktop/integration-tests/models/GoToAnything.js
packages/app-desktop/integration-tests/models/MainScreen.js
packages/app-desktop/integration-tests/models/NoteEditorScreen.js
@ -467,6 +468,7 @@ packages/app-desktop/integration-tests/simpleBackup.spec.js
packages/app-desktop/integration-tests/util/activateMainMenuItem.js
packages/app-desktop/integration-tests/util/createStartupArgs.js
packages/app-desktop/integration-tests/util/firstNonDevToolsWindow.js
packages/app-desktop/integration-tests/util/getImageSourceSize.js
packages/app-desktop/integration-tests/util/setFilePickerResponse.js
packages/app-desktop/integration-tests/util/setMessageBoxResponse.js
packages/app-desktop/integration-tests/util/test.js

2
.gitignore vendored
View File

@ -434,6 +434,7 @@ packages/app-desktop/gui/utils/loadScript.js
packages/app-desktop/gulpfile.js
packages/app-desktop/integration-tests/goToAnything.spec.js
packages/app-desktop/integration-tests/main.spec.js
packages/app-desktop/integration-tests/markdownEditor.spec.js
packages/app-desktop/integration-tests/models/GoToAnything.js
packages/app-desktop/integration-tests/models/MainScreen.js
packages/app-desktop/integration-tests/models/NoteEditorScreen.js
@ -446,6 +447,7 @@ packages/app-desktop/integration-tests/simpleBackup.spec.js
packages/app-desktop/integration-tests/util/activateMainMenuItem.js
packages/app-desktop/integration-tests/util/createStartupArgs.js
packages/app-desktop/integration-tests/util/firstNonDevToolsWindow.js
packages/app-desktop/integration-tests/util/getImageSourceSize.js
packages/app-desktop/integration-tests/util/setFilePickerResponse.js
packages/app-desktop/integration-tests/util/setMessageBoxResponse.js
packages/app-desktop/integration-tests/util/test.js

26
packages/app-desktop/integration-tests/main.spec.ts
View File

@ -8,6 +8,7 @@ import createStartupArgs from './util/createStartupArgs';
import firstNonDevToolsWindow from './util/firstNonDevToolsWindow';
import setFilePickerResponse from './util/setFilePickerResponse';
import setMessageBoxResponse from './util/setMessageBoxResponse';
import getImageSourceSize from './util/getImageSourceSize';
test.describe('main', () => {
@ -108,27 +109,10 @@ test.describe('main', () => {
await setMessageBoxResponse(electronApp, /^No/i);
await editor.attachFileButton.click();
const getImageSize = async () => {
const viewerFrame = editor.getNoteViewerIframe();
const renderedImage = viewerFrame.getByAltText(filename);
const viewerFrame = editor.getNoteViewerIframe();
const renderedImage = viewerFrame.getByAltText(filename);
// Use state: 'attached' -- we don't need the image to be on the screen (just present
// in the DOM).
await renderedImage.waitFor({ state: 'attached' });
// We load a copy of the image to avoid returning an overriden width set with
// .width = some_number
return await renderedImage.evaluate((originalImage: HTMLImageElement) => {
return new Promise<[number, number]>(resolve => {
const testImage = new Image();
testImage.onload = () => {
resolve([testImage.width, testImage.height]);
};
testImage.src = originalImage.src;
});
});
};
const fullSize = await getImageSize();
const fullSize = await getImageSourceSize(renderedImage);
// To make it easier to find the image (one image per note), we switch to a new, empty note.
await mainScreen.createNewNote('Image resize test (part 2)');
@ -138,7 +122,7 @@ test.describe('main', () => {
await setMessageBoxResponse(electronApp, /^Yes/i);
await editor.attachFileButton.click();
const resizedSize = await getImageSize();
const resizedSize = await getImageSourceSize(renderedImage);
expect(resizedSize[0]).toBeLessThan(fullSize[0]);
expect(resizedSize[1]).toBeLessThan(fullSize[1]);

30
packages/app-desktop/integration-tests/markdownEditor.spec.ts Normal file
View File

@ -0,0 +1,30 @@
import { test, expect } from './util/test';
import MainScreen from './models/MainScreen';
import { join } from 'path';
import getImageSourceSize from './util/getImageSourceSize';
test.describe('markdownEditor', () => {
test('preview pane should render images in HTML notes', async ({ mainWindow, electronApp }) => {
const mainScreen = new MainScreen(mainWindow);
await mainScreen.waitFor();
await mainScreen.importHtmlDirectory(electronApp, join(__dirname, 'resources', 'html-import'));
const importedFolder = mainScreen.sidebar.container.getByText('html-import');
await importedFolder.waitFor();
await importedFolder.click();
const importedHtmlFileItem = mainScreen.noteListContainer.getByText('test-html-file-with-image');
await importedHtmlFileItem.click();
const viewerFrame = mainScreen.noteEditor.getNoteViewerIframe();
// Should render headers
await expect(viewerFrame.locator('h1')).toHaveText('Test HTML file!');
// Should render images
const image = viewerFrame.getByAltText('An SVG image.');
await expect(image).toBeAttached();
await expect(await getImageSourceSize(image)).toMatchObject([117, 30]);
});
});

10
packages/app-desktop/integration-tests/models/MainScreen.ts
View File

@ -3,6 +3,7 @@ import NoteEditorScreen from './NoteEditorScreen';
import activateMainMenuItem from '../util/activateMainMenuItem';
import Sidebar from './Sidebar';
import GoToAnything from './GoToAnything';
import setFilePickerResponse from '../util/setFilePickerResponse';
export default class MainScreen {
public readonly newNoteButton: Locator;
@ -56,4 +57,13 @@ export default class MainScreen {
const searchBar = this.page.getByPlaceholder('Search...');
await searchBar.fill(text);
}
public async importHtmlDirectory(electronApp: ElectronApplication, path: string) {
await setFilePickerResponse(electronApp, [path]);
const startedImport = await activateMainMenuItem(electronApp, 'HTML - HTML document (Directory)', 'Import');
if (!startedImport) {
throw new Error('Unable to find HTML directory import menu item.');
}
}
}

1
packages/app-desktop/integration-tests/resources/html-import/image.svg Normal file
View File

@ -0,0 +1 @@
<svg viewBox="4.86 6.543831521739131 4.585597826086956 1.171875" width="117" height="30" version="1.1" baseProfile="full" xmlns="http://www.w3.org/2000/svg" class="js-draw--autoresize"><style id="js-draw-style-sheet">path{stroke-linecap:round;stroke-linejoin:round;}text{white-space:pre;}</style><path d="M9.445597826086956,7.71570652173913L9.445597826086956,6.543831521739131L4.86,6.543831521739131L4.86,7.71570652173913L9.445597826086956,7.71570652173913" fill="#ffffff" class="js-draw-image-background"></path><text style="transform: matrix(0.0509511, 0, 0, 0.0509511, 4.86, 7.41); font-family: Courier; font-size: 30px; fill: rgb(255, 0, 0);" data-highp-transform="matrix(0.05095108695652172,0,0,0.05095108695652172,4.86,7.41)">Image</text></svg>
Side by Side

After

Width:  |  Height:  |  Size: 750 B

8
packages/app-desktop/integration-tests/resources/html-import/test-html-file-with-image.html Normal file
View File

@ -0,0 +1,8 @@
<!DOCTYPE html>
<html>
<body>
<h1>Test HTML file!</h1>
<p>This is a test file. Resource:</p>
<img src="./image.svg" alt="An SVG image."/>
</body>
</html>

21
packages/app-desktop/integration-tests/util/getImageSourceSize.ts Normal file
View File

@ -0,0 +1,21 @@
import { Locator } from '@playwright/test';
const getImageSourceSize = async (imageLocator: Locator) => {
// Use state: 'attached' -- we don't need the image to be on the screen (just present
// in the DOM).
await imageLocator.waitFor({ state: 'attached' });
// We load a copy of the image to avoid returning an overriden width set with
// .width = some_number
return await imageLocator.evaluate((originalImage: HTMLImageElement) => {
return new Promise<[number, number]>(resolve => {
const testImage = new Image();
testImage.onload = () => {
resolve([testImage.width, testImage.height]);
};
testImage.src = originalImage.src;
});
});
};
export default getImageSourceSize;

7
packages/renderer/HtmlToHtml.ts
View File

@ -92,7 +92,7 @@ export default class HtmlToHtml implements MarkupRenderer {
...options,
};
const cacheKey = md5(escape(JSON.stringify({ markup, options })));
const cacheKey = md5(escape(JSON.stringify({ markup, options, baseUrl: this.resourceBaseUrl_ })));
let html = this.cache_.value(cacheKey);
if (!html) {
@ -100,11 +100,10 @@ export default class HtmlToHtml implements MarkupRenderer {
allowedFilePrefixes: options.allowedFilePrefixes,
});
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
html = htmlUtils.processImageTags(html, (data: any) => {
html = htmlUtils.processImageTags(html, (data) => {
if (!data.src) return null;
const r = utils.imageReplacement(this.ResourceModel_, data.src, options.resources, this.resourceBaseUrl_, options.itemIdToUrl);
const r = utils.imageReplacement(this.ResourceModel_, data, options.resources, this.resourceBaseUrl_, options.itemIdToUrl);
if (!r) return null;
if (typeof r === 'string') {

23
packages/renderer/htmlUtils.ts
View File

@ -56,14 +56,31 @@ export const isSelfClosingTag = (tagName: string) => {
return selfClosingElements.includes(tagName.toLowerCase());
};
type ProcessImageResult = {
type: 'replaceElement';
html: string;
}|{
type: 'replaceSource';
src: string;
}|{
type: 'setAttributes';
attrs: Record<string, string>;
};
interface ProcessImageEvent {
src: string;
before: string;
after: string;
}
type ProcessImageCallback = (data: ProcessImageEvent)=> ProcessImageResult;
class HtmlUtils {
// eslint-disable-next-line @typescript-eslint/ban-types -- Old code before rule was applied
public processImageTags(html: string, callback: Function) {
public processImageTags(html: string, callback: ProcessImageCallback) {
if (!html) return '';
return html.replace(imageRegex, (_v, before, src, after) => {
const action = callback({ src: src });
const action = callback({ src, before, after });
if (!action) return `<img${before}src="${src}"${after}>`;
@ -80,7 +97,7 @@ class HtmlUtils {
return `<img${before}${attrHtml}${after}>`;
}
throw new Error(`Invalid action: ${action.type}`);
throw new Error(`Invalid action: ${(action as Record<string, string>).type}`);
});
}