fix(composer): Fix paragraph breaks getting lost on paste and sending

AI-assisted: GitHub Copilot (Claude Sonnet 4.6)
Signed-off-by: Jan C. Borchardt <925062+jancborchardt@users.noreply.github.com>

Author: jancborchardt

lib/Service/MimeMessage.php

@@ -11,7 +11,6 @@
 
 use DOMDocument;
 use DOMElement;
-use DOMNode;
 use Horde_Mime_Part;
 use Horde_Text_Filter;
 use OCA\Mail\Exception\InvalidDataUriException;
@@ -100,7 +99,7 @@ private function buildMessagePart(?string $contentPlain, ?string $contentHtml, a
 			$plainPart->setType('text/plain');
 			$plainPart->setCharset('UTF-8');
 			$plainPart->setContents(
-				Horde_Text_Filter::filter($contentHtml, 'Html2text', ['callback' => [$this, 'htmlToTextCallback']])
+				Horde_Text_Filter::filter($contentHtml, 'Html2text', [])
 			);
 		}
 
@@ -258,23 +257,4 @@ private function separateAttachments(array $attachments): array {
 
 		return [$inline, $normal];
 	}
-
-	/**
-	 * A callback for Horde_Text_Filter.
-	 *
-	 * The purpose of this callback is to overwrite the default behavior
-	 * of html2text filter to convert <p>Hello</p> => Hello\n\n with
-	 * <p>Hello</p> => Hello\n.
-	 *
-	 * @param DOMDocument $doc
-	 * @param DOMNode $node
-	 * @return string|null non-null, add this text to the output and skip further processing of the node.
-	 */
-	public function htmlToTextCallback(DOMDocument $doc, DOMNode $node) {
-		if ($node instanceof DOMElement && strtolower($node->tagName) === 'p') {
-			return $node->textContent . "\n";
-		}
-
-		return null;
-	}
 }

src/ckeditor/mail/MailPlugin.js

@@ -54,7 +54,6 @@ import {
 } from 'ckeditor5'
 import { getLinkWithPicker, searchProvider } from '@nextcloud/vue/components/NcRichText'
 import TextDirectionPlugin from '../ckeditor/direction/TextDirectionPlugin.js'
-import MailPlugin from '../ckeditor/mail/MailPlugin.js'
 import QuotePlugin from '../ckeditor/quote/QuotePlugin.js'
 import SignaturePlugin from '../ckeditor/signature/SignaturePlugin.js'
 import PickerPlugin from '../ckeditor/smartpicker/PickerPlugin.js'
@@ -153,7 +152,6 @@ export default {
 				Font,
 				RemoveFormat,
 				Base64UploadAdapter,
-				MailPlugin,
 				SourceEditing,
 				TextDirectionPlugin,
 			])
@@ -671,6 +669,11 @@ export default {
 	cursor: text;
 	margin: 0 !important;
 }
+
+// Show empty lines correctly in composer
+:deep(p + p) {
+	margin-top: 1em !important;
+}
 </style>
 
 <style>

src/components/TextEditor.vue

@@ -8,7 +8,6 @@ import { Paragraph } from 'ckeditor5'
 import mitt from 'mitt'
 import { vi } from 'vitest'
 import TextEditor from '../../../components/TextEditor.vue'
-import MailPlugin from '../../../ckeditor/mail/MailPlugin.js'
 import Nextcloud from '../../../mixins/Nextcloud.js'
 import VirtualTestEditor from '../../virtualtesteditor.js'
 
@@ -90,7 +89,7 @@ describe('TextEditor', () => {
 
 		expect(wrapper.emitted().ready[0]).toBeTruthy()
 	})
-	it('register conversion to add margin: 0px to every <p> element', async () => {
+	it('does not add inline margin style to paragraph elements', async () => {
 		const wrapper = shallowMount(TextEditor, {
 			localVue,
 			propsData: {
@@ -106,7 +105,7 @@ describe('TextEditor', () => {
 		const editor = await VirtualTestEditor.create({
 			licenseKey: 'GPL',
 			initialData: '<p>bonjour bonjour</p>',
-			plugins: [Paragraph, MailPlugin],
+			plugins: [Paragraph],
 		})
 
 		editor.ui = {
@@ -123,7 +122,7 @@ describe('TextEditor', () => {
 
 		expect(wrapper.emitted().ready[0]).toBeTruthy()
 		expect(wrapper.emitted().ready[0][0].getData())
-			.toEqual('<p style="margin:0;">bonjour bonjour</p>')
+			.toEqual('<p>bonjour bonjour</p>')
 	})
 
 	it('emits updated data while editing source', async () => {

src/tests/unit/components/MailPlugin.spec.js

@@ -87,9 +87,9 @@ describe('text', () => {
 			expect(actual).toEqual(expected)
 		})
 
-		it('produces a single line break between paragraphs', () => {
+		it('produces a blank line between paragraphs', () => {
 			const source = html('<p>hello</p><p>world</p>')
-			const expected = plain('hello\nworld')
+			const expected = plain('hello\n\nworld')
 
 			const actual = toPlain(source)
 
@@ -106,7 +106,7 @@ describe('text', () => {
 		})
 
 		it('produces a single line break after each block element', () => {
-			const selectors = ['p', 'div', 'header', 'footer', 'form', 'article', 'aside', 'main', 'nav', 'section']
+			const selectors = ['div', 'header', 'footer', 'form', 'article', 'aside', 'main', 'nav', 'section']
 			const source = html(selectors
 				.map((tag) => `<${tag}>foobar</${tag}>`)
 				.join(''))
@@ -118,7 +118,7 @@ describe('text', () => {
 		})
 
 		it('produces exactly one line break for each closing block element', () => {
-			const selectors = ['p', 'div', 'header', 'footer', 'form', 'article', 'aside', 'main', 'nav', 'section']
+			const selectors = ['div', 'header', 'footer', 'form', 'article', 'aside', 'main', 'nav', 'section']
 			const source = html(selectors
 				.map((tag) => `<${tag}><${tag}>foobar</${tag}></${tag}>`)
 				.join(''))
@@ -142,7 +142,7 @@ describe('text', () => {
 			const source = html('<html>'
 				+ '<body><p>Hello!</p><p>this <i>is</i> <b>some</b> random <strong>text</strong></p></body>'
 				+ '</html>')
-			const expected = plain('Hello!\nthis is some random text')
+			const expected = plain('Hello!\n\nthis is some random text')
 
 			const actual = toPlain(source)
 

src/tests/unit/components/TextEditor.spec.js

@@ -100,9 +100,10 @@ export function toPlain(text) {
 		return text
 	}
 
-	// Build shared options for all block tags
-	const blockTags = ['p', 'div', 'header', 'footer', 'form', 'article', 'aside', 'main', 'nav', 'section']
-	const blockSelectors = blockTags.map((tag) => ({
+	// Build shared options for non-paragraph block tags.
+	// <p> is handled separately by customParagraph below.
+	const nonParagraphBlockTags = ['div', 'header', 'footer', 'form', 'article', 'aside', 'main', 'nav', 'section']
+	const blockSelectors = nonParagraphBlockTags.map((tag) => ({
 		selector: tag,
 		format: 'customBlock',
 		options: {
@@ -130,6 +131,23 @@ export function toPlain(text) {
 				// each closing tag.
 				builder.addLineBreak()
 			},
+			// <p> is a paragraph element: two forced line breaks produce the blank
+			// line that separates paragraphs in plain text, matching email convention
+			// where a paragraph break equals one blank line (i.e. \n\n).
+			customParagraph(elem, walk, builder, formatOptions) {
+				builder.openBlock({
+					isPre: formatOptions.preserveLeadingWhitespace,
+					leadingLineBreaks: 0,
+				})
+				walk(elem.children, builder)
+				builder.closeBlock({
+					trailingLineBreaks: 0,
+					blockTransform: (text) => text
+						.replace(/^ {2,}/gm, ' '),
+				})
+				builder.addLineBreak()
+				builder.addLineBreak()
+			},
 			customBlockQuote(elem, walk, builder, formatOptions) {
 				builder.openBlock({
 					leadingLineBreaks: formatOptions.leadingLineBreaks,
@@ -163,6 +181,13 @@ export function toPlain(text) {
 					trailingLineBreaks: 1,
 				},
 			},
+			{
+				selector: 'p',
+				format: 'customParagraph',
+				options: {
+					preserveLeadingWhitespace: true,
+				},
+			},
 			...blockSelectors,
 		],
 	})