Mobile: Rich Text Editor: Fix additional blank lines added around list items on save (#12935)

.eslintignore

@@ -1117,6 +1117,8 @@ packages/editor/ProseMirror/utils/extractSelectedLinesTo.js
 packages/editor/ProseMirror/utils/forEachHeading.js
 packages/editor/ProseMirror/utils/jumpToHash.js
 packages/editor/ProseMirror/utils/makeLinksClickableInElement.js
+packages/editor/ProseMirror/utils/postprocessEditorOutput.test.js
+packages/editor/ProseMirror/utils/postprocessEditorOutput.js
 packages/editor/ProseMirror/utils/preprocessEditorInput.test.js
 packages/editor/ProseMirror/utils/preprocessEditorInput.js
 packages/editor/ProseMirror/utils/sanitizeHtml.js

.gitignore

@@ -1090,6 +1090,8 @@ packages/editor/ProseMirror/utils/extractSelectedLinesTo.js
 packages/editor/ProseMirror/utils/forEachHeading.js
 packages/editor/ProseMirror/utils/jumpToHash.js
 packages/editor/ProseMirror/utils/makeLinksClickableInElement.js
+packages/editor/ProseMirror/utils/postprocessEditorOutput.test.js
+packages/editor/ProseMirror/utils/postprocessEditorOutput.js
 packages/editor/ProseMirror/utils/preprocessEditorInput.test.js
 packages/editor/ProseMirror/utils/preprocessEditorInput.js
 packages/editor/ProseMirror/utils/sanitizeHtml.js

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

@@ -385,6 +385,22 @@ describe('RichTextEditor', () => {
 		expect(editor.textContent).toContain('3^2 + 4^2 = 5^2');
 	});
 
+	it('should save lists as single-spaced', async () => {
+		let body = 'Test:\n\n- this\n- is\n- a\n- test.';
+
+		render(<WrappedEditor
+			noteBody={body}
+			onBodyChange={newBody => { body = newBody; }}
+		/>);
+
+		const window = await getEditorWindow();
+		mockTyping(window, ' Testing');
+
+		await waitFor(async () => {
+			expect(body.trim()).toBe('Test:\n\n- this\n- is\n- a\n- test. Testing');
+		});
+	});
+
 	it('should preserve table of contents blocks on edit', async () => {
 		let body = '# Heading\n\n# Heading 2\n\n[toc]\n\nTest.';
 

packages/app-mobile/contentScripts/richTextEditorBundle/contentScript/index.ts

@@ -10,17 +10,6 @@ import convertHtmlToMarkdown from './convertHtmlToMarkdown';
 import { ExportedWebViewGlobals as MarkdownEditorWebViewGlobals } from '../../markdownEditorBundle/types';
 import { EditorEventType } from '@joplin/editor/events';
 
-const postprocessHtml = (html: HTMLElement) => {
-	// Fix resource URLs
-	const resources = html.querySelectorAll<HTMLImageElement>('img[data-resource-id]');
-	for (const resource of resources) {
-		const resourceId = resource.getAttribute('data-resource-id');
-		resource.src = `:/${resourceId}`;
-	}
-
-	return html;
-};
-
 const wrapHtmlForMarkdownConversion = (html: HTMLElement) => {
 	// Add a container element -- when converting to HTML, Turndown
 	// sometimes doesn't process the toplevel element in the same way
@@ -32,8 +21,6 @@ const wrapHtmlForMarkdownConversion = (html: HTMLElement) => {
 
 
 const htmlToMarkdown = (html: HTMLElement): string => {
-	html = postprocessHtml(html);
-
 	return convertHtmlToMarkdown(html);
 };
 
@@ -91,27 +78,11 @@ export const initialize = async (
 				removeUnusedPluginAssets: options.isFullPageRender,
 			});
 		},
-		renderHtmlToMarkup: (node) => {
+		renderHtmlToMarkup: (html) => {
-			// By default, if `src` is specified on an image, the browser will try to load the image, even if it isn't added
-			// to the DOM. (A similar problem is described here: https://stackoverflow.com/q/62019538).
-			// Since :/resourceId isn't a valid image URI, this results in a large number of warnings. As a workaround,
-			// move the element to a temporary document before processing:
-			const dom = document.implementation.createHTMLDocument();
-			node = dom.importNode(node, true);
-
-			let html: HTMLElement;
-			if ((node instanceof HTMLElement)) {
-				html = node;
-			} else {
-				const container = document.createElement('div');
-				container.appendChild(html);
-				html = container;
-			}
-
 			if (settings.language === EditorLanguageType.Markdown) {
 				return htmlToMarkdown(wrapHtmlForMarkdownConversion(html));
 			} else {
-				return postprocessHtml(html).outerHTML;
+				return html.outerHTML;
 			}
 		},
 	}, (parent, language, onChange) => {

packages/editor/ProseMirror/createEditor.ts

@@ -26,6 +26,7 @@ import { OnCreateCodeEditor as OnCreateCodeEditor, RendererControl } from './typ
 import resourcePlaceholderPlugin, { onResourceDownloaded } from './plugins/resourcePlaceholderPlugin';
 import getFileFromPasteEvent from '../utils/getFileFromPasteEvent';
 import { RenderResult } from '../../renderer/types';
+import postprocessEditorOutput from './utils/postprocessEditorOutput';
 import detailsPlugin from './plugins/detailsPlugin';
 
 interface ProseMirrorControl extends EditorControl {
@@ -40,7 +41,9 @@ const createEditor = async (
 	createCodeEditor: OnCreateCodeEditor,
 ): Promise<ProseMirrorControl> => {
 	const renderNodeToMarkup = (node: Node|DocumentFragment) => {
-		return renderer.renderHtmlToMarkup(node);
+		return renderer.renderHtmlToMarkup(
+			postprocessEditorOutput(node),
+		);
 	};
 
 	const proseMirrorParser = ProseMirrorDomParser.fromSchema(schema);

packages/editor/ProseMirror/types.ts

@@ -7,7 +7,7 @@ interface MarkupToHtmlOptions {
 }
 
 export type MarkupToHtml = (markup: string, options: MarkupToHtmlOptions)=> Promise<RenderResult>;
-export type HtmlToMarkup = (html: Node|DocumentFragment)=> string;
+export type HtmlToMarkup = (html: HTMLElement)=> string;
 
 export interface RendererControl {
 	renderMarkupToHtml: MarkupToHtml;

packages/editor/ProseMirror/utils/postprocessEditorOutput.test.ts

@@ -0,0 +1,34 @@
+import postprocessEditorOutput from './postprocessEditorOutput';
+
+const normalizeHtmlString = (html: string) => {
+	return html.replace(/\s+/g, ' ').trim();
+};
+
+describe('postprocessEditorOutput', () => {
+	// Removing extra space around list items prevents extra space from being
+	// added when converting from HTML to Markdown
+	test('should remove extra paragraphs from around list items', () => {
+		const doc = new DOMParser().parseFromString(`
+			<body>
+			<ul>
+				<li><p>Test</p></li>
+				<li>Test 2</li>
+				<li><p></p><p>Test 3</p><p></p></li>
+			</ul>
+		`, 'text/html');
+
+		const output = postprocessEditorOutput(doc.body);
+
+		expect(
+			normalizeHtmlString(output.querySelector('ul').outerHTML),
+		).toBe(
+			normalizeHtmlString(`
+				<ul>
+					<li>Test</li>
+					<li>Test 2</li>
+					<li>Test 3</li>
+				</ul>
+			`),
+		);
+	});
+});

packages/editor/ProseMirror/utils/postprocessEditorOutput.ts

@@ -0,0 +1,48 @@
+import trimEmptyParagraphs from './trimEmptyParagraphs';
+
+const fixResourceUrls = (container: HTMLElement) => {
+	const resources = container.querySelectorAll<HTMLImageElement>('img[data-resource-id]');
+	for (const resource of resources) {
+		const resourceId = resource.getAttribute('data-resource-id');
+		resource.src = `:/${resourceId}`;
+	}
+};
+
+const removeListItemWrapperParagraphs = (container: HTMLElement) => {
+	const listItems = container.querySelectorAll<HTMLLIElement>('li');
+	for (const item of listItems) {
+		trimEmptyParagraphs(item);
+
+		if (item.children.length === 1) {
+			const firstChild = item.children[0];
+			if (firstChild.tagName === 'P') {
+				firstChild.replaceWith(...firstChild.childNodes);
+			}
+		}
+	}
+};
+
+const postprocessEditorOutput = (node: Node|DocumentFragment) => {
+	// By default, if `src` is specified on an image, the browser will try to load the image, even if it isn't added
+	// to the DOM. (A similar problem is described here: https://stackoverflow.com/q/62019538).
+	// Since :/resourceId isn't a valid image URI, this results in a large number of warnings. As a workaround,
+	// move the element to a temporary document before processing:
+	const dom = document.implementation.createHTMLDocument();
+	node = dom.importNode(node, true);
+
+	let html: HTMLElement;
+	if ((node instanceof HTMLElement)) {
+		html = node;
+	} else {
+		const container = document.createElement('div');
+		container.appendChild(node);
+		html = container;
+	}
+
+	fixResourceUrls(html);
+	removeListItemWrapperParagraphs(html);
+
+	return html;
+};
+
+export default postprocessEditorOutput;