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]);