Desktop: Resolves #16436: Line return after a bulleted list is stripped when switching away from a note in the Rich Text Editor (#16717)

Co-authored-by: Laurent Cozic <laurent22@users.noreply.github.com>

packages/app-cli/tests/HtmlToMd.ts

@@ -3,6 +3,7 @@ import * as os from 'os';
 import { readFile } from 'fs/promises';
 import { filename } from '@joplin/lib/path-utils';
 import HtmlToMd, { ParseOptions } from '@joplin/lib/HtmlToMd';
+import MdToHtml from '@joplin/renderer/MdToHtml';
 
 describe('HtmlToMd', () => {
 
@@ -129,4 +130,19 @@ describe('HtmlToMd', () => {
 		expect(collapsedResult).toContain('\n\n');
 	});
 
+	it.each([
+		['inline code ending with a space at the end of a heading', '<h2><code>npm</code> install&nbsp;<code>a&nbsp;</code></h2>', 'a\u00A0'],
+		['a heading that only contains inline code with a space', '<h2><code>&nbsp;</code></h2>', '\u00A0'],
+	])('should not change %s when the note is saved again', async (_title, html, codeText) => {
+		const htmlToMd = new HtmlToMd();
+		const mdToHtml = new MdToHtml();
+
+		const firstSave = htmlToMd.parse(html);
+		const reopened = await mdToHtml.render(firstSave, null, { bodyOnly: true });
+		const secondSave = htmlToMd.parse(reopened.html);
+
+		expect(reopened.html).toContain(`>${codeText}</code>`);
+		expect(secondSave).toBe(firstSave);
+	});
+
 });

packages/app-cli/tests/html_to_md/heading_with_nonbreaking_space.html

@@ -0,0 +1,5 @@
+<p>Headings with nonbreaking spaces should be preserved:</p>
+<h2>&nbsp;</h2>
+<h2>&nbsp; &nbsp;</h2>
+<h3>&nbsp;Leading space</h3>
+<h2><code>a&nbsp; b</code></h2>

packages/app-cli/tests/html_to_md/heading_with_nonbreaking_space.md

@@ -0,0 +1,9 @@
+Headings with nonbreaking spaces should be preserved:
+
+##  
+
+##    
+
+###  Leading space
+
+## `a  b`

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

@@ -733,10 +733,10 @@ const TinyMCE = (props: NoteBodyEditorProps, ref: Ref<NoteBodyEditorRef>) => {
 				noneditable_class: 'joplin-editable', // Can be a regex too
 				iframe_aria_text: _('Rich Text editor. Press Escape then Tab to escape focus.'),
 
-				// #p: Pad empty paragraphs with &nbsp; to prevent them from being removed.
+				// #p, #h1-#h6: Pad empty paragraphs and headings with &nbsp; to prevent them from being removed.
 				// *[*]: Allow all elements and attributes -- we already filter in sanitize_html
 				// See https://www.tiny.cloud/docs/configure/content-filtering/#controlcharacters
-				valid_elements: '#p,*[*]',
+				valid_elements: '#p,#h1[*],#h2[*],#h3[*],#h4[*],#h5[*],#h6[*],*[*]',
 
 				menubar: false,
 				relative_urls: false,

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

@@ -329,6 +329,49 @@ test.describe('richTextEditor', () => {
 		await expect(editorBody).toHaveText('Unsaved text');
 	});
 
+	test('should not drop empty lines made by pressing Enter at the start of a heading', async ({ mainWindow }) => {
+		const mainScreen = await new MainScreen(mainWindow).setup();
+		await mainScreen.createNewNote('Other note');
+		await mainScreen.createNewNote('Heading note');
+		const otherNoteHeader = mainScreen.noteList.getNoteItemByTitle('Other note');
+		const headingNoteHeader = mainScreen.noteList.getNoteItemByTitle('Heading note');
+		const editor = mainScreen.noteEditor;
+
+		const markdownEditor = await editor.showMarkdownEditor();
+		await markdownEditor.typeText('Text above');
+		await markdownEditor.pressKey('Enter');
+		await markdownEditor.pressKey('Enter');
+		await markdownEditor.typeText('## Heading');
+
+		const richTextEditor = await editor.showRichTextEditor();
+		const headings = richTextEditor.content.locator('h2');
+		await expect(headings).toHaveCount(1);
+
+		await headings.getByText('Heading').click();
+		await mainWindow.keyboard.press('Home');
+		await mainWindow.keyboard.press('Enter');
+		await mainWindow.keyboard.press('Enter');
+		await expect(headings).toHaveCount(3);
+
+		const switchToOtherNoteAndBack = async (expectedText: string) => {
+			await otherNoteHeader.click();
+			await expect(editor.noteTitleInput).toHaveValue('Other note');
+			await headingNoteHeader.click();
+			await expect(editor.noteTitleInput).toHaveValue('Heading note');
+			await expect(richTextEditor.body).toContainText(expectedText);
+		};
+
+		await switchToOtherNoteAndBack('Text above');
+		await expect(headings).toHaveCount(3);
+
+		// Saving again after the note was reloaded from Markdown, when the empty headings can get an id
+		await richTextEditor.content.getByText('Text above').click();
+		await mainWindow.keyboard.press('End');
+		await mainWindow.keyboard.type('!');
+		await switchToOtherNoteAndBack('Text above!');
+		await expect(headings).toHaveCount(3);
+	});
+
 	test('should highlight search matches', async ({ mainWindow }) => {
 		const mainScreen = await new MainScreen(mainWindow).setup();
 		await mainScreen.createNewNote('Testing');

packages/renderer/MdToHtml.ts

@@ -9,6 +9,7 @@ import { FsDriver, ItemIdToUrlHandler, MarkupRenderer, OptionsResourceModel, Ren
 import hljs from './highlight';
 // Use a require() to support bundling on mobile:
 import MarkdownIt = require('markdown-it');
+import type Token = require('markdown-it/lib/token');
 
 import { AllHtmlEntities as Entities } from 'html-entities';
 const htmlentities = new Entities().encode;
@@ -79,6 +80,11 @@ function slugify(s: string): string {
 	return uslug(s);
 }
 
+const removeEmptyHeadingId = (token: Token, { title }: { title: string }) => {
+	if (title.trim()) return;
+	token.attrs = token.attrs.filter(([name]) => name !== 'id');
+};
+
 // Share across all instances of MdToHtml
 const inMemoryCache = new InMemoryCache(20);
 
@@ -619,7 +625,7 @@ export default class MdToHtml implements MarkupRenderer {
 			});
 		}
 
-		loadPlugin(markdownItAnchor, { slugify: slugify });
+		loadPlugin(markdownItAnchor, { slugify: slugify, callback: removeEmptyHeadingId });
 
 		for (const key in plugins) {
 			if (this.pluginEnabled(key)) {

packages/turndown/src/commonmark-rules.js

@@ -77,9 +77,15 @@ rules.heading = {
 
   replacement: function (content, node, options) {
     var hLevel = Number(node.nodeName.charAt(1))
+    var originalContentLength = content.length
+    // The markdown renderer trims raw nonbreaking spaces at the start and end of a heading,
+    // so escape only those. Inside a code span, ` ` would be shown as literal text.
+    content = content
+      .replace(/^\u{00A0}+/u, s => ' '.repeat(s.length))
+      .replace(/\u{00A0}+$/u, s => ' '.repeat(s.length))
 
     if (options.headingStyle === 'setext' && hLevel < 3) {
-      var underline = repeat((hLevel === 1 ? '=' : '-'), content.length)
+      var underline = repeat((hLevel === 1 ? '=' : '-'), originalContentLength)
       return (
         '\n\n' + content + '\n' + underline + '\n\n'
       )

packages/turndown/src/node.js

@@ -14,10 +14,15 @@ function isBlank (node) {
     !isMeaningfulWhenBlank(node) &&
     /^\s*$/i.test(node.textContent) &&
     !hasVoid(node) &&
-    !hasMeaningfulWhenBlank(node)
+    !hasMeaningfulWhenBlank(node) &&
+    !isNonBreakingSpaceHeading(node)
   )
 }
 
+function isNonBreakingSpaceHeading (node) {
+  return /^H[1-6]$/.test(node.nodeName) && node.textContent.includes('\u00A0')
+}
+
 function flankingWhitespace (node, options) {
   if (node.isBlock || (options.preformattedCode && node.isCode)) {
     return { leading: '', trailing: '' }
@@ -25,6 +30,14 @@ function flankingWhitespace (node, options) {
 
   var edges = edgeWhitespace(node.textContent)
 
+  // Nonbreaking spaces at the edges of inline code are part of the code, so keep them inside it
+  if (node.nodeName === 'CODE') {
+    edges.leading = edges.leadingAscii
+    edges.trailing = edges.trailingAscii
+    edges.leadingNonAscii = ''
+    edges.trailingNonAscii = ''
+  }
+
   // abandon leading ASCII WS if left-flanked by ASCII WS
   if (edges.leadingAscii && isFlankedByWhitespace('left', node, options)) {
     edges.leading = edges.leadingNonAscii