From ac05b7d389b599d87f34bc82a396fe91ceb93010 Mon Sep 17 00:00:00 2001 From: Henry Heino <46334387+personalizedrefrigerator@users.noreply.github.com> Date: Tue, 26 Aug 2025 00:46:00 -0700 Subject: [PATCH] Mobile: Rich Text Editor: Fix additional blank lines added around list items on save (#12935) --- .eslintignore | 2 + .gitignore | 2 + .../NoteEditor/RichTextEditor.test.tsx | 16 +++++++ .../contentScript/index.ts | 33 +------------ packages/editor/ProseMirror/createEditor.ts | 5 +- packages/editor/ProseMirror/types.ts | 2 +- .../utils/postprocessEditorOutput.test.ts | 34 +++++++++++++ .../utils/postprocessEditorOutput.ts | 48 +++++++++++++++++++ 8 files changed, 109 insertions(+), 33 deletions(-) create mode 100644 packages/editor/ProseMirror/utils/postprocessEditorOutput.test.ts create mode 100644 packages/editor/ProseMirror/utils/postprocessEditorOutput.ts diff --git a/.eslintignore b/.eslintignore index 9c222df32f..a99ba105ef 100644 --- a/.eslintignore +++ b/.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 diff --git a/.gitignore b/.gitignore index 5fda8fab81..8e8fe7c7ed 100644 --- a/.gitignore +++ b/.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 diff --git a/packages/app-mobile/components/NoteEditor/RichTextEditor.test.tsx b/packages/app-mobile/components/NoteEditor/RichTextEditor.test.tsx index 1c05ddf60d..4319ccf780 100644 --- a/packages/app-mobile/components/NoteEditor/RichTextEditor.test.tsx +++ b/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( { 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.'; diff --git a/packages/app-mobile/contentScripts/richTextEditorBundle/contentScript/index.ts b/packages/app-mobile/contentScripts/richTextEditorBundle/contentScript/index.ts index 6505febe6e..65bbee572f 100644 --- a/packages/app-mobile/contentScripts/richTextEditorBundle/contentScript/index.ts +++ b/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('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) => { - // 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; - } - + renderHtmlToMarkup: (html) => { if (settings.language === EditorLanguageType.Markdown) { return htmlToMarkdown(wrapHtmlForMarkdownConversion(html)); } else { - return postprocessHtml(html).outerHTML; + return html.outerHTML; } }, }, (parent, language, onChange) => { diff --git a/packages/editor/ProseMirror/createEditor.ts b/packages/editor/ProseMirror/createEditor.ts index 01badb6c04..a3d0551346 100644 --- a/packages/editor/ProseMirror/createEditor.ts +++ b/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 => { const renderNodeToMarkup = (node: Node|DocumentFragment) => { - return renderer.renderHtmlToMarkup(node); + return renderer.renderHtmlToMarkup( + postprocessEditorOutput(node), + ); }; const proseMirrorParser = ProseMirrorDomParser.fromSchema(schema); diff --git a/packages/editor/ProseMirror/types.ts b/packages/editor/ProseMirror/types.ts index 32084475d7..856bfd999a 100644 --- a/packages/editor/ProseMirror/types.ts +++ b/packages/editor/ProseMirror/types.ts @@ -7,7 +7,7 @@ interface MarkupToHtmlOptions { } export type MarkupToHtml = (markup: string, options: MarkupToHtmlOptions)=> Promise; -export type HtmlToMarkup = (html: Node|DocumentFragment)=> string; +export type HtmlToMarkup = (html: HTMLElement)=> string; export interface RendererControl { renderMarkupToHtml: MarkupToHtml; diff --git a/packages/editor/ProseMirror/utils/postprocessEditorOutput.test.ts b/packages/editor/ProseMirror/utils/postprocessEditorOutput.test.ts new file mode 100644 index 0000000000..f3a26135cc --- /dev/null +++ b/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(` + +
    +

  • Test

    • +
  • Test 2
    • +

  • Test 3

    • +
+ `, 'text/html'); + + const output = postprocessEditorOutput(doc.body); + + expect( + normalizeHtmlString(output.querySelector('ul').outerHTML), + ).toBe( + normalizeHtmlString(` +
    +
  • Test
    • +
  • Test 2
    • +
  • Test 3
    • +
+ `), + ); + }); +}); diff --git a/packages/editor/ProseMirror/utils/postprocessEditorOutput.ts b/packages/editor/ProseMirror/utils/postprocessEditorOutput.ts new file mode 100644 index 0000000000..51d328104c --- /dev/null +++ b/packages/editor/ProseMirror/utils/postprocessEditorOutput.ts @@ -0,0 +1,48 @@ +import trimEmptyParagraphs from './trimEmptyParagraphs'; + +const fixResourceUrls = (container: HTMLElement) => { + const resources = container.querySelectorAll('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('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;