From 797fb875e1ac71e61ea4ba9678bda1532bc02c87 Mon Sep 17 00:00:00 2001 From: Till Oberbeckmann <29061652+b3rsrk@users.noreply.github.com> Date: Thu, 29 Oct 2026 10:01:12 +0100 Subject: [PATCH] fix: keep the paragraph spacing of a draft written elsewhere #1416 A draft does not have to come from the composer: the JMAP API, another client, an automation or a template can leave one in Drafts, and its paragraphs carry no style, so every mail client lays them out with the user agent's 1em margins. Since #1340 the composer stamps its own 4px margin on every paragraph it serializes, so opening such a draft to read it over and pressing Send re-flowed the message, and the composer showed it at a spacing the recipient would never see. What is loaded now keeps the spacing it came with: a paragraph with text and no margin of its own is stamped with the mail default on the way in, so the editor shows the message's own layout and what goes out is what came in. The empty paragraph a new message is scaffolded with is left alone, as are table cells, the quoted original and an embedded signature. addParagraphSpacing skips a paragraph that already states a margin, so nothing is declared twice. Co-Authored-By: Claude Opus 5 --- .../rich-text-editor-empty-lines.test.tsx | 8 +- .../rich-text-editor-loaded-spacing.test.tsx | 77 +++++++++++++++++++ ...ich-text-editor-paragraph-spacing.test.tsx | 28 +++++-- components/email/paragraph-breaks.ts | 52 ++++++++++++- components/email/rich-text-editor.tsx | 10 ++- 5 files changed, 162 insertions(+), 13 deletions(-) create mode 100644 components/email/__tests__/rich-text-editor-loaded-spacing.test.tsx diff --git a/components/email/__tests__/rich-text-editor-empty-lines.test.tsx b/components/email/__tests__/rich-text-editor-empty-lines.test.tsx index 2075c55ae..3c31f15f5 100644 --- a/components/email/__tests__/rich-text-editor-empty-lines.test.tsx +++ b/components/email/__tests__/rich-text-editor-empty-lines.test.tsx @@ -19,11 +19,13 @@ async function setup(content: string) { return editor as unknown as Editor; } -// The composer marks every block dir="auto" and inlines the paragraph margin -// (#1340); that's not what these tests are about. +// The composer marks every block dir="auto" and inlines the paragraph margin - +// its own (#1340) or the one a loaded message brought along (#1416); neither is +// what these tests are about. const serialize = (editor: Editor) => serializeEditorContent(editor) .replace(/ dir="auto"/g, '') - .replace(/ style="margin:4px 0;"/g, ''); + .replace(/ style="margin: ?\d+(?:px)? 0(?:px)?;"/g, '') + .replace(/ style="margin: ?1em 0(?:px)?;"/g, ''); // Re-open what was saved, the way a draft or a signature swap does. function reload(editor: Editor, html: string) { diff --git a/components/email/__tests__/rich-text-editor-loaded-spacing.test.tsx b/components/email/__tests__/rich-text-editor-loaded-spacing.test.tsx new file mode 100644 index 000000000..3f11d9070 --- /dev/null +++ b/components/email/__tests__/rich-text-editor-loaded-spacing.test.tsx @@ -0,0 +1,77 @@ +import { act, render, waitFor } from '@testing-library/react'; +import { describe, expect, it } from 'vitest'; +import React from 'react'; +import type { Editor } from '@tiptap/core'; + +import { RichTextEditor } from '../rich-text-editor'; +import { serializeEditorContent } from '../quoted-html'; + +async function setup(content: string) { + let editor: Editor | null = null; + render( + {}} + onEditorReady={(ed) => { editor = ed as unknown as Editor; }} + /> + ); + await waitFor(() => expect(editor).not.toBeNull()); + return editor as unknown as Editor; +} + +const serialize = (editor: Editor) => serializeEditorContent(editor).replace(/ dir="auto"/g, ''); +const margins = (html: string) => Array.from(html.matchAll(/margin: ?([^;"]+)/g)).map((m) => m[1].trim()); + +describe('RichTextEditor spacing of a loaded message (#1416)', () => { + it('sends a draft written by another client with the spacing it came with', async () => { + const editor = await setup('

one

two

'); + + expect(margins(serialize(editor))).toEqual(['1em 0px', '1em 0px']); + }); + + it('shows that spacing in the editor, so what is proofread is what is sent', async () => { + const editor = await setup('

one

'); + + expect(margins(editor.getHTML())).toEqual(['1em 0px']); + }); + + it('keeps a paragraph that states a spacing of its own', async () => { + const editor = await setup('

wide

'); + + expect(margins(serialize(editor))).toEqual(['24px 0px']); + }); + + it('leaves the empty paragraph a new message starts with to the composer', async () => { + const editor = await setup('

'); + + expect(serialize(editor)).toBe('

'); + }); + + it('does not stack a margin when the draft is opened again', async () => { + const editor = await setup('

one

two

'); + const saved = serialize(editor); + + act(() => { editor.commands.setContent(saved, { emitUpdate: false }); }); + + expect(serialize(editor)).toBe(saved); + }); + + it('leaves the quoted original and an embedded signature alone', async () => { + const editor = await setup( + '

reply

' + + '

Alice

' + + '

quoted

', + ); + + const html = serialize(editor); + + expect(html).toContain('

quoted

'); + expect(html).toContain('

Alice

'); + }); + + it('leaves a table cell flush, in the mail as in the composer', async () => { + const editor = await setup('

cell

'); + + expect(serialize(editor)).toContain('

cell

'); + }); +}); diff --git a/components/email/__tests__/rich-text-editor-paragraph-spacing.test.tsx b/components/email/__tests__/rich-text-editor-paragraph-spacing.test.tsx index ccbe4020e..999e39dba 100644 --- a/components/email/__tests__/rich-text-editor-paragraph-spacing.test.tsx +++ b/components/email/__tests__/rich-text-editor-paragraph-spacing.test.tsx @@ -19,23 +19,40 @@ async function setup(content: string) { return editor as unknown as Editor; } +// Lines written in the composer: start from the empty paragraph a new message +// is scaffolded with and type into it. A paragraph loaded from somewhere else +// keeps the spacing it came with instead (rich-text-editor-loaded-spacing). +async function compose(...lines: string[]) { + const editor = await setup('

'); + act(() => { + lines.forEach((line, index) => { + if (index > 0) editor.commands.splitBlock(); + editor.commands.insertContent(line); + }); + }); + return editor; +} + const serialize = (editor: Editor) => serializeEditorContent(editor).replace(/ dir="auto"/g, ''); describe('RichTextEditor paragraph spacing (#1340)', () => { it('sends each line with the 4px margin the composer shows, not the 1em default', async () => { - const editor = await setup('

one

two

'); + const editor = await compose('one', 'two'); expect(serialize(editor)).toBe('

one

two

'); }); it('keeps the alignment next to the margin', async () => { - const editor = await setup('

mid

'); + const editor = await compose('mid'); + act(() => { editor.commands.setTextAlign('center'); }); - expect(serialize(editor)).toBe('

mid

'); + expect(serialize(editor)).toContain('margin:4px 0;'); + expect(serialize(editor)).toContain('text-align: center'); }); it('spaces paragraphs in lists too', async () => { - const editor = await setup(''); + const editor = await compose('a'); + act(() => { editor.commands.toggleBulletList(); }); expect(serialize(editor)).toContain('

  • a

    • '); }); @@ -47,7 +64,8 @@ describe('RichTextEditor paragraph spacing (#1340)', () => { }); it('does not stack margins when a saved draft is opened again', async () => { - const editor = await setup('

    one

    two

    '); + const editor = await compose('one', 'two'); + act(() => { editor.commands.setTextAlign('right'); }); const saved = serialize(editor); act(() => { editor.commands.setContent(saved, { emitUpdate: false }); }); diff --git a/components/email/paragraph-breaks.ts b/components/email/paragraph-breaks.ts index 9edf8dc08..2d283c62e 100644 --- a/components/email/paragraph-breaks.ts +++ b/components/email/paragraph-breaks.ts @@ -1,4 +1,5 @@ -import { SIGNATURE_RANGE_MARKER } from "@/components/email/signature-block"; +import { SIGNATURE_BLOCK_MARKER, SIGNATURE_RANGE_MARKER } from "@/components/email/signature-block"; +import { QUOTED_HTML_MARKER } from "@/components/email/quoted-html"; /** * Empty lines in the composer (#1015). @@ -53,15 +54,62 @@ export function addTrailingParagraphBreaks(root: ParentNode): void { const PARAGRAPH_MARGIN = "margin:4px 0;"; const TABLE_PARAGRAPH_MARGIN = "margin:0;"; +/** A paragraph that brings a margin of its own, from the mail it was loaded from. */ +const OWN_MARGIN = /(?:^|;)\s*margin(?:-block)?(?:-start|-end|-top|-bottom)?\s*:/i; + export function addParagraphSpacing(root: ParentNode): void { root.querySelectorAll("p").forEach((p) => { if (p.hasAttribute(SIGNATURE_RANGE_MARKER)) return; + const style = p.getAttribute("style") ?? ""; + // A paragraph that already states its spacing keeps it: the composer's + // own margin would only be overridden by it anyway, and writing both + // leaves two margin declarations in every line of the sent mail. + if (OWN_MARGIN.test(style)) return; const margin = p.closest("table") ? TABLE_PARAGRAPH_MARGIN : PARAGRAPH_MARGIN; // Ahead of any style the editor wrote, so an explicit margin still wins. - p.setAttribute("style", margin + (p.getAttribute("style") ?? "")); + p.setAttribute("style", margin + style); }); } +/** + * Spacing of a message the composer did not write (#1416). + * + * A draft does not have to come from this composer: the JMAP API, another + * client, an automation or a template can put one in the Drafts folder, and + * its paragraphs carry no style, so every mail client lays them out with the + * user agent's 1em margins. The composer shows its own paragraphs 4px apart + * and sends that spacing out (#1340), so opening such a draft to proofread it + * and pressing Send re-flowed a message nobody meant to re-flow. + * + * Materialise the spacing a loaded paragraph already has, so the editor shows + * the message's own layout and what goes out is what came in. Only paragraphs + * with content are stamped: the empty one a new message is scaffolded with has + * no spacing to preserve, and the paragraphs the user then writes should + * follow the composer like every other line they type. + */ +const MAIL_PARAGRAPH_MARGIN = "margin:1em 0;"; + +export function keepLoadedParagraphSpacing(html: string): string { + if (!html.includes(" { + if (p.hasAttribute(SIGNATURE_RANGE_MARKER)) return; + // The quoted original and an embedded signature are held verbatim by + // their own nodes - neither is laid out by the composer. + if (p.closest(`[${QUOTED_HTML_MARKER}], [${SIGNATURE_BLOCK_MARKER}]`)) return; + // A paragraph in a table cell sits flush in the composer and in the mail. + if (p.closest("table")) return; + if (!(p.textContent ?? "").trim() && !p.querySelector("img")) return; + const style = p.getAttribute("style") ?? ""; + if (OWN_MARGIN.test(style)) return; + p.setAttribute("style", MAIL_PARAGRAPH_MARGIN + style); + stamped = true; + }); + return stamped ? root.innerHTML : html; +} + /** A paragraph's `style` without the margin addParagraphSpacing put in front. */ export function paragraphStyleWithoutSpacing(el: HTMLElement): string | null { const style = el.getAttribute("style"); diff --git a/components/email/rich-text-editor.tsx b/components/email/rich-text-editor.tsx index 5d82fbd97..f462c9568 100644 --- a/components/email/rich-text-editor.tsx +++ b/components/email/rich-text-editor.tsx @@ -23,7 +23,7 @@ import { TableRow } from "@tiptap/extension-table-row"; import { TableHeader } from "@tiptap/extension-table-header"; import { TableCell } from "@tiptap/extension-table-cell"; import { QuotedHtml, serializeEditorContent } from "@/components/email/quoted-html"; -import { paragraphStyleWithoutSpacing, trailingParagraphBreakRule } from "@/components/email/paragraph-breaks"; +import { keepLoadedParagraphSpacing, paragraphStyleWithoutSpacing, trailingParagraphBreakRule } from "@/components/email/paragraph-breaks"; import { SignatureBlock } from "@/components/email/signature-block"; import { styledBlockAttributes } from "@/components/email/styled-block-attributes"; import { RecipientMention, RecipientMentionList, type RecipientMentionListHandle } from "@/components/email/recipient-mention"; @@ -384,9 +384,13 @@ export function RichTextEditor({ // Sync external content changes (e.g. template application). Compare against // the custom serialization so a doc that only differs inside a QuotedHtml // island isn't needlessly re-parsed (which would reset the island DOM). + // What comes in keeps the paragraph spacing it was written with, so a draft + // from another client is shown and sent the way it was meant (#1416). useEffect(() => { - if (editor && content !== serializeEditorContent(editor)) { - editor.commands.setContent(content, { emitUpdate: false }); + if (!editor) return; + const loaded = keepLoadedParagraphSpacing(content); + if (loaded !== serializeEditorContent(editor)) { + editor.commands.setContent(loaded, { emitUpdate: false }); } }, [content, editor]);