Desktop: Fixes #10551: Watch resources for changes when opened from the Rich Text Editor (#10554)

.eslintignore

@@ -455,6 +455,7 @@ packages/app-desktop/integration-tests/models/MainScreen.js
 packages/app-desktop/integration-tests/models/NoteEditorScreen.js
 packages/app-desktop/integration-tests/models/SettingsScreen.js
 packages/app-desktop/integration-tests/models/Sidebar.js
+packages/app-desktop/integration-tests/richTextEditor.spec.js
 packages/app-desktop/integration-tests/sidebar.spec.js
 packages/app-desktop/integration-tests/simpleBackup.spec.js
 packages/app-desktop/integration-tests/util/activateMainMenuItem.js
@@ -463,6 +464,7 @@ packages/app-desktop/integration-tests/util/firstNonDevToolsWindow.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
+packages/app-desktop/integration-tests/util/waitForNextOpenPath.js
 packages/app-desktop/playwright.config.js
 packages/app-desktop/plugins/GotoAnything.js
 packages/app-desktop/services/bridge.js
@@ -1221,6 +1223,7 @@ packages/lib/themes/solarizedLight.js
 packages/lib/themes/type.js
 packages/lib/time.js
 packages/lib/types.js
+packages/lib/urlUtils.js
 packages/lib/utils/ActionLogger.test.js
 packages/lib/utils/ActionLogger.js
 packages/lib/utils/credentialFiles.js

.gitignore

@@ -434,6 +434,7 @@ packages/app-desktop/integration-tests/models/MainScreen.js
 packages/app-desktop/integration-tests/models/NoteEditorScreen.js
 packages/app-desktop/integration-tests/models/SettingsScreen.js
 packages/app-desktop/integration-tests/models/Sidebar.js
+packages/app-desktop/integration-tests/richTextEditor.spec.js
 packages/app-desktop/integration-tests/sidebar.spec.js
 packages/app-desktop/integration-tests/simpleBackup.spec.js
 packages/app-desktop/integration-tests/util/activateMainMenuItem.js
@@ -442,6 +443,7 @@ packages/app-desktop/integration-tests/util/firstNonDevToolsWindow.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
+packages/app-desktop/integration-tests/util/waitForNextOpenPath.js
 packages/app-desktop/playwright.config.js
 packages/app-desktop/plugins/GotoAnything.js
 packages/app-desktop/services/bridge.js
@@ -1200,6 +1202,7 @@ packages/lib/themes/solarizedLight.js
 packages/lib/themes/type.js
 packages/lib/time.js
 packages/lib/types.js
+packages/lib/urlUtils.js
 packages/lib/utils/ActionLogger.test.js
 packages/lib/utils/ActionLogger.js
 packages/lib/utils/credentialFiles.js

packages/app-desktop/gui/MainScreen/commands/openItem.ts

@@ -3,9 +3,10 @@ import shim from '@joplin/lib/shim';
 import { _ } from '@joplin/lib/locale';
 import bridge from '../../../services/bridge';
 import { openItemById } from '../../NoteEditor/utils/contextMenu';
-const { parseResourceUrl, urlProtocol } = require('@joplin/lib/urlUtils');
+import { fileUrlToResourceUrl, parseResourceUrl, urlProtocol } from '@joplin/lib/urlUtils';
 import { fileUriToPath } from '@joplin/utils/url';
-const { urlDecode } = require('@joplin/lib/string-utils');
+import { urlDecode } from '@joplin/lib/string-utils';
+import Setting from '@joplin/lib/models/Setting';
 
 export const declaration: CommandDeclaration = {
 	name: 'openItem',
@@ -16,6 +17,11 @@ export const runtime = (): CommandRuntime => {
 		execute: async (context: CommandContext, link: string) => {
 			if (!link) throw new Error('Link cannot be empty');
 
+			const fromFileUrl = fileUrlToResourceUrl(link, Setting.value('resourceDir'));
+			if (fromFileUrl) {
+				link = fromFileUrl;
+			}
+
 			if (link.startsWith('joplin://') || link.startsWith(':/')) {
 				const parsedUrl = parseResourceUrl(link);
 				if (parsedUrl) {

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

@@ -89,50 +89,6 @@ test.describe('main', () => {
 		await expect(viewerFrame.locator('.joplin-editable > .katex').first()).toBeAttached();
 	});
 
-	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.focusCodeMirrorEditor();
-
-		// 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 correctly resize large images', async ({ electronApp, mainWindow }) => {
 		const mainScreen = new MainScreen(mainWindow);
 		await mainScreen.createNewNote('Image resize test (part 1)');

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

@@ -0,0 +1,86 @@
+import { test, expect } from './util/test';
+import MainScreen from './models/MainScreen';
+import setFilePickerResponse from './util/setFilePickerResponse';
+import waitForNextOpenPath from './util/waitForNextOpenPath';
+import { basename } from 'path';
+
+test.describe('richTextEditor', () => {
+	test('HTML links should be preserved when editing a note', async ({ electronApp, mainWindow }) => {
+		const mainScreen = new MainScreen(mainWindow);
+		await mainScreen.createNewNote('Testing!');
+		const editor = mainScreen.noteEditor;
+
+		// Set the note's content
+		await editor.focusCodeMirrorEditor();
+
+		// 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 watch resources for changes when opened with ctrl+click', async ({ electronApp, mainWindow }) => {
+		const mainScreen = new MainScreen(mainWindow);
+		await mainScreen.createNewNote('Testing!');
+		const editor = mainScreen.noteEditor;
+
+		// Set the note's content
+		await editor.focusCodeMirrorEditor();
+
+		// Attach this file to the note (create a resource ID)
+		const pathToAttach = __filename;
+		await setFilePickerResponse(electronApp, [pathToAttach]);
+		await editor.attachFileButton.click();
+
+		// Switch to the RTE
+		await editor.toggleEditorsButton.click();
+		await editor.richTextEditor.waitFor();
+
+		await editor.richTextEditor.click();
+
+		// Click on the attached file URL
+		const openPathResult = waitForNextOpenPath(electronApp);
+		const targetLink = editor.getTinyMCEFrameLocator().getByRole('link', { name: basename(pathToAttach) });
+		if (process.platform === 'darwin') {
+			await targetLink.click({ modifiers: ['Meta'] });
+		} else {
+			await targetLink.click({ modifiers: ['Control'] });
+		}
+
+		// Should watch the file
+		await mainWindow.getByText(/^The following attachments are being watched for changes/i).waitFor();
+		expect(await openPathResult).toContain(basename(pathToAttach));
+	});
+
+});
+

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

@@ -0,0 +1,16 @@
+import { ElectronApplication } from '@playwright/test';
+
+const waitForNextOpenPath = (electronApp: ElectronApplication) => {
+	return electronApp.evaluate(async ({ shell }) => {
+		return new Promise<string>(resolve => {
+			const originalOpenPath = shell.openPath;
+			shell.openPath = async (path: string) => {
+				shell.openPath = originalOpenPath;
+				resolve(path);
+				return '';
+			};
+		});
+	});
+};
+
+export default waitForNextOpenPath;

packages/app-mobile/commands/openItem.ts

@@ -1,7 +1,7 @@
 import { CommandRuntime, CommandDeclaration, CommandContext } from '@joplin/lib/services/CommandService';
 import shim from '@joplin/lib/shim';
 import { _ } from '@joplin/lib/locale';
-const { parseResourceUrl, urlProtocol } = require('@joplin/lib/urlUtils');
+import { parseResourceUrl, urlProtocol } from '@joplin/lib/urlUtils';
 import Logger from '@joplin/utils/Logger';
 import goToNote from './util/goToNote';
 

packages/app-mobile/components/screens/Note.tsx

@@ -63,7 +63,7 @@ import pickDocument from '../../utils/pickDocument';
 import debounce from '../../utils/debounce';
 import { focus } from '@joplin/lib/utils/focusHandler';
 import CommandService from '@joplin/lib/services/CommandService';
-const urlUtils = require('@joplin/lib/urlUtils');
+import * as urlUtils from '@joplin/lib/urlUtils';
 
 // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
 const emptyArray: any[] = [];

packages/lib/urlUtils.test.js

@@ -52,6 +52,26 @@ describe('urlUtils', () => {
 		}
 	}));
 
+	it.each([
+		[
+			'file:///home/builder/.config/joplindev-desktop/profile-owmhbsat/resources/4a12670298dd46abbb140ffc8a10b583.md',
+			'/home/builder/.config/joplindev-desktop/profile-owmhbsat/resources',
+			{ itemId: '4a12670298dd46abbb140ffc8a10b583', hash: '' },
+		],
+		[
+			'file:///home/builder/.config/joplindev-desktop/profile-owmhbsat/resources/4a12670298dd46abbb140ffc8a10b583.md5#foo',
+			'/home/builder/.config/joplindev-desktop/profile-owmhbsat/resources',
+			{ itemId: '4a12670298dd46abbb140ffc8a10b583', hash: 'foo' },
+		],
+		[
+			'file:///home/builder/.config/joplindev-desktop/profile-owmhbsat/resources/4a12670298dd46abbb140ffc8a10b583.png?t=12345',
+			'/home/builder/.config/joplindev-desktop/profile-owmhbsat/resources',
+			{ itemId: '4a12670298dd46abbb140ffc8a10b583', hash: '' },
+		],
+	])('should detect resource file URLs', (url, resourceDir, expected) => {
+		expect(urlUtils.parseResourceUrl(urlUtils.fileUrlToResourceUrl(url, resourceDir))).toMatchObject(expected);
+	});
+
 	it('should extract resource URLs', (async () => {
 		const testCases = [
 			['Bla [](:/11111111111111111111111111111111) bla [](:/22222222222222222222222222222222) bla', ['11111111111111111111111111111111', '22222222222222222222222222222222']],

packages/lib/urlUtils.ts

@@ -1,53 +1,52 @@
-const { rtrimSlashes } = require('./path-utils');
+import { rtrimSlashes, toFileProtocolPath } from './path-utils';
-const { urlDecode } = require('./string-utils');
+import { urlDecode } from './string-utils';
 
-const urlUtils = {};
+export const hash = (url: string) => {
-
-urlUtils.hash = function(url) {
 	const s = url.split('#');
 	if (s.length <= 1) return '';
 	return s[s.length - 1];
 };
 
-urlUtils.urlWithoutPath = function(url) {
+export const urlWithoutPath = (url: string) => {
 	const parsed = require('url').parse(url, true);
 	return `${parsed.protocol}//${parsed.host}`;
 };
 
-urlUtils.urlProtocol = function(url) {
+export const urlProtocol = (url: string) => {
 	if (!url) return '';
 	const parsed = require('url').parse(url, true);
 	return parsed.protocol;
 };
 
-urlUtils.prependBaseUrl = function(url, baseUrl) {
+export const prependBaseUrl = (url: string, baseUrl: string) => {
 	baseUrl = rtrimSlashes(baseUrl).trim(); // All the code below assumes that the baseUrl does not end up with a slash
 	url = url.trim();
 
 	if (!url) url = '';
 	if (!baseUrl) return url;
 	if (url.indexOf('#') === 0) return url; // Don't prepend if it's a local anchor
-	if (urlUtils.urlProtocol(url)) return url; // Don't prepend the base URL if the URL already has a scheme
+	if (urlProtocol(url)) return url; // Don't prepend the base URL if the URL already has a scheme
 
 	if (url.length >= 2 && url.indexOf('//') === 0) {
 		// If it starts with // it's a protcol-relative URL
-		return urlUtils.urlProtocol(baseUrl) + url;
+		return urlProtocol(baseUrl) + url;
 	} else if (url && url[0] === '/') {
 		// If it starts with a slash, it's an absolute URL so it should be relative to the domain (and not to the full baseUrl)
-		return urlUtils.urlWithoutPath(baseUrl) + url;
+		return urlWithoutPath(baseUrl) + url;
 	} else {
 		return baseUrl + (url ? `/${url}` : '');
 	}
 };
 
+
 const resourceRegex = /^(joplin:\/\/|:\/)([0-9a-zA-Z]{32})(|#[^\s]*)(|\s".*?")$/;
 
-urlUtils.isResourceUrl = function(url) {
+export const isResourceUrl = (url: string) => {
 	return !!url.match(resourceRegex);
 };
 
-urlUtils.parseResourceUrl = function(url) {
+export const parseResourceUrl = (url: string) => {
-	if (!urlUtils.isResourceUrl(url)) return null;
+	if (!isResourceUrl(url)) return null;
 
 	const match = url.match(resourceRegex);
 
@@ -65,13 +64,35 @@ urlUtils.parseResourceUrl = function(url) {
 	};
 };
 
-urlUtils.extractResourceUrls = function(text) {
+export const fileUrlToResourceUrl = (fileUrl: string, resourceDir: string) => {
+	let resourceDirUrl = toFileProtocolPath(resourceDir);
+	if (!resourceDirUrl.endsWith('/')) {
+		resourceDirUrl += '/';
+	}
+
+	if (fileUrl.startsWith(resourceDirUrl)) {
+		let result = fileUrl.substring(resourceDirUrl.length);
+		// Remove the timestamp parameter, keep the hash.
+		result = result.replace(/\?t=\d+(#.*)?$/, '$1');
+		// Remove the file extension.
+		result = result.replace(/\.[a-z0-9]+(#.*)?$/, '$1');
+		result = `joplin://${result}`;
+
+		if (isResourceUrl(result)) {
+			return result;
+		}
+	}
+
+	return null;
+};
+
+export const extractResourceUrls = (text: string) => {
 	const markdownLinksRE = /\]\((.*?)\)/g;
 	const output = [];
 	let result = null;
 
 	while ((result = markdownLinksRE.exec(text)) !== null) {
-		const resourceUrlInfo = urlUtils.parseResourceUrl(result[1]);
+		const resourceUrlInfo = parseResourceUrl(result[1]);
 		if (resourceUrlInfo) output.push(resourceUrlInfo);
 	}
 
@@ -91,7 +112,7 @@ urlUtils.extractResourceUrls = function(text) {
 	return output;
 };
 
-urlUtils.objectToQueryString = function(query) {
+export const objectToQueryString = (query: Record<string, string>) => {
 	if (!query) return '';
 
 	let queryString = '';
@@ -105,4 +126,3 @@ urlUtils.objectToQueryString = function(query) {
 	return queryString;
 };
 
-module.exports = urlUtils;