Desktop: Fixes #9304: Fix HTML resource links lost when editing notes in the rich text editor (#9435)

.eslintignore

@@ -387,6 +387,7 @@ packages/app-desktop/integration-tests/models/SettingsScreen.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/setFilePickerResponse.js
 packages/app-desktop/integration-tests/util/test.js
 packages/app-desktop/playwright.config.js
 packages/app-desktop/plugins/GotoAnything.js

.gitignore

@@ -369,6 +369,7 @@ packages/app-desktop/integration-tests/models/SettingsScreen.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/setFilePickerResponse.js
 packages/app-desktop/integration-tests/util/test.js
 packages/app-desktop/playwright.config.js
 packages/app-desktop/plugins/GotoAnything.js

packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/TinyMCE.tsx

@@ -857,7 +857,18 @@ const TinyMCE = (props: NoteBodyEditorProps, ref: any) => {
 			const resourcesEqual = resourceInfosEqual(lastOnChangeEventInfo.current.resourceInfos, props.resourceInfos);
 
 			if (lastOnChangeEventInfo.current.content !== props.content || !resourcesEqual) {
-				const result = await props.markupToHtml(props.contentMarkupLanguage, props.content, markupRenderOptions({ resourceInfos: props.resourceInfos }));
+				const result = await props.markupToHtml(
+					props.contentMarkupLanguage,
+					props.content,
+					markupRenderOptions({
+						resourceInfos: props.resourceInfos,
+
+						// Allow file:// URLs that point to the resource directory.
+						// This prevents HTML-style resource URLs (e.g. <a href="file://path/to/resource/.../"></a>)
+						// from being discarded.
+						allowedFilePrefixes: [props.resourceDirectory],
+					}),
+				);
 				if (cancelled) return;
 
 				editor.setContent(awfulBrHack(result.html));

packages/app-desktop/gui/NoteEditor/NoteEditor.tsx

@@ -439,6 +439,7 @@ function NoteEditor(props: NoteEditorProps) {
 		contentMarkupLanguage: formNote.markup_language,
 		contentOriginalCss: formNote.originalCss,
 		resourceInfos: resourceInfos,
+		resourceDirectory: Setting.value('resourceDir'),
 		htmlToMarkdown: htmlToMarkdown,
 		markupToHtml: markupToHtml,
 		allAssets: allAssets,

packages/app-desktop/gui/NoteEditor/utils/types.ts

@@ -82,6 +82,7 @@ export interface NoteBodyEditorProps {
 	visiblePanes: string[];
 	keyboardMode: string;
 	resourceInfos: ResourceInfos;
+	resourceDirectory: string;
 	locale: string;
 	// eslint-disable-next-line @typescript-eslint/ban-types -- Old code before rule was applied
 	onDrop: Function;

packages/app-desktop/gui/NoteEditor/utils/useMarkupToHtml.ts

@@ -26,6 +26,7 @@ export interface MarkupToHtmlOptions {
 	noteId?: string;
 	vendorDir?: string;
 	platformName?: string;
+	allowedFilePrefixes?: string[];
 }
 
 export default function useMarkupToHtml(deps: HookDependencies) {

packages/app-desktop/integration-tests/main.spec.ts

@@ -7,6 +7,7 @@ import { writeFile } from 'fs-extra';
 import { join } from 'path';
 import createStartupArgs from './util/createStartupArgs';
 import firstNonDevToolsWindow from './util/firstNonDevToolsWindow';
+import setFilePickerResponse from './util/setFilePickerResponse';
 
 
 test.describe('main', () => {
@@ -20,17 +21,7 @@ test.describe('main', () => {
 
 	test('should be able to create and edit a new note', async ({ mainWindow }) => {
 		const mainScreen = new MainScreen(mainWindow);
-		await mainScreen.newNoteButton.click();
-
-		const editor = mainScreen.noteEditor;
-		await editor.waitFor();
-
-		// Wait for the title input to have the correct placeholder
-		await mainWindow.locator('input[placeholder^="Creating new note"]').waitFor();
-
-		// Fill the title
-		await editor.noteTitleInput.click();
-		await editor.noteTitleInput.fill('Test note');
+		const editor = await mainScreen.createNewNote('Test note');
 
 		// Note list should contain the new note
 		await expect(mainScreen.noteListContainer.getByText('Test note')).toBeVisible();
@@ -49,6 +40,50 @@ test.describe('main', () => {
 		await expect(viewerFrame.locator('h1')).toHaveText('Test note!');
 	});
 
+	test('HTML links should be preserved when editing a note in the WYSIWYG editor', async ({ electronApp, mainWindow }) => {
+		const mainScreen = new MainScreen(mainWindow);
+		await mainScreen.createNewNote('Testing!');
+		const editor = mainScreen.noteEditor;
+
+		// Set the note's content
+		await editor.codeMirrorEditor.click();
+
+		// Attach this file to the note (create a resource ID)
+		await setFilePickerResponse(electronApp, [__filename]);
+		await editor.attachFileButton.click();
+
+		// Wait to render
+		const viewerFrame = editor.getNoteViewerIframe();
+		await viewerFrame.locator('a[data-from-md]').waitFor();
+
+		// Should have an attached resource
+		const codeMirrorContent = await editor.codeMirrorEditor.innerText();
+
+		const resourceUrlExpression = /\[.*\]\(:\/(\w+)\)/;
+		expect(codeMirrorContent).toMatch(resourceUrlExpression);
+		const resourceId = codeMirrorContent.match(resourceUrlExpression)[1];
+
+		// Create a new note with just an HTML link
+		await mainScreen.createNewNote('Another test');
+		await editor.codeMirrorEditor.click();
+		await mainWindow.keyboard.type(`<a href=":/${resourceId}">HTML Link</a>`);
+
+		// Switch to the RTE
+		await editor.toggleEditorsButton.click();
+		await editor.richTextEditor.waitFor();
+
+		// Edit the note to cause the original content to update
+		await editor.getTinyMCEFrameLocator().locator('a').click();
+		await mainWindow.keyboard.type('Test...');
+
+		await editor.toggleEditorsButton.click();
+		await editor.codeMirrorEditor.waitFor();
+
+		// Note should still contain the resource ID and note title
+		const finalCodeMirrorContent = await editor.codeMirrorEditor.innerText();
+		expect(finalCodeMirrorContent).toContain(`:/${resourceId}`);
+	});
+
 	test('should be possible to remove sort order buttons in settings', async ({ electronApp, mainWindow }) => {
 		const mainScreen = new MainScreen(mainWindow);
 		await mainScreen.waitFor();

packages/app-desktop/integration-tests/models/MainScreen.ts

@@ -6,7 +6,7 @@ export default class MainScreen {
 	public readonly noteListContainer: Locator;
 	public readonly noteEditor: NoteEditorScreen;
 
-	public constructor(page: Page) {
+	public constructor(private page: Page) {
 		this.newNoteButton = page.locator('.new-note-button');
 		this.noteListContainer = page.locator('.rli-noteList');
 		this.noteEditor = new NoteEditorScreen(page);
@@ -17,4 +17,20 @@ export default class MainScreen {
 		await this.noteEditor.waitFor();
 		await this.noteListContainer.waitFor();
 	}
+
+	// Follows the steps a user would use to create a new note.
+	public async createNewNote(title: string) {
+		await this.waitFor();
+		await this.newNoteButton.click();
+		await this.noteEditor.waitFor();
+
+		// Wait for the title input to have the correct placeholder
+		await this.page.locator('input[placeholder^="Creating new note"]').waitFor();
+
+		// Fill the title
+		await this.noteEditor.noteTitleInput.click();
+		await this.noteEditor.noteTitleInput.fill(title);
+
+		return this.noteEditor;
+	}
 }

packages/app-desktop/integration-tests/models/NoteEditorScreen.ts

@@ -3,13 +3,19 @@ import { Locator, Page } from '@playwright/test';
 
 export default class NoteEditorPage {
 	public readonly codeMirrorEditor: Locator;
+	public readonly richTextEditor: Locator;
 	public readonly noteTitleInput: Locator;
+	public readonly attachFileButton: Locator;
+	public readonly toggleEditorsButton: Locator;
 	private readonly containerLocator: Locator;
 
 	public constructor(private readonly page: Page) {
 		this.containerLocator = page.locator('.rli-editor');
 		this.codeMirrorEditor = this.containerLocator.locator('.codeMirrorEditor');
+		this.richTextEditor = this.containerLocator.locator('iframe[title="Rich Text Area"]');
 		this.noteTitleInput = this.containerLocator.locator('.title-input');
+		this.attachFileButton = this.containerLocator.locator('[title^="Attach file"]');
+		this.toggleEditorsButton = this.containerLocator.locator('[title^="Toggle editors"]');
 	}
 
 	public getNoteViewerIframe() {
@@ -19,8 +25,15 @@ export default class NoteEditorPage {
 		return this.page.frame({ url: /.*note-viewer[/\\]index.html.*/ });
 	}
 
+	public getTinyMCEFrameLocator() {
+		// We use frameLocator(':scope') to convert the richTextEditor Locator into
+		// a FrameLocator. (:scope selects the locator itself).
+		// https://playwright.dev/docs/api/class-framelocator
+		return this.richTextEditor.frameLocator(':scope');
+	}
+
 	public async waitFor() {
-		await this.codeMirrorEditor.waitFor();
 		await this.noteTitleInput.waitFor();
+		await this.toggleEditorsButton.waitFor();
 	}
 }

packages/app-desktop/integration-tests/util/setFilePickerResponse.ts

@@ -0,0 +1,13 @@
+import { ElectronApplication } from '@playwright/test';
+
+const setFilePickerResponse = (electronApp: ElectronApplication, response: string[]) => {
+	return electronApp.evaluate(async ({ dialog }, response) => {
+		dialog.showOpenDialog = async () => ({
+			canceled: false,
+			filePaths: response,
+		});
+		dialog.showOpenDialogSync = () => response;
+	}, response);
+};
+
+export default setFilePickerResponse;

packages/renderer/HtmlToHtml.ts

@@ -38,6 +38,7 @@ interface RenderOptions {
 	postMessageSyntax: string;
 	enableLongPress: boolean;
 	itemIdToUrl?: ItemIdToUrlHandler;
+	allowedFilePrefixes?: string[];
 }
 
 // https://github.com/es-shims/String.prototype.trimStart/blob/main/implementation.js
@@ -99,7 +100,9 @@ export default class HtmlToHtml {
 		let html = this.cache_.value(cacheKey);
 
 		if (!html) {
-			html = htmlUtils.sanitizeHtml(markup);
+			html = htmlUtils.sanitizeHtml(markup, {
+				allowedFilePrefixes: options.allowedFilePrefixes,
+			});
 
 			html = htmlUtils.processImageTags(html, (data: any) => {
 				if (!data.src) return null;

packages/renderer/MdToHtml.ts

@@ -192,6 +192,10 @@ export interface RuleOptions {
 	vendorDir?: string;
 	itemIdToUrl?: ItemIdToUrlHandler;
 
+	// Passed to the HTML sanitizer: Allows file:// URLs with
+	// paths with the included prefixes.
+	allowedFilePrefixes?: string[];
+
 	platformName?: string;
 }
 

packages/renderer/MdToHtml/rules/sanitize_html.ts

@@ -34,7 +34,13 @@ export default {
 					// So the sanitizeHtml function must handle this kind of non-valid HTML.
 
 					if (!sanitizedContent) {
-						sanitizedContent = htmlUtils.sanitizeHtml(token.content, { addNoMdConvClass: true });
+						sanitizedContent = htmlUtils.sanitizeHtml(
+							token.content,
+							{
+								addNoMdConvClass: true,
+								allowedFilePrefixes: ruleOptions.allowedFilePrefixes,
+							},
+						);
 					}
 
 					token.content = sanitizedContent;

packages/renderer/htmlUtils.ts

@@ -189,10 +189,12 @@ class HtmlUtils {
 			// If true, adds a "jop-noMdConv" class to all the tags.
 			// It can be used afterwards to restore HTML tags in Markdown.
 			addNoMdConvClass: false,
-			allowedFilePrefixes: [],
 			...options,
 		};
 
+		// If options.allowedFilePrefixes is `undefined`, default to [].
+		options.allowedFilePrefixes ??= [];
+
 		const output: string[] = [];
 
 		const tagStack: string[] = [];