Desktop: Fixes #11105: Plugin API: Save changes made with editor.setText (#11117)

.eslintignore

@@ -475,6 +475,7 @@ packages/app-desktop/integration-tests/models/NoteList.js
 packages/app-desktop/integration-tests/models/SettingsScreen.js
 packages/app-desktop/integration-tests/models/Sidebar.js
 packages/app-desktop/integration-tests/noteList.spec.js
+packages/app-desktop/integration-tests/pluginApi.spec.js
 packages/app-desktop/integration-tests/richTextEditor.spec.js
 packages/app-desktop/integration-tests/settings.spec.js
 packages/app-desktop/integration-tests/sidebar.spec.js

.gitignore

@@ -452,6 +452,7 @@ packages/app-desktop/integration-tests/models/NoteList.js
 packages/app-desktop/integration-tests/models/SettingsScreen.js
 packages/app-desktop/integration-tests/models/Sidebar.js
 packages/app-desktop/integration-tests/noteList.spec.js
+packages/app-desktop/integration-tests/pluginApi.spec.js
 packages/app-desktop/integration-tests/richTextEditor.spec.js
 packages/app-desktop/integration-tests/settings.spec.js
 packages/app-desktop/integration-tests/sidebar.spec.js

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

@@ -218,15 +218,6 @@ function NoteEditor(props: NoteEditorProps) {
 		}
 	}, [handleProvisionalFlag, formNote, setFormNote, isNewNote, titleHasBeenManuallyChanged, scheduleNoteListResort, scheduleSaveNote]);
 
-	useWindowCommandHandler({
-		dispatch: props.dispatch,
-		setShowLocalSearch,
-		noteSearchBarRef,
-		editorRef,
-		titleInputRef,
-		setFormNote,
-	});
-
 	const onDrop = useDropHandler({ editorRef });
 
 	const onBodyChange = useCallback((event: OnChangeEvent) => onFieldChange('body', event.content, event.changeId), [onFieldChange]);
@@ -234,6 +225,15 @@ function NoteEditor(props: NoteEditorProps) {
 	// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
 	const onTitleChange = useCallback((event: any) => onFieldChange('title', event.target.value), [onFieldChange]);
 
+	useWindowCommandHandler({
+		dispatch: props.dispatch,
+		setShowLocalSearch,
+		noteSearchBarRef,
+		editorRef,
+		titleInputRef,
+		onBodyChange,
+	});
+
 	// const onTitleKeydown = useCallback((event:any) => {
 	// 	const keyCode = event.keyCode;
 

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

@@ -1,5 +1,5 @@
 import { RefObject, useEffect } from 'react';
-import { FormNote, NoteBodyEditorRef, ScrollOptionTypes } from './types';
+import { NoteBodyEditorRef, OnChangeEvent, ScrollOptionTypes } from './types';
 import editorCommandDeclarations, { enabledCondition } from '../editorCommandDeclarations';
 import CommandService, { CommandDeclaration, CommandRuntime, CommandContext } from '@joplin/lib/services/CommandService';
 import time from '@joplin/lib/time';
@@ -12,7 +12,7 @@ const commandsWithDependencies = [
 	require('../commands/pasteAsText'),
 ];
 
-type SetFormNoteCallback = (callback: (prev: FormNote)=> FormNote)=> void;
+type OnBodyChange = (event: OnChangeEvent)=> void;
 
 interface HookDependencies {
 	// eslint-disable-next-line @typescript-eslint/ban-types -- Old code before rule was applied
@@ -23,13 +23,13 @@ interface HookDependencies {
 	noteSearchBarRef: any;
 	editorRef: RefObject<NoteBodyEditorRef>;
 	titleInputRef: RefObject<HTMLInputElement>;
-	setFormNote: SetFormNoteCallback;
+	onBodyChange: OnBodyChange;
 }
 
 function editorCommandRuntime(
 	declaration: CommandDeclaration,
 	editorRef: RefObject<NoteBodyEditorRef>,
-	setFormNote: SetFormNoteCallback,
+	onBodyChange: OnBodyChange,
 ): CommandRuntime {
 	return {
 		// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
@@ -55,9 +55,7 @@ function editorCommandRuntime(
 					value: args[0],
 				});
 			} else if (declaration.name === 'editor.setText') {
-				setFormNote((prev: FormNote) => {
-					return { ...prev, body: args[0] };
-				});
+				onBodyChange({ content: args[0], changeId: 0 });
 			} else {
 				return editorRef.current.execCommand({
 					name: declaration.name,
@@ -78,11 +76,11 @@ function editorCommandRuntime(
 }
 
 export default function useWindowCommandHandler(dependencies: HookDependencies) {
-	const { setShowLocalSearch, noteSearchBarRef, editorRef, titleInputRef, setFormNote } = dependencies;
+	const { setShowLocalSearch, noteSearchBarRef, editorRef, titleInputRef, onBodyChange } = dependencies;
 
 	useEffect(() => {
 		for (const declaration of editorCommandDeclarations) {
-			CommandService.instance().registerRuntime(declaration.name, editorCommandRuntime(declaration, editorRef, setFormNote));
+			CommandService.instance().registerRuntime(declaration.name, editorCommandRuntime(declaration, editorRef, onBodyChange));
 		}
 
 		const dependencies = {
@@ -105,5 +103,5 @@ export default function useWindowCommandHandler(dependencies: HookDependencies)
 				CommandService.instance().unregisterRuntime(command.declaration.name);
 			}
 		};
-	}, [editorRef, setShowLocalSearch, noteSearchBarRef, titleInputRef, setFormNote]);
+	}, [editorRef, setShowLocalSearch, noteSearchBarRef, titleInputRef, onBodyChange]);
 }

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

@@ -30,4 +30,16 @@ export default class GoToAnything {
 	public async expectToBeOpen() {
 		await expect(this.containerLocator).toBeAttached();
 	}
+
+	public async runCommand(electronApp: ElectronApplication, command: string) {
+		if (!command.startsWith(':')) {
+			command = `:${command}`;
+		}
+
+		await this.open(electronApp);
+		await this.inputLocator.fill(command);
+		await this.containerLocator.locator('.match-highlight').first().waitFor();
+		await this.inputLocator.press('Enter');
+		await this.expectToBeClosed();
+	}
 }

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

@@ -1,5 +1,6 @@
 
 import { Locator, Page } from '@playwright/test';
+import { expect } from '../util/test';
 
 export default class NoteEditorPage {
 	public readonly codeMirrorEditor: Locator;
@@ -31,6 +32,31 @@ export default class NoteEditorPage {
 		return this.containerLocator.getByRole('button', { name: title });
 	}
 
+	public async contentLocator() {
+		const richTextBody = this.getRichTextFrameLocator().locator('body');
+		const markdownEditor = this.codeMirrorEditor;
+
+		// Work around an issue where .or doesn't work with frameLocators.
+		// See https://github.com/microsoft/playwright/issues/27688#issuecomment-1771403495
+		await Promise.race([
+			richTextBody.waitFor({ state: 'visible' }).catch(()=>{}),
+			markdownEditor.waitFor({ state: 'visible' }).catch(()=>{}),
+		]);
+		if (await richTextBody.isVisible()) {
+			return richTextBody;
+		} else {
+			return markdownEditor;
+		}
+	}
+
+	public async expectToHaveText(content: string) {
+		// expect(...).toHaveText can fail in the Rich Text Editor (perhaps due to frame locators).
+		// Using expect.poll refreshes the locator on each attempt, which seems to prevent flakiness.
+		await expect.poll(
+			async () => (await this.contentLocator()).textContent(),
+		).toBe(content);
+	}
+
 	public getNoteViewerFrameLocator() {
 		// The note viewer can change content when the note re-renders. As such,
 		// a new locator needs to be created after re-renders (and this can't be a
@@ -38,7 +64,7 @@ export default class NoteEditorPage {
 		return this.noteViewerContainer.frameLocator(':scope');
 	}
 
-	public getTinyMCEFrameLocator() {
+	public getRichTextFrameLocator() {
 		// We use frameLocator(':scope') to convert the richTextEditor Locator into
 		// a FrameLocator. (:scope selects the locator itself).
 		// https://playwright.dev/docs/api/class-framelocator
@@ -53,4 +79,10 @@ export default class NoteEditorPage {
 		await this.noteTitleInput.waitFor();
 		await this.toggleEditorsButton.waitFor();
 	}
+
+	public async goBack() {
+		const backButton = this.toolbarButtonLocator('Back');
+		await expect(backButton).not.toBeDisabled();
+		await backButton.click();
+	}
 }

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

@@ -0,0 +1,32 @@
+
+import { test } from './util/test';
+import MainScreen from './models/MainScreen';
+
+test.describe('pluginApi', () => {
+	for (const richTextEditor of [false, true]) {
+		test(`the editor.setText command should update the current note (use RTE: ${richTextEditor})`, async ({ startAppWithPlugins }) => {
+			const { app, mainWindow } = await startAppWithPlugins(['resources/test-plugins/execCommand.js']);
+			const mainScreen = new MainScreen(mainWindow);
+			await mainScreen.createNewNote('First note');
+			const editor = mainScreen.noteEditor;
+
+			await editor.focusCodeMirrorEditor();
+			await mainWindow.keyboard.type('This content should be overwritten.');
+
+			if (richTextEditor) {
+				await editor.toggleEditorsButton.click();
+				await editor.richTextEditor.click();
+			}
+
+			await mainScreen.goToAnything.runCommand(app, 'testUpdateEditorText');
+			await editor.expectToHaveText('PASS');
+
+			// Should still have the same text after switching notes:
+			await mainScreen.createNewNote('Second note');
+			await editor.goBack();
+
+			await editor.expectToHaveText('PASS');
+		});
+	}
+});
+

packages/app-desktop/integration-tests/resources/test-plugins/execCommand.js

@@ -0,0 +1,31 @@
+// Allows referencing the Joplin global:
+/* eslint-disable no-undef */
+
+// Allows the `joplin-manifest` block comment:
+/* eslint-disable multiline-comment-style */
+
+/* joplin-manifest:
+{
+	"id": "org.joplinapp.plugins.example.execCommand",
+	"manifest_version": 1,
+	"app_min_version": "3.1",
+	"name": "JS Bundle test",
+	"description": "JS Bundle Test plugin",
+	"version": "1.0.0",
+	"author": "",
+	"homepage_url": "https://joplinapp.org"
+}
+*/
+
+joplin.plugins.register({
+	onStart: async function() {
+		await joplin.commands.register({
+			name: 'testUpdateEditorText',
+			label: 'Test setting the editor\'s text with editor.setText',
+			iconName: 'fas fa-drum',
+			execute: async () => {
+				await joplin.commands.execute('editor.setText', 'PASS');
+			},
+		});
+	},
+});

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

@@ -38,7 +38,7 @@ test.describe('richTextEditor', () => {
 		await editor.richTextEditor.waitFor();
 
 		// Edit the note to cause the original content to update
-		await editor.getTinyMCEFrameLocator().locator('a').click();
+		await editor.getRichTextFrameLocator().locator('a').click();
 		await mainWindow.keyboard.type('Test...');
 
 		await editor.toggleEditorsButton.click();
@@ -70,7 +70,7 @@ test.describe('richTextEditor', () => {
 
 		// Click on the attached file URL
 		const openPathResult = waitForNextOpenPath(electronApp);
-		const targetLink = editor.getTinyMCEFrameLocator().getByRole('link', { name: basename(pathToAttach) });
+		const targetLink = editor.getRichTextFrameLocator().getByRole('link', { name: basename(pathToAttach) });
 		if (process.platform === 'darwin') {
 			await targetLink.click({ modifiers: ['Meta'] });
 		} else {

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

@@ -6,10 +6,12 @@ import createStartupArgs from './createStartupArgs';
 import firstNonDevToolsWindow from './firstNonDevToolsWindow';
 
 
+type StartWithPluginsResult = { app: ElectronApplication; mainWindow: Page };
 
 type JoplinFixtures = {
 	profileDirectory: string;
 	electronApp: ElectronApplication;
+	startAppWithPlugins: (pluginPaths: string[])=> Promise<StartWithPluginsResult>;
 	startupPluginsLoaded: Promise<void>;
 	mainWindow: Page;
 };
@@ -17,6 +19,20 @@ type JoplinFixtures = {
 // A custom fixture that loads an electron app. See
 // https://playwright.dev/docs/test-fixtures
 
+const getAndResizeMainWindow = async (electronApp: ElectronApplication) => {
+	const mainWindow = await firstNonDevToolsWindow(electronApp);
+
+	// Setting the viewport size helps keep test environments consistent.
+	await mainWindow.setViewportSize({
+		width: 1200,
+		height: 800,
+	});
+
+	return mainWindow;
+};
+
+const testDir = dirname(__dirname);
+
 export const test = base.extend<JoplinFixtures>({
 	// Playwright fails if we don't use the object destructuring
 	// pattern in the first argument.
@@ -25,7 +41,7 @@ export const test = base.extend<JoplinFixtures>({
 	//
 	// eslint-disable-next-line no-empty-pattern
 	profileDirectory: async ({ }, use) => {
-		const profilePath = resolve(join(dirname(__dirname), 'test-profile'));
+		const profilePath = resolve(join(testDir, 'test-profile'));
 		const profileSubdir = join(profilePath, uuid.createNano());
 		await mkdirp(profileSubdir);
 
@@ -44,6 +60,34 @@ export const test = base.extend<JoplinFixtures>({
 		await electronApp.close();
 	},
 
+	startAppWithPlugins: async ({ profileDirectory }, use) => {
+		const startupArgs = createStartupArgs(profileDirectory);
+		let electronApp: ElectronApplication;
+
+		await use(async (pluginPaths: string[]) => {
+			if (electronApp) {
+				throw new Error('Electron app already created');
+			}
+			electronApp = await electron.launch({
+				args: [
+					...startupArgs,
+					'--dev-plugins',
+					pluginPaths.map(path => resolve(testDir, path)).join(','),
+				],
+			});
+
+			return {
+				app: electronApp,
+				mainWindow: await getAndResizeMainWindow(electronApp),
+			};
+		});
+
+		if (electronApp) {
+			await electronApp.firstWindow();
+			await electronApp.close();
+		}
+	},
+
 	startupPluginsLoaded: async ({ electronApp }, use) => {
 		const startupPluginsLoadedPromise = electronApp.evaluate(({ ipcMain }) => {
 			return new Promise<void>(resolve => {
@@ -55,15 +99,7 @@ export const test = base.extend<JoplinFixtures>({
 	},
 
 	mainWindow: async ({ electronApp }, use) => {
-		const mainWindow = await firstNonDevToolsWindow(electronApp);
-
-		// Setting the viewport size helps keep test environments consistent.
-		await mainWindow.setViewportSize({
-			width: 1200,
-			height: 800,
-		});
-
-		await use(mainWindow);
+		await use(await getAndResizeMainWindow(electronApp));
 	},
 });
 

packages/app-desktop/plugins/GotoAnything.tsx

@@ -543,11 +543,22 @@ class DialogComponent extends React.PureComponent<Props, State> {
 		const resultId = getResultId(item);
 		const isSelected = resultId === this.state.selectedItemId;
 		const rowStyle = isSelected ? style.rowSelected : style.row;
+
+		const wrapKeywordMatches = (unescapedContent: string) => {
+			return surroundKeywords(
+				this.state.keywords,
+				unescapedContent,
+				`<span class="match-highlight" style="font-weight: bold; color: ${theme.searchMarkerColor}; background-color: ${theme.searchMarkerBackgroundColor}">`,
+				'</span>',
+				{ escapeHtml: true },
+			);
+		};
+
 		const titleHtml = item.fragments
 			? `<span style="font-weight: bold; color: ${theme.color};">${item.title}</span>`
-			: surroundKeywords(this.state.keywords, item.title, `<span style="font-weight: bold; color: ${theme.searchMarkerColor}; background-color: ${theme.searchMarkerBackgroundColor}">`, '</span>', { escapeHtml: true });
+			: wrapKeywordMatches(item.title);
 
-		const fragmentsHtml = !item.fragments ? null : surroundKeywords(this.state.keywords, item.fragments, `<span style="color: ${theme.searchMarkerColor}; background-color: ${theme.searchMarkerBackgroundColor}">`, '</span>', { escapeHtml: true });
+		const fragmentsHtml = !item.fragments ? null : wrapKeywordMatches(item.fragments);
 
 		const folderIcon = <i style={{ fontSize: theme.fontSize, marginRight: 2 }} className="fa fa-book" role='img' aria-label={_('Notebook')} />;
 		const pathComp = !item.path ? null : <div style={style.rowPath}>{folderIcon} {item.path}</div>;