Desktop: Fixes #9932: Fix only one CodeMirror 6 content script loaded per plugin (#9934)

packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/v6/Editor.tsx

@@ -1,7 +1,7 @@
 import * as React from 'react';
 import { ForwardedRef } from 'react';
 import { useEffect, useState, useRef, forwardRef, useImperativeHandle } from 'react';
-import { EditorProps, LogMessageCallback, OnEventCallback, PluginData } from '@joplin/editor/types';
+import { EditorProps, LogMessageCallback, OnEventCallback, ContentScriptData } from '@joplin/editor/types';
 import createEditor from '@joplin/editor/CodeMirror/createEditor';
 import CodeMirrorControl from '@joplin/editor/CodeMirror/CodeMirrorControl';
 import { PluginStates } from '@joplin/lib/services/plugins/reducer';
@@ -57,13 +57,13 @@ const Editor = (props: Props, ref: ForwardedRef<CodeMirrorControl>) => {
 			return;
 		}
 
-		const plugins: PluginData[] = [];
+		const contentScripts: ContentScriptData[] = [];
 		for (const pluginId in props.pluginStates) {
 			const pluginState = props.pluginStates[pluginId];
 			const codeMirrorContentScripts = pluginState.contentScripts[ContentScriptType.CodeMirrorPlugin] ?? [];
 
 			for (const contentScript of codeMirrorContentScripts) {
-				plugins.push({
+				contentScripts.push({
 					pluginId,
 					contentScriptId: contentScript.id,
 					contentScriptJs: () => shim.fsDriver().readFile(contentScript.path),
@@ -80,7 +80,7 @@ const Editor = (props: Props, ref: ForwardedRef<CodeMirrorControl>) => {
 			}
 		}
 
-		void editor.setPlugins(plugins);
+		void editor.setContentScripts(contentScripts);
 	}, [editor, props.pluginStates]);
 
 	useEffect(() => {

packages/app-mobile/components/NoteEditor/NoteEditor.tsx

@@ -15,7 +15,7 @@ import { EditorControl, EditorSettings, SelectionRange } from './types';
 import { _ } from '@joplin/lib/locale';
 import MarkdownToolbar from './MarkdownToolbar/MarkdownToolbar';
 import { ChangeEvent, EditorEvent, EditorEventType, SelectionRangeChangeEvent, UndoRedoDepthChangeEvent } from '@joplin/editor/events';
-import { EditorCommandType, EditorKeymap, EditorLanguageType, PluginData, SearchState } from '@joplin/editor/types';
+import { EditorCommandType, EditorKeymap, EditorLanguageType, ContentScriptData, SearchState } from '@joplin/editor/types';
 import supportsCommand from '@joplin/editor/CodeMirror/editorCommands/supportsCommand';
 import SelectionFormatting, { defaultSelectionFormatting } from '@joplin/editor/SelectionFormatting';
 import Logger from '@joplin/utils/Logger';
@@ -240,8 +240,8 @@ const useEditorControl = (
 				injectJS('document.activeElement?.blur();');
 			},
 
-			setPlugins: async (plugins: PluginData[]) => {
-				injectJS(`cm.setPlugins(${JSON.stringify(plugins)});`);
+			setContentScripts: async (plugins: ContentScriptData[]) => {
+				injectJS(`cm.setContentScripts(${JSON.stringify(plugins)});`);
 			},
 
 			setSearchState: setSearchStateCallback,

packages/editor/CodeMirror/CodeMirrorControl.ts

@@ -1,5 +1,5 @@
 import { EditorView } from '@codemirror/view';
-import { EditorCommandType, EditorControl, EditorSettings, LogMessageCallback, PluginData, SearchState } from '../types';
+import { EditorCommandType, EditorControl, EditorSettings, LogMessageCallback, ContentScriptData, SearchState } from '../types';
 import CodeMirror5Emulation from './CodeMirror5Emulation/CodeMirror5Emulation';
 import editorCommands, { EditorCommandFunction } from './editorCommands/editorCommands';
 import { EditorSelection, Extension, StateEffect } from '@codemirror/state';
@@ -136,7 +136,7 @@ export default class CodeMirrorControl extends CodeMirror5Emulation implements E
 		});
 	}
 
-	public setPlugins(plugins: PluginData[]) {
+	public setContentScripts(plugins: ContentScriptData[]) {
 		return this._pluginControl.setPlugins(plugins);
 	}
 

packages/editor/CodeMirror/createEditor.test.ts

@@ -14,6 +14,10 @@ import createEditorSettings from './testUtil/createEditorSettings';
 describe('createEditor', () => {
 	beforeAll(() => {
 		jest.useFakeTimers();
+
+		for (const scriptContainer of document.querySelectorAll('#joplin-plugin-scripts-container')) {
+			scriptContainer.remove();
+		}
 	});
 
 	// This checks for a regression -- occasionally, when updating packages,
@@ -89,7 +93,7 @@ describe('createEditor', () => {
 		};
 
 		// Should be able to load a plugin
-		await editor.setPlugins([
+		await editor.setContentScripts([
 			testPlugin1,
 		]);
 
@@ -99,14 +103,14 @@ describe('createEditor', () => {
 		// Because plugin loading is done by adding script elements to the document,
 		// we test for the presence of these script elements, rather than waiting for
 		// them to run.
-		expect(document.querySelectorAll('#joplin-plugin-scripts-container')).toHaveLength(1);
+		expect(document.querySelectorAll('#joplin-plugin-scripts-container script')).toHaveLength(1);
 
 		// Only one script should be present.
 		const scriptContainer = document.querySelector('#joplin-plugin-scripts-container');
 		expect(scriptContainer.querySelectorAll('script')).toHaveLength(1);
 
 		// Adding another plugin should add another script element
-		await editor.setPlugins([
+		await editor.setContentScripts([
 			testPlugin2, testPlugin1,
 		]);
 		await jest.runAllTimersAsync();
@@ -116,6 +120,53 @@ describe('createEditor', () => {
 
 		// Removing the editor should remove the script container
 		editor.remove();
-		expect(document.querySelectorAll('#joplin-plugin-scripts-container')).toHaveLength(0);
+		expect(document.querySelectorAll('#joplin-plugin-scripts-container script')).toHaveLength(0);
+	});
+
+	it('should support multiple content scripts from the same plugin', async () => {
+		const initialText = '# Test\nThis is a test.';
+		const editorSettings = createEditorSettings(Setting.THEME_LIGHT);
+
+		const editor = createEditor(document.body, {
+			initialText,
+			settings: editorSettings,
+			onEvent: _event => {},
+			onLogMessage: _message => {},
+		});
+
+		const getContentScriptJs = jest.fn(async () => {
+			return `
+				exports.default = context => {
+					context.postMessage(context.pluginId);
+				};
+			`;
+		});
+		const postMessageHandler = jest.fn();
+
+		const pluginId = 'a.plugin.id';
+		const testPlugin1 = {
+			pluginId,
+			contentScriptId: 'a.plugin.id.contentScript',
+			loadCssAsset: async (_name: string) => '',
+			contentScriptJs: getContentScriptJs,
+			postMessageHandler,
+		};
+		const testPlugin2 = {
+			pluginId,
+			contentScriptId: 'another.plugin.id.contentScript',
+			loadCssAsset: async (_name: string) => '',
+			contentScriptJs: getContentScriptJs,
+			postMessageHandler,
+		};
+
+		await editor.setContentScripts([
+			testPlugin1, testPlugin2,
+		]);
+
+		// Allows plugins to load
+		await jest.runAllTimersAsync();
+
+		// Should be one script container for each plugin
+		expect(document.querySelectorAll('#joplin-plugin-scripts-container script')).toHaveLength(2);
 	});
 });

packages/editor/CodeMirror/pluginApi/PluginLoader.ts

@@ -1,4 +1,4 @@
-import { LogMessageCallback, PluginData } from '../../types';
+import { LogMessageCallback, ContentScriptData } from '../../types';
 import CodeMirrorControl from '../CodeMirrorControl';
 import codeMirrorRequire from './codeMirrorRequire';
 
@@ -8,9 +8,11 @@ let pluginLoaderCounter = 0;
 type OnScriptLoadCallback = (exports: any)=> void;
 type OnPluginRemovedCallback = ()=> void;
 
+const contentScriptToId = (contentScript: ContentScriptData) => `${contentScript.pluginId}--${contentScript.contentScriptId}`;
+
 export default class PluginLoader {
 	private pluginScriptsContainer: HTMLElement;
-	private loadedPluginIds: string[] = [];
+	private loadedContentScriptIds: string[] = [];
 	private pluginRemovalCallbacks: Record<string, OnPluginRemovedCallback> = {};
 	private pluginLoaderId: number;
 
@@ -32,17 +34,18 @@ export default class PluginLoader {
 		(window as any).__pluginLoaderRequireFunctions[this.pluginLoaderId] = codeMirrorRequire;
 	}
 
-	public async setPlugins(plugins: PluginData[]) {
-		for (const plugin of plugins) {
-			if (!this.loadedPluginIds.includes(plugin.pluginId)) {
-				this.addPlugin(plugin);
+	public async setPlugins(contentScripts: ContentScriptData[]) {
+		for (const contentScript of contentScripts) {
+			const id = contentScriptToId(contentScript);
+			if (!this.loadedContentScriptIds.includes(id)) {
+				this.addPlugin(contentScript);
 			}
 		}
 
 		// Remove old plugins
-		const pluginIds = plugins.map(plugin => plugin.pluginId);
-		const removedIds = this.loadedPluginIds
-			.filter(id => !pluginIds.includes(id));
+		const contentScriptIds = contentScripts.map(contentScriptToId);
+		const removedIds = this.loadedContentScriptIds
+			.filter(id => !contentScriptIds.includes(id));
 
 		for (const id of removedIds) {
 			if (id in this.pluginRemovalCallbacks) {
@@ -51,10 +54,10 @@ export default class PluginLoader {
 		}
 	}
 
-	private addPlugin(plugin: PluginData) {
+	private addPlugin(plugin: ContentScriptData) {
 		const onRemoveCallbacks: OnPluginRemovedCallback[] = [];
 
-		this.logMessage(`Loading plugin ${plugin.pluginId}`);
+		this.logMessage(`Loading plugin ${plugin.pluginId}, content script ${plugin.contentScriptId}`);
 
 		const addScript = (onLoad: OnScriptLoadCallback) => {
 			const scriptElement = document.createElement('script');
@@ -68,7 +71,7 @@ export default class PluginLoader {
 				const js = await plugin.contentScriptJs();
 
 				// Stop if cancelled
-				if (!this.loadedPluginIds.includes(plugin.pluginId)) {
+				if (!this.loadedContentScriptIds.includes(contentScriptToId(plugin))) {
 					return;
 				}
 
@@ -109,13 +112,13 @@ export default class PluginLoader {
 			this.pluginScriptsContainer.appendChild(styleContainer);
 		};
 
-		this.pluginRemovalCallbacks[plugin.pluginId] = () => {
+		this.pluginRemovalCallbacks[contentScriptToId(plugin)] = () => {
 			for (const callback of onRemoveCallbacks) {
 				callback();
 			}
 
-			this.loadedPluginIds = this.loadedPluginIds.filter(id => {
-				return id !== plugin.pluginId;
+			this.loadedContentScriptIds = this.loadedContentScriptIds.filter(id => {
+				return id !== contentScriptToId(plugin);
 			});
 		};
 
@@ -173,7 +176,7 @@ export default class PluginLoader {
 			}
 		});
 
-		this.loadedPluginIds.push(plugin.pluginId);
+		this.loadedContentScriptIds.push(contentScriptToId(plugin));
 	}
 
 	public remove() {

packages/editor/types.ts

@@ -64,7 +64,7 @@ export enum EditorCommandType {
 
 // Because the editor package can run in a WebView, plugin content scripts
 // need to be provided as text, rather than as file paths.
-export interface PluginData {
+export interface ContentScriptData {
 	pluginId: string;
 	contentScriptId: string;
 	contentScriptJs: ()=> Promise<string>;
@@ -95,7 +95,7 @@ export interface EditorControl {
 
 	setSearchState(state: SearchState): void;
 
-	setPlugins(plugins: PluginData[]): Promise<void>;
+	setContentScripts(plugins: ContentScriptData[]): Promise<void>;
 }
 
 export enum EditorLanguageType {