Desktop: Fixes #9512: Pasting rich text in the RTE sometimes result in invalid markup

.eslintignore

@@ -294,6 +294,7 @@ packages/app-desktop/gui/NoteEditor/utils/clipboardUtils.js
 packages/app-desktop/gui/NoteEditor/utils/contextMenu.js
 packages/app-desktop/gui/NoteEditor/utils/contextMenuUtils.js
 packages/app-desktop/gui/NoteEditor/utils/index.js
+packages/app-desktop/gui/NoteEditor/utils/markupRenderOptions.js
 packages/app-desktop/gui/NoteEditor/utils/resourceHandling.test.js
 packages/app-desktop/gui/NoteEditor/utils/resourceHandling.js
 packages/app-desktop/gui/NoteEditor/utils/types.js

.gitignore

@@ -274,6 +274,7 @@ packages/app-desktop/gui/NoteEditor/utils/clipboardUtils.js
 packages/app-desktop/gui/NoteEditor/utils/contextMenu.js
 packages/app-desktop/gui/NoteEditor/utils/contextMenuUtils.js
 packages/app-desktop/gui/NoteEditor/utils/index.js
+packages/app-desktop/gui/NoteEditor/utils/markupRenderOptions.js
 packages/app-desktop/gui/NoteEditor/utils/resourceHandling.test.js
 packages/app-desktop/gui/NoteEditor/utils/resourceHandling.js
 packages/app-desktop/gui/NoteEditor/utils/types.js

packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/v5/CodeMirror.tsx

@@ -2,7 +2,7 @@ import * as React from 'react';
 import { useState, useEffect, useRef, forwardRef, useCallback, useImperativeHandle, useMemo, ForwardedRef } from 'react';
 
 // eslint-disable-next-line no-unused-vars
-import { EditorCommand, NoteBodyEditorProps, NoteBodyEditorRef } from '../../../utils/types';
+import { EditorCommand, MarkupToHtmlOptions, NoteBodyEditorProps, NoteBodyEditorRef } from '../../../utils/types';
 import { commandAttachFileToBody, getResourcesFromPasteEvent } from '../../../utils/resourceHandling';
 import { ScrollOptions, ScrollOptionTypes } from '../../../utils/types';
 import { CommandValue } from '../../../utils/types';
@@ -29,7 +29,6 @@ const debounce = require('debounce');
 
 import { reg } from '@joplin/lib/registry';
 import ErrorBoundary from '../../../../ErrorBoundary';
-import { MarkupToHtmlOptions } from '../../../utils/useMarkupToHtml';
 import useStyles from '../utils/useStyles';
 import useContextMenu from '../utils/useContextMenu';
 import useWebviewIpcMessage from '../utils/useWebviewIpcMessage';

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

@@ -1,7 +1,7 @@
 import * as React from 'react';
 import { useState, useEffect, useRef, forwardRef, useCallback, useImperativeHandle, useMemo, ForwardedRef } from 'react';
 
-import { EditorCommand, NoteBodyEditorProps, NoteBodyEditorRef, OnChangeEvent } from '../../../utils/types';
+import { EditorCommand, MarkupToHtmlOptions, NoteBodyEditorProps, NoteBodyEditorRef, OnChangeEvent } from '../../../utils/types';
 import { getResourcesFromPasteEvent } from '../../../utils/resourceHandling';
 import { ScrollOptions, ScrollOptionTypes } from '../../../utils/types';
 import NoteTextViewer from '../../../../NoteTextViewer';
@@ -16,7 +16,6 @@ import { MarkupToHtml } from '@joplin/renderer';
 const { clipboard } = require('electron');
 import { reg } from '@joplin/lib/registry';
 import ErrorBoundary from '../../../../ErrorBoundary';
-import { MarkupToHtmlOptions } from '../../../utils/useMarkupToHtml';
 import { EditorKeymap, EditorLanguageType, EditorSettings } from '@joplin/editor/types';
 import useStyles from '../utils/useStyles';
 import { EditorEvent, EditorEventType } from '@joplin/editor/events';

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

@@ -1,6 +1,6 @@
 import * as React from 'react';
 import { useState, useEffect, useCallback, useRef, forwardRef, useImperativeHandle } from 'react';
-import { ScrollOptions, ScrollOptionTypes, EditorCommand, NoteBodyEditorProps, ResourceInfos } from '../../utils/types';
+import { ScrollOptions, ScrollOptionTypes, EditorCommand, NoteBodyEditorProps, ResourceInfos, HtmlToMarkdownHandler } from '../../utils/types';
 import { resourcesStatus, commandAttachFileToBody, getResourcesFromPasteEvent, processPastedHtml, attachedResources } from '../../utils/resourceHandling';
 import useScroll from './utils/useScroll';
 import styles_ from './styles';
@@ -20,7 +20,6 @@ import BaseItem from '@joplin/lib/models/BaseItem';
 import setupToolbarButtons from './utils/setupToolbarButtons';
 import { plainTextToHtml } from '@joplin/lib/htmlUtils';
 import openEditDialog from './utils/openEditDialog';
-import { MarkupToHtmlOptions } from '../../utils/useMarkupToHtml';
 import { themeStyle } from '@joplin/lib/theme';
 import { loadScript } from '../../../utils/loadScript';
 import bridge from '../../../../services/bridge';
@@ -30,25 +29,11 @@ import { joplinCommandToTinyMceCommands, TinyMceCommand } from './utils/joplinCo
 import shouldPasteResources from './utils/shouldPasteResources';
 import lightTheme from '@joplin/lib/themes/light';
 import { Options as NoteStyleOptions } from '@joplin/renderer/noteStyle';
+import markupRenderOptions from '../../utils/markupRenderOptions';
 const md5 = require('md5');
 const { clipboard } = require('electron');
 const supportedLocales = require('./supportedLocales');
 
-function markupRenderOptions(override: MarkupToHtmlOptions = null): MarkupToHtmlOptions {
-	return {
-		plugins: {
-			checkbox: {
-				checkboxRenderingType: 2,
-			},
-			link_open: {
-				linkRenderingType: 2,
-			},
-		},
-		replaceResourceInternalToExternalLinks: true,
-		...override,
-	};
-}
-
 // In TinyMCE 5.2, when setting the body to '<div id="rendered-md"></div>',
 // it would end up as '<div id="rendered-md"><br/></div>' once rendered
 // (an additional <br/> was inserted).
@@ -123,7 +108,7 @@ const TinyMCE = (props: NoteBodyEditorProps, ref: any) => {
 	const { scrollToPercent } = useScroll({ editor, onScroll: props.onScroll });
 
 	usePluginServiceRegistration(ref);
-	useContextMenu(editor, props.plugins, props.dispatch);
+	useContextMenu(editor, props.plugins, props.dispatch, props.htmlToMarkdown, props.markupToHtml);
 
 	const dispatchDidUpdate = (editor: any) => {
 		if (dispatchDidUpdateIID_) shim.clearTimeout(dispatchDidUpdateIID_);
@@ -978,7 +963,7 @@ const TinyMCE = (props: NoteBodyEditorProps, ref: any) => {
 	props_onChangeRef.current = props.onChange;
 
 	// eslint-disable-next-line @typescript-eslint/ban-types -- Old code before rule was applied
-	const prop_htmlToMarkdownRef = useRef<Function>();
+	const prop_htmlToMarkdownRef = useRef<HtmlToMarkdownHandler>();
 	prop_htmlToMarkdownRef.current = props.htmlToMarkdown;
 
 	const nextOnChangeEventInfo = useRef<any>(null);
@@ -1136,7 +1121,11 @@ const TinyMCE = (props: NoteBodyEditorProps, ref: any) => {
 					editor.insertContent(result.html);
 				} else { // Paste regular text
 					if (pastedHtml) { // Handles HTML
-						const modifiedHtml = await processPastedHtml(pastedHtml);
+						const modifiedHtml = await processPastedHtml(
+							pastedHtml,
+							prop_htmlToMarkdownRef.current,
+							markupToHtml.current,
+						);
 						editor.insertContent(modifiedHtml);
 					} else { // Handles plain text
 						pasteAsPlainText(pastedText);

packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/utils/useContextMenu.ts

@@ -12,6 +12,7 @@ import Setting from '@joplin/lib/models/Setting';
 
 import Resource from '@joplin/lib/models/Resource';
 import { TinyMceEditorEvents } from './types';
+import { HtmlToMarkdownHandler, MarkupToHtmlHandler } from '../../../utils/types';
 
 const menuUtils = new MenuUtils(CommandService.instance());
 
@@ -42,11 +43,11 @@ interface ContextMenuActionOptions {
 const contextMenuActionOptions: ContextMenuActionOptions = { current: null };
 
 // eslint-disable-next-line @typescript-eslint/ban-types -- Old code before rule was applied
-export default function(editor: any, plugins: PluginStates, dispatch: Function) {
+export default function(editor: any, plugins: PluginStates, dispatch: Function, htmlToMd: HtmlToMarkdownHandler, mdToHtml: MarkupToHtmlHandler) {
 	useEffect(() => {
 		if (!editor) return () => {};
 
-		const contextMenuItems = menuItems(dispatch);
+		const contextMenuItems = menuItems(dispatch, htmlToMd, mdToHtml);
 
 		function onContextMenu(_event: any, params: any) {
 			const element = contextMenuElement(editor, params.x, params.y);
@@ -82,6 +83,8 @@ export default function(editor: any, plugins: PluginStates, dispatch: Function)
 				fireEditorEvent: (event: TinyMceEditorEvents) => {
 					editor.fire(event);
 				},
+				htmlToMd,
+				mdToHtml,
 			};
 
 			let template = [];
@@ -118,5 +121,5 @@ export default function(editor: any, plugins: PluginStates, dispatch: Function)
 				bridge().window().webContents.off('context-menu', onContextMenu);
 			}
 		};
-	}, [editor, plugins, dispatch]);
+	}, [editor, plugins, dispatch, htmlToMd, mdToHtml]);
 }

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

@@ -341,7 +341,7 @@ function NoteEditor(props: NoteEditorProps) {
 		// eslint-disable-next-line @seiyab/react-hooks/exhaustive-deps -- Old code before rule was applied
 	}, [formNote, handleProvisionalFlag]);
 
-	const onMessage = useMessageHandler(scrollWhenReady, setScrollWhenReady, editorRef, setLocalSearchResultCount, props.dispatch, formNote);
+	const onMessage = useMessageHandler(scrollWhenReady, setScrollWhenReady, editorRef, setLocalSearchResultCount, props.dispatch, formNote, htmlToMarkdown, markupToHtml);
 
 	const externalEditWatcher_noteChange = useCallback((event: any) => {
 		if (event.id === formNote.id) {

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

@@ -14,6 +14,7 @@ import { TinyMceEditorEvents } from '../NoteBody/TinyMCE/utils/types';
 import { itemIsReadOnlySync, ItemSlice } from '@joplin/lib/models/utils/readOnly';
 import Setting from '@joplin/lib/models/Setting';
 import ItemChange from '@joplin/lib/models/ItemChange';
+import { HtmlToMarkdownHandler, MarkupToHtmlHandler } from './types';
 const fs = require('fs-extra');
 const { writeFile } = require('fs-extra');
 const { clipboard } = require('electron');
@@ -77,7 +78,7 @@ export async function openItemById(itemId: string, dispatch: Function, hash = ''
 }
 
 // eslint-disable-next-line @typescript-eslint/ban-types -- Old code before rule was applied
-export function menuItems(dispatch: Function): ContextMenuItems {
+export function menuItems(dispatch: Function, htmlToMd: HtmlToMarkdownHandler, mdToHtml: MarkupToHtmlHandler): ContextMenuItems {
 	return {
 		open: {
 			label: _('Open...'),
@@ -179,7 +180,7 @@ export function menuItems(dispatch: Function): ContextMenuItems {
 				let content = pastedHtml ? pastedHtml : clipboard.readText();
 
 				if (pastedHtml) {
-					content = await processPastedHtml(pastedHtml);
+					content = await processPastedHtml(pastedHtml, htmlToMd, mdToHtml);
 				}
 
 				options.insertContent(content);
@@ -207,7 +208,7 @@ export function menuItems(dispatch: Function): ContextMenuItems {
 export default async function contextMenu(options: ContextMenuOptions, dispatch: Function) {
 	const menu = new Menu();
 
-	const items = menuItems(dispatch);
+	const items = menuItems(dispatch, options.htmlToMd, options.mdToHtml);
 
 	if (!('readyOnly' in options)) options.isReadOnly = true;
 	for (const itemKey in items) {

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

@@ -1,6 +1,9 @@
 import Resource from '@joplin/lib/models/Resource';
 import Logger from '@joplin/utils/Logger';
+import { HtmlToMarkdownHandler, MarkupToHtmlHandler } from './types';
+
 const logger = Logger.create('contextMenuUtils');
+
 export enum ContextMenuItemType {
 	None = '',
 	Image = 'image',
@@ -22,6 +25,8 @@ export interface ContextMenuOptions {
 	isReadOnly?: boolean;
 	// eslint-disable-next-line @typescript-eslint/ban-types -- Old code before rule was applied
 	fireEditorEvent: Function;
+	htmlToMd: HtmlToMarkdownHandler;
+	mdToHtml: MarkupToHtmlHandler;
 }
 
 export interface ContextMenuItem {

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

@@ -0,0 +1,16 @@
+import { MarkupToHtmlOptions } from './types';
+
+export default (override: MarkupToHtmlOptions = null): MarkupToHtmlOptions => {
+	return {
+		plugins: {
+			checkbox: {
+				checkboxRenderingType: 2,
+			},
+			link_open: {
+				linkRenderingType: 2,
+			},
+		},
+		replaceResourceInternalToExternalLinks: true,
+		...override,
+	};
+};

packages/app-desktop/gui/NoteEditor/utils/resourceHandling.test.ts

@@ -1,5 +1,8 @@
 import Setting from '@joplin/lib/models/Setting';
 import { processPastedHtml } from './resourceHandling';
+import markupLanguageUtils from '@joplin/lib/markupLanguageUtils';
+import HtmlToMd from '@joplin/lib/HtmlToMd';
+import { HtmlToMarkdownHandler, MarkupToHtmlHandler } from './types';
 
 describe('resourceHandling', () => {
 	it('should sanitize pasted HTML', async () => {
@@ -19,7 +22,32 @@ describe('resourceHandling', () => {
 		];
 
 		for (const [html, expected] of testCases) {
-			expect(await processPastedHtml(html)).toBe(expected);
+			expect(await processPastedHtml(html, null, null)).toBe(expected);
 		}
 	});
+
+	it('should clean up pasted HTML', async () => {
+		const markupToHtml: MarkupToHtmlHandler = async (markupLanguage, markup, options) => {
+			const conv = markupLanguageUtils.newMarkupToHtml({}, {
+				resourceBaseUrl: `file://${Setting.value('resourceDir')}/`,
+				customCss: '',
+			});
+			return conv.render(markupLanguage, markup, {}, options);
+		};
+
+		const htmlToMd: HtmlToMarkdownHandler = async (_markupLanguage, html, _originalCss) => {
+			const conv = new HtmlToMd();
+			return conv.parse(html);
+		};
+
+		const testCases = [
+			['<p style="background-color: red">Hello</p><p style="display: hidden;">World</p>', '<p>Hello</p>\n<p>World</p>\n'],
+			['', ''],
+		];
+
+		for (const [html, expected] of testCases) {
+			expect(await processPastedHtml(html, htmlToMd, markupToHtml)).toBe(expected);
+		}
+	});
+
 });

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

@@ -10,6 +10,8 @@ import rendererHtmlUtils, { extractHtmlBody } from '@joplin/renderer/htmlUtils';
 import Logger from '@joplin/utils/Logger';
 import { fileUriToPath } from '@joplin/utils/url';
 import { MarkupLanguage } from '@joplin/renderer';
+import { HtmlToMarkdownHandler, MarkupToHtmlHandler } from './types';
+import markupRenderOptions from './markupRenderOptions';
 const joplinRendererUtils = require('@joplin/renderer').utils;
 const { clipboard } = require('electron');
 const mimeUtils = require('@joplin/lib/mime-utils.js').mime;
@@ -135,7 +137,7 @@ export async function getResourcesFromPasteEvent(event: any) {
 	return output;
 }
 
-export async function processPastedHtml(html: string) {
+export async function processPastedHtml(html: string, htmlToMd: HtmlToMarkdownHandler | null, mdToHtml: MarkupToHtmlHandler | null) {
 	const allImageUrls: string[] = [];
 	const mappedResources: Record<string, string> = {};
 
@@ -179,6 +181,15 @@ export async function processPastedHtml(html: string) {
 		}
 	}
 
+	// TinyMCE can accept any type of HTML, including HTML that may not be preserved once saved as
+	// Markdown. For example the content may have a dark background which would be supported by
+	// TinyMCE, but lost once the note is saved. So here we convert the HTML to Markdown then back
+	// to HTML to ensure that the content we paste will be handled correctly by the app.
+	if (htmlToMd && mdToHtml) {
+		const md = await htmlToMd(MarkupLanguage.Markdown, html, '');
+		html = (await mdToHtml(MarkupLanguage.Markdown, md, markupRenderOptions({ bodyOnly: true }))).html;
+	}
+
 	return extractHtmlBody(rendererHtmlUtils.sanitizeHtml(
 		htmlUtils.replaceImageUrls(html, (src: string) => {
 			return mappedResources[src];

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

@@ -3,7 +3,6 @@ import { ToolbarButtonInfo } from '@joplin/lib/services/commands/ToolbarButtonUt
 import { PluginStates } from '@joplin/lib/services/plugins/reducer';
 import { MarkupLanguage } from '@joplin/renderer';
 import { RenderResult, RenderResultPluginAsset } from '@joplin/renderer/types';
-import { MarkupToHtmlOptions } from './useMarkupToHtml';
 import { Dispatch } from 'redux';
 import { ProcessResultsRow } from '@joplin/lib/services/search/SearchEngine';
 
@@ -61,6 +60,24 @@ export interface NoteBodyEditorRef {
 	execCommand(command: CommandValue): Promise<void>;
 }
 
+export interface MarkupToHtmlOptions {
+	replaceResourceInternalToExternalLinks?: boolean;
+	resourceInfos?: ResourceInfos;
+	contentMaxWidth?: number;
+	plugins?: Record<string, any>;
+	bodyOnly?: boolean;
+	mapsToLine?: boolean;
+	useCustomPdfViewer?: boolean;
+	noteId?: string;
+	vendorDir?: string;
+	platformName?: string;
+	allowedFilePrefixes?: string[];
+	whiteBackgroundNoteRendering?: boolean;
+}
+
+export type MarkupToHtmlHandler = (markupLanguage: MarkupLanguage, markup: string, options: MarkupToHtmlOptions)=> Promise<RenderResult>;
+export type HtmlToMarkdownHandler = (markupLanguage: number, html: string, originalCss: string)=> Promise<string>;
+
 export interface NoteBodyEditorProps {
 	style: any;
 	ref: any;
@@ -81,9 +98,8 @@ export interface NoteBodyEditorProps {
 	onWillChange(event: any): void;
 	onMessage(event: any): void;
 	onScroll(event: { percent: number }): void;
-	markupToHtml: (markupLanguage: MarkupLanguage, markup: string, options: MarkupToHtmlOptions)=> Promise<RenderResult>;
+	markupToHtml: MarkupToHtmlHandler;
-	// eslint-disable-next-line @typescript-eslint/ban-types -- Old code before rule was applied
+	htmlToMarkdown: HtmlToMarkdownHandler;
-	htmlToMarkdown: Function;
 	allAssets: (markupLanguage: MarkupLanguage, options: AllAssetsOptions)=> Promise<RenderResultPluginAsset[]>;
 	disabled: boolean;
 	// eslint-disable-next-line @typescript-eslint/ban-types -- Old code before rule was applied

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

@@ -1,12 +1,12 @@
 import { PluginStates } from '@joplin/lib/services/plugins/reducer';
 import { useCallback, useMemo } from 'react';
-import { ResourceInfos } from './types';
 import markupLanguageUtils from '../../../utils/markupLanguageUtils';
 import Setting from '@joplin/lib/models/Setting';
 import shim from '@joplin/lib/shim';
 
 const { themeStyle } = require('@joplin/lib/theme');
 import Note from '@joplin/lib/models/Note';
+import { MarkupToHtmlOptions } from './types';
 
 interface HookDependencies {
 	themeId: number;
@@ -16,21 +16,6 @@ interface HookDependencies {
 	whiteBackgroundNoteRendering: boolean;
 }
 
-export interface MarkupToHtmlOptions {
-	replaceResourceInternalToExternalLinks?: boolean;
-	resourceInfos?: ResourceInfos;
-	contentMaxWidth?: number;
-	plugins?: Record<string, any>;
-	bodyOnly?: boolean;
-	mapsToLine?: boolean;
-	useCustomPdfViewer?: boolean;
-	noteId?: string;
-	vendorDir?: string;
-	platformName?: string;
-	allowedFilePrefixes?: string[];
-	whiteBackgroundNoteRendering?: boolean;
-}
-
 export default function useMarkupToHtml(deps: HookDependencies) {
 	const { themeId, customCss, plugins, whiteBackgroundNoteRendering } = deps;
 

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

@@ -1,5 +1,5 @@
 import { useCallback } from 'react';
-import { FormNote } from './types';
+import { FormNote, HtmlToMarkdownHandler, MarkupToHtmlHandler } from './types';
 import contextMenu from './contextMenu';
 import CommandService from '@joplin/lib/services/CommandService';
 import PostMessageService from '@joplin/lib/services/PostMessageService';
@@ -8,7 +8,7 @@ import { reg } from '@joplin/lib/registry';
 const bridge = require('@electron/remote').require('./bridge').default;
 
 // eslint-disable-next-line @typescript-eslint/ban-types -- Old code before rule was applied
-export default function useMessageHandler(scrollWhenReady: any, setScrollWhenReady: Function, editorRef: any, setLocalSearchResultCount: Function, dispatch: Function, formNote: FormNote) {
+export default function useMessageHandler(scrollWhenReady: any, setScrollWhenReady: Function, editorRef: any, setLocalSearchResultCount: Function, dispatch: Function, formNote: FormNote, htmlToMd: HtmlToMarkdownHandler, mdToHtml: MarkupToHtmlHandler) {
 	return useCallback(async (event: any) => {
 		const msg = event.channel ? event.channel : '';
 		const args = event.args;
@@ -44,6 +44,8 @@ export default function useMessageHandler(scrollWhenReady: any, setScrollWhenRea
 				htmlToCopy: '',
 				insertContent: () => { console.warn('insertContent() not implemented'); },
 				fireEditorEvent: () => { console.warn('fireEditorEvent() not implemented'); },
+				htmlToMd,
+				mdToHtml,
 			}, dispatch);
 
 			menu.popup({ window: bridge().window() });

packages/app-desktop/gui/PdfViewer.tsx

@@ -61,6 +61,8 @@ export default function PdfViewer(props: Props) {
 			htmlToCopy: '',
 			insertContent: () => { console.warn('insertContent() not implemented'); },
 			fireEditorEvent: () => { console.warn('fireEditorEvent() not implemented'); },
+			htmlToMd: async (_a, b, _c) => b,
+			mdToHtml: async (_a, b, _c) => { return { html: b, pluginAssets: [], cssStrings: [] }; },
 		} as ContextMenuOptions, props.dispatch);
 
 		menu.popup({ window: bridge().window() });