From 83026fe621ddc1f80e8f2ff0a8850d1ec3416d46 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Sun, 16 Aug 2026 22:20:49 +0200 Subject: [PATCH] fix: keep already written text when applying a template in compose #540 --- components/email/email-composer.tsx | 40 ++++++++++++++++++++++----- lib/__tests__/signature-utils.test.ts | 31 +++++++++++++++++++++ lib/__tests__/template-utils.test.ts | 37 +++++++++++++++++++++++++ lib/signature-utils.ts | 23 +++++++++++++++ lib/template-utils.ts | 38 +++++++++++++++++++++++-- 5 files changed, 160 insertions(+), 9 deletions(-) diff --git a/components/email/email-composer.tsx b/components/email/email-composer.tsx index 3396e106e..d808aff6d 100644 --- a/components/email/email-composer.tsx +++ b/components/email/email-composer.tsx @@ -31,11 +31,11 @@ import { useContactStore, getContactDisplayName, getContactPrimaryEmail } from " import { useTemplateStore } from "@/stores/template-store"; import { SubAddressHelper } from "@/components/identity/sub-address-helper"; import { generateSubAddress } from "@/lib/sub-addressing"; -import { substitutePlaceholders, spliceTemplateAboveSignature } from "@/lib/template-utils"; +import { substitutePlaceholders, spliceTemplateAboveSignature, composeBodyHasUserContent } from "@/lib/template-utils"; import { TemplatePicker } from "@/components/templates/template-picker"; import { TemplateForm } from "@/components/templates/template-form"; import type { EmailTemplate } from "@/lib/template-types"; -import { appendPlainTextSignature, getPlainTextSignature, plainTextBodyHasSignature } from "@/lib/signature-utils"; +import { appendPlainTextSignature, getPlainTextSignature, plainTextBodyHasSignature, plainTextBodyWithoutSignature } from "@/lib/signature-utils"; import { findComposeIdentityId, findDraftIdentityId, resolveReplyFrom } from "@/lib/reply-identity"; import { buildReplyRecipients, isSelfSent } from "@/lib/reply-recipients"; import { computeReplyThreadingHeaders } from "@/lib/email-threading"; @@ -1199,18 +1199,44 @@ export function EmailComposer({ : `

${filledBody.replace(/&/g, '&').replace(//g, '>').replace(/\n/g, '
')}

`; if (mode === 'compose') { - setSubject(filledSubject); + // An empty template subject must not wipe one the user typed (#540). + if (filledSubject) { + setSubject(filledSubject); + } // Compose bodies carry the embedded signature (see // shouldEmbedSignatureInNewMail) and the send path assumes it stays // there, so replace only the message content, not the signature block. // Keyed on the previous body rather than the mode: a re-opened draft is // in compose mode too but carries its own signature (#823). + // Once the user has written something, the template is inserted at the + // caret instead of replacing that text, as in reply/forward mode (#540). if (plainTextMode) { - setBody((prev) => plainTextBodyHasSignature(prev, signatureIdentity) - ? appendPlainTextSignature(bodyContent, signatureIdentity, { separator: signatureSeparatorEnabled }) - : bodyContent); + const textarea = bodyRef.current; + const hasUserText = !!textarea + && plainTextBodyWithoutSignature(textarea.value, signatureIdentity).trim().length > 0; + if (hasUserText && textarea) { + const start = textarea.selectionStart ?? textarea.value.length; + const end = textarea.selectionEnd ?? start; + setBody((prev) => prev.slice(0, start) + bodyContent + prev.slice(end)); + requestAnimationFrame(() => { + const caret = start + bodyContent.length; + textarea.focus(); + textarea.setSelectionRange(caret, caret); + }); + } else { + setBody((prev) => plainTextBodyHasSignature(prev, signatureIdentity) + ? appendPlainTextSignature(bodyContent, signatureIdentity, { separator: signatureSeparatorEnabled }) + : bodyContent); + } } else { - setBody((prev) => spliceTemplateAboveSignature(prev, bodyContent)); + const editor = editorRef.current; + // serializeEditorContent (not getHTML) so the check sees the same + // markup the body state holds. + if (editor && composeBodyHasUserContent(serializeEditorContent(editor))) { + editor.chain().focus().insertContent(bodyContent).run(); + } else { + setBody((prev) => spliceTemplateAboveSignature(prev, bodyContent)); + } } if (template.defaultRecipients?.to?.length) { setTo(template.defaultRecipients.to.map(parseRecipient)); diff --git a/lib/__tests__/signature-utils.test.ts b/lib/__tests__/signature-utils.test.ts index 878d6c0ad..795e57419 100644 --- a/lib/__tests__/signature-utils.test.ts +++ b/lib/__tests__/signature-utils.test.ts @@ -6,6 +6,7 @@ import { getPlainTextSignature, hasMeaningfulHtmlBody, plainTextBodyHasSignature, + plainTextBodyWithoutSignature, } from '../signature-utils'; describe('signature-utils', () => { @@ -55,6 +56,36 @@ describe('signature-utils', () => { }); }); + describe('plainTextBodyWithoutSignature', () => { + const identity = { textSignature: 'Regards,\nAlice' }; + + it('returns nothing for a body that is only the signature (#540)', () => { + expect(plainTextBodyWithoutSignature(appendPlainTextSignature('', identity), identity)).toBe(''); + }); + + it('returns the text the user typed above the signature', () => { + expect(plainTextBodyWithoutSignature(appendPlainTextSignature('Hello Mr.', identity), identity)).toBe('Hello Mr.'); + }); + + it('strips the signature after a CRLF round-trip', () => { + const saved = appendPlainTextSignature('Hello Mr.', identity).replace(/\n/g, '\r\n'); + expect(plainTextBodyWithoutSignature(saved, identity)).toBe('Hello Mr.'); + }); + + it('handles a body written without the "-- " separator', () => { + const saved = appendPlainTextSignature('Hello Mr.', identity, { separator: false }); + expect(plainTextBodyWithoutSignature(saved, identity)).toBe('Hello Mr.'); + }); + + it('leaves a body that does not end with the signature untouched', () => { + expect(plainTextBodyWithoutSignature('Hello Mr.', identity)).toBe('Hello Mr.'); + }); + + it('leaves the body untouched when the identity has no signature', () => { + expect(plainTextBodyWithoutSignature('Hello Mr.', {})).toBe('Hello Mr.'); + }); + }); + describe('appendHtmlSignature', () => { it('appends a sanitized html signature, preserving formatting', () => { expect(appendHtmlSignature('
Hello
', { htmlSignature: 'Alice' })) diff --git a/lib/__tests__/template-utils.test.ts b/lib/__tests__/template-utils.test.ts index 32162a182..16404ade4 100644 --- a/lib/__tests__/template-utils.test.ts +++ b/lib/__tests__/template-utils.test.ts @@ -11,6 +11,7 @@ import { exportTemplates, importTemplates, spliceTemplateAboveSignature, + composeBodyHasUserContent, } from '../template-utils'; import type { EmailTemplate } from '../template-types'; @@ -371,3 +372,39 @@ describe('spliceTemplateAboveSignature', () => { expect(result).toContain('Sig'); }); }); + +describe('composeBodyHasUserContent', () => { + const signature = '

--

My signature

'; + + it('is false for a fresh compose body (empty paragraph + signature)', () => { + expect(composeBodyHasUserContent(`

${signature}`)).toBe(false); + }); + + it('is false for an empty body', () => { + expect(composeBodyHasUserContent('')).toBe(false); + expect(composeBodyHasUserContent('

')).toBe(false); + }); + + it('is true once the user typed above the signature (#540)', () => { + expect(composeBodyHasUserContent(`

Hello Mr.

${signature}`)).toBe(true); + }); + + it('is true for a body without a signature block', () => { + expect(composeBodyHasUserContent('

Hello Mr.

')).toBe(true); + }); + + it('ignores signature text so a signature-only body stays untouched', () => { + expect(composeBodyHasUserContent(`

${signature}`)).toBe(false); + expect(composeBodyHasUserContent(`


${signature}`)).toBe(false); + }); + + it('counts text-free content such as a pasted image', () => { + expect(composeBodyHasUserContent(`

${signature}`)).toBe(true); + }); + + it('handles a signature range without an end marker', () => { + const openEnded = '

My signature
'; + expect(composeBodyHasUserContent(`

${openEnded}`)).toBe(false); + expect(composeBodyHasUserContent(`

Hello Mr.

${openEnded}`)).toBe(true); + }); +}); diff --git a/lib/signature-utils.ts b/lib/signature-utils.ts index 330fe45f7..180ae7854 100644 --- a/lib/signature-utils.ts +++ b/lib/signature-utils.ts @@ -61,6 +61,29 @@ export function plainTextBodyHasSignature( return normalizeSignatureLineBreaks(body).endsWith(plainTextSignature); } +/** + * The plain-text body with a trailing signature - and the `-- ` separator + * line in front of it - removed. Returns the body unchanged when it does not + * end with the identity's signature. Used to tell an untouched compose body + * (signature only) from one the user has already written into (#540). + */ +export function plainTextBodyWithoutSignature( + body: string, + signature?: SignatureSource | null, +): string { + const plainTextSignature = getPlainTextSignature(signature); + if (!plainTextSignature) { + return body; + } + const normalized = normalizeSignatureLineBreaks(body); + if (!normalized.endsWith(plainTextSignature)) { + return body; + } + return normalized + .slice(0, normalized.length - plainTextSignature.length) + .replace(/\n*(?:-- ?)?\n*$/, ''); +} + /** * Append a signature to an HTML body, preserving rich formatting. Used by the * quick-reply path so an HTML signature keeps its markup instead of being diff --git a/lib/template-utils.ts b/lib/template-utils.ts index a4ca6f32d..114fa84b2 100644 --- a/lib/template-utils.ts +++ b/lib/template-utils.ts @@ -80,6 +80,9 @@ export function filterTemplates(templates: EmailTemplate[], query: string): Emai ); } +const SIGNATURE_START_SELECTOR = '[data-signature-block="separator"], [data-signature-block="start"]'; +const SIGNATURE_END_SELECTOR = '[data-signature-block="end"]'; + // Compose bodies carry the embedded signature bracketed by // data-signature-block markers (see email-composer's // buildEmbeddedSignatureHtml). Applying a template must replace only the @@ -87,9 +90,9 @@ export function filterTemplates(templates: EmailTemplate[], query: string): Emai // of overwriting the whole body. export function spliceTemplateAboveSignature(prevHtml: string, templateHtml: string): string { const doc = new DOMParser().parseFromString(prevHtml, 'text/html'); - const startEl = doc.querySelector('[data-signature-block="separator"], [data-signature-block="start"]'); + const startEl = doc.querySelector(SIGNATURE_START_SELECTOR); if (!startEl) return templateHtml; - const endEl = doc.querySelector('[data-signature-block="end"]'); + const endEl = doc.querySelector(SIGNATURE_END_SELECTOR); const host = doc.createElement('div'); let cursor: Node | null = startEl; while (cursor) { @@ -100,6 +103,37 @@ export function spliceTemplateAboveSignature(prevHtml: string, templateHtml: str return templateHtml + host.innerHTML; } +/** + * Whether a compose body holds anything the user wrote above the embedded + * signature. A fresh compose body is just an empty paragraph plus the + * signature range, so this stays false until the user types something - + * the composer then inserts the template at the caret rather than splicing + * it over the draft (#540). + */ +export function composeBodyHasUserContent(prevHtml: string): boolean { + if (!prevHtml.trim()) return false; + + const doc = new DOMParser().parseFromString(prevHtml, 'text/html'); + const startEl = doc.querySelector(SIGNATURE_START_SELECTOR); + if (startEl) { + // Drop the signature range [start … end] inclusive, mirroring the + // traversal spliceTemplateAboveSignature uses to keep it. + const endEl = doc.querySelector(SIGNATURE_END_SELECTOR); + let cursor: Node | null = startEl; + while (cursor) { + const next: Node | null = cursor.nextSibling; + const isEnd = cursor === endEl; + cursor.parentNode?.removeChild(cursor); + if (isEnd) break; + cursor = next; + } + } + + if (doc.body.textContent?.trim()) return true; + // Text-free content still counts as a draft worth keeping. + return doc.body.querySelector('img, table, hr, [data-quoted-html]') !== null; +} + function sanitizeText(value: unknown): string { return DOMPurify.sanitize(String(value || ''), STRIP_HTML_CONFIG); }