From 3ef2e6946f6ddb42efa98025956de29aea95168b Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Tue, 25 Aug 2026 19:49:49 +0200 Subject: [PATCH] fix: embed the signature into re-opened drafts #848 --- ...composer-reopened-draft-signature.test.tsx | 286 ++++++++++++++++++ components/email/email-composer.tsx | 31 +- 2 files changed, 316 insertions(+), 1 deletion(-) create mode 100644 components/email/__tests__/composer-reopened-draft-signature.test.tsx diff --git a/components/email/__tests__/composer-reopened-draft-signature.test.tsx b/components/email/__tests__/composer-reopened-draft-signature.test.tsx new file mode 100644 index 000000000..9bd5baea0 --- /dev/null +++ b/components/email/__tests__/composer-reopened-draft-signature.test.tsx @@ -0,0 +1,286 @@ +import { render, screen, fireEvent, waitFor } from '@testing-library/react'; +import { describe, it, expect, vi, afterEach } from 'vitest'; +import React from 'react'; +import { EmailComposer } from '../email-composer'; +import { useSettingsStore } from '@/stores/settings-store'; +import { useIdentityStore } from '@/stores/identity-store'; +import { SIGNATURE_BLOCK_MARKER, SIGNATURE_RANGE_MARKER, buildSignatureBlock } from '../signature-block'; + +// ─── Heavy component mocks (mirrors composer-draft-attachments.test.tsx) ───── + +vi.mock('@/components/email/rich-text-editor', () => ({ + RichTextEditor: () => React.createElement('div', { 'data-testid': 'rich-text-editor' }), +})); + +vi.mock('@/components/plugins/plugin-slot', () => ({ PluginSlot: () => null })); +vi.mock('@/components/identity/sub-address-helper', () => ({ SubAddressHelper: () => null })); +vi.mock('@/components/templates/template-picker', () => ({ TemplatePicker: () => null })); +vi.mock('@/components/templates/template-form', () => ({ TemplateForm: () => null })); +vi.mock('@/components/files/file-preview-modal', () => ({ FilePreviewModal: () => null })); +vi.mock('@/hooks/use-focus-trap', () => ({ + useFocusTrap: () => ({ current: null }), +})); +vi.mock('@/hooks/use-pro-multi-account-identities', () => ({ + useProMultiAccountIdentities: () => ({ enabled: false, groups: [], allIdentities: [] }), + stripCrossAccountIdentityPrefix: (id: string) => ({ localAccountId: null, rawId: id }), +})); + +// ─── Store mocks ────────────────────────────────────────────────────────────── + +vi.mock('@/stores/auth-store', () => { + const state = { + client: null, + identities: [], + primaryIdentity: null, + isAuthenticated: false, + isDemoMode: false, + activeAccountId: null, + connectionLost: false, + getClientForAccount: () => undefined, + getAllConnectedClients: () => new Map(), + syncIdentities: () => {}, + refreshIdentities: async () => {}, + }; + const hook = (sel?: (s: typeof state) => unknown) => + typeof sel === 'function' ? sel(state) : state; + hook.getState = () => state; + hook.setState = (p: Partial) => Object.assign(state, p); + return { useAuthStore: hook }; +}); + +vi.mock('@/stores/identity-store', () => { + const state = { + identities: [ + { id: 'id-me', email: 'me@example.com', name: 'Me', htmlSignature: 'Alice', textSignature: 'Alice' }, + ] as Array>, + defaultIdentityId: 'id-me', + }; + const hook = (sel?: (s: typeof state) => unknown) => + typeof sel === 'function' ? sel(state) : state; + hook.getState = () => state; + hook.setState = (p: Partial) => Object.assign(state, p); + return { useIdentityStore: hook }; +}); + +vi.mock('@/stores/account-store', () => { + const state = { accounts: [], getAccountById: () => undefined }; + const hook = (sel?: (s: typeof state) => unknown) => + typeof sel === 'function' ? sel(state) : state; + hook.getState = () => state; + hook.setState = (p: Partial) => Object.assign(state, p); + return { useAccountStore: hook }; +}); + +vi.mock('@/stores/email-store', () => { + const state = { + draftSaveEnabled: false, + sendRawEmail: async () => ({ sent: true }), + }; + const hook = (sel?: (s: typeof state) => unknown) => + typeof sel === 'function' ? sel(state) : state; + hook.getState = () => state; + hook.setState = (p: Partial) => Object.assign(state, p); + return { useEmailStore: hook }; +}); + +vi.mock('@/stores/settings-store', () => { + const state = { + timeFormat: '24h', + plainTextMode: false, + subAddressDelimiter: '+', + autoSelectReplyIdentity: true, + attachmentReminderEnabled: false, + attachmentReminderKeywords: [], + emptySubjectWarningEnabled: true, + sendDelaySeconds: 0, + // The default position: replies/forwards get the signature appended at + // send time rather than embedded above the quote. + signaturePosition: 'below_quote', + signatureSeparatorEnabled: true, + requestReadReceiptDefault: false, + addTrustedSender: () => {}, + trustedSendersAddressBook: null, + updateSetting: () => {}, + }; + const hook = (sel?: (s: typeof state) => unknown) => + typeof sel === 'function' ? sel(state) : state; + hook.getState = () => state; + hook.setState = (p: Partial) => Object.assign(state, p); + return { useSettingsStore: hook }; +}); + +vi.mock('@/stores/contact-store', () => { + const state = { + contacts: [], + getAutocomplete: async () => [], + addToTrustedSendersBook: async () => {}, + }; + const hook = (sel?: (s: typeof state) => unknown) => + typeof sel === 'function' ? sel(state) : state; + hook.getState = () => state; + hook.setState = (p: Partial) => Object.assign(state, p); + return { useContactStore: hook }; +}); + +vi.mock('@/stores/template-store', () => { + const state = { templates: [], addTemplate: async () => {} }; + const hook = (sel?: (s: typeof state) => unknown) => + typeof sel === 'function' ? sel(state) : state; + hook.getState = () => state; + hook.setState = (p: Partial) => Object.assign(state, p); + return { useTemplateStore: hook }; +}); + +// ─── Misc dependency mocks ──────────────────────────────────────────────────── + +vi.mock('@/stores/toast-store', () => ({ + toast: { info: () => {}, error: () => {}, success: () => {} }, +})); + +vi.mock('@/lib/plugin-hooks', () => ({ + emailHooks: { + onComposerOpen: { call: async () => [] }, + onRecipientChange: { call: async () => [] }, + getRecipientSuggestions: { call: async () => [] }, + onRecipientChipsChange: { transform: async (chips: unknown) => chips }, + onDraftChange: { emit: () => {} }, + onBeforeDraftAutoSave: { transform: async (draft: unknown) => draft }, + onBeforeEmailSend: { intercept: async () => true }, + onComposeSend: { intercept: async () => true }, + onTransformOutgoingEmail: { transform: async (email: unknown) => email }, + }, + contactHooks: { + search: { call: async () => [] }, + onProvideRecipientSuggestions: { transform: async (initial: unknown) => initial }, + }, +})); + +vi.mock('@/lib/email-sanitization', () => ({ + sanitizeSignatureHtml: (v: string) => v, + sanitizeSignatureHtmlForDisplay: (v: string) => v, + sanitizeEmailHtml: (v: string) => v, + sanitizePluginBodyHtml: (v: string) => v, + escapeHtml: (v: string) => v, + parseHtmlSafely: (html: string) => new DOMParser().parseFromString(html, 'text/html'), +})); + +vi.mock('@/lib/email-threading', () => ({ + computeReplyThreadingHeaders: () => ({ inReplyTo: [], references: [] }), +})); +vi.mock('@/lib/sub-addressing', () => ({ generateSubAddress: () => '' })); +vi.mock('@/lib/debug', () => ({ debug: { log: () => {}, warn: () => {}, error: () => {} } })); +vi.mock('@/components/email/quoted-html', () => ({ + buildQuotedHtmlBlock: () => '', + serializeEditorContent: () => '', +})); +vi.mock('@/lib/template-utils', () => ({ substitutePlaceholders: (s: string) => s })); + +// ─── Tests ──────────────────────────────────────────────────────────────────── + +/** + * #848: a draft is always re-opened in `compose` mode with its body handed in + * through initialData - so getInitialBody, the only place that embeds the + * signature into a compose body, never runs. The send path nevertheless + * treated every compose body as already carrying the signature and skipped + * the append, so a "below quote" reply draft (whose body has no signature) + * went out without one, with the preview hidden and no hint to the user. + * + * The body now gets the signature embedded when it is mounted from + * initialData and does not carry it yet - keeping the compose-mode invariant + * (#329: the signature lives in the editor, editable) true instead of assumed. + */ + +const REOPENED_REPLY_DRAFT = { + to: 'bob@example.com', + cc: '', + bcc: '', + subject: 'Re: Quarterly numbers', + // What a "below quote" reply draft saved before #823 looks like: the user's + // text plus the quote, and no signature anywhere. + body: '

Thanks, looks good.

original mail
', + showCc: false, + showBcc: false, + selectedIdentityId: 'id-me', + subAddressTag: '', + mode: 'compose' as const, + draftId: 'draft-v1', +}; + +const sendButton = () => screen.getAllByTestId('composer-send')[0] as HTMLButtonElement; +const countOf = (haystack: string, needle: string) => haystack.split(needle).length - 1; + +async function sendAndCapture(initialData: typeof REOPENED_REPLY_DRAFT) { + const onSend = vi.fn(); + render(); + fireEvent.click(sendButton()); + await waitFor(() => expect(onSend).toHaveBeenCalledTimes(1)); + return onSend.mock.calls[0][0] as { body: string; htmlBody?: string }; +} + +describe('re-opened draft signature (#848)', () => { + afterEach(() => { + useSettingsStore.setState({ plainTextMode: false }); + vi.clearAllMocks(); + }); + + it('sends a re-opened reply draft with exactly one signature', async () => { + const sent = await sendAndCapture(REOPENED_REPLY_DRAFT); + + expect(sent.htmlBody).toContain('Alice'); + expect(countOf(sent.htmlBody!, SIGNATURE_BLOCK_MARKER)).toBe(1); + // Embedded after the quote - where the "below quote" position puts it and + // where a #823-saved draft carries it. + expect(sent.htmlBody!.indexOf('original mail')).toBeLessThan(sent.htmlBody!.indexOf('Alice')); + expect(sent.body).toContain('Alice'); + }); + + it('does not show the read-only preview as a second copy of the embedded signature', () => { + render(); + // The editor is mocked, so the only way "Alice" could appear is the + // below-editor preview - and the signature is in the body now. + expect(screen.queryByText('Alice')).not.toBeInTheDocument(); + }); + + it('leaves a draft that already carries the embedded signature alone', async () => { + const body = `

Thanks.

original mail
` + + `

--

${buildSignatureBlock('Alice')}

`; + const sent = await sendAndCapture({ ...REOPENED_REPLY_DRAFT, body }); + + expect(countOf(sent.htmlBody!, SIGNATURE_BLOCK_MARKER)).toBe(1); + expect(countOf(sent.htmlBody!, 'Alice')).toBe(1); + }); + + it('leaves the body untouched when the identity has no signature', async () => { + const identities = useIdentityStore.getState().identities; + useIdentityStore.setState({ identities: [{ id: 'id-me', email: 'me@example.com', name: 'Me', mayDelete: true }] }); + try { + const sent = await sendAndCapture(REOPENED_REPLY_DRAFT); + expect(sent.htmlBody).not.toContain(SIGNATURE_RANGE_MARKER); + expect(sent.htmlBody).not.toContain('Alice'); + } finally { + useIdentityStore.setState({ identities }); + } + }); + + describe('plain text mode', () => { + it('appends the text signature to a re-opened draft that lacks it', async () => { + useSettingsStore.setState({ plainTextMode: true }); + const sent = await sendAndCapture({ + ...REOPENED_REPLY_DRAFT, + body: 'Thanks, looks good.\n\n> original mail', + }); + + expect(sent.body).toBe('Thanks, looks good.\n\n> original mail\n\n-- \nAlice'); + }); + + it('does not duplicate a text signature the draft already ends with', async () => { + useSettingsStore.setState({ plainTextMode: true }); + const sent = await sendAndCapture({ + ...REOPENED_REPLY_DRAFT, + body: 'Thanks.\n\n> original mail\n\n-- \nAlice', + }); + + expect(sent.body).toBe('Thanks.\n\n> original mail\n\n-- \nAlice'); + }); + }); +}); diff --git a/components/email/email-composer.tsx b/components/email/email-composer.tsx index 28082b666..35f647cd8 100644 --- a/components/email/email-composer.tsx +++ b/components/email/email-composer.tsx @@ -507,6 +507,33 @@ export function EmailComposer({ return prefix; }; + // A body handed in through `initialData` - a re-opened draft, a mailto: + // link, a restored composer stash - is mounted in `compose` mode like a new + // mail, but unlike one it was never built by getInitialBody, so nothing + // embedded the signature. The send/draft/preview paths still treated every + // compose body as carrying it, so such a body went out without any + // signature and without a hint in the UI (#848): every "below quote" reply + // draft saved before #823, drafts written by other clients, mailto bodies. + // Embed it here, where the body is actually built, so the assumption holds + // and the signature stays editable/removable in the editor (#329). A body + // that already carries it (a #823 draft, the stash of a compose in + // progress) is left alone. + const withSignatureEmbedded = (provided: string): string => { + if (!shouldEmbedSignatureInNewMail) return provided; + if (plainTextMode) { + if (plainTextBodyHasSignature(provided, initialSignatureIdentity)) return provided; + return appendPlainTextSignature(provided, initialSignatureIdentity, { + separator: signatureSeparatorEnabled, + }); + } + if (containsEmbeddedSignature(provided)) return provided; + const embedded = buildEmbeddedSignatureHtml(initialSignatureIdentity, { + embed: true, + separator: signatureSeparatorEnabled, + }); + return `${provided || '

'}${embedded}`; + }; + // Committed recipients are structured arrays; the in-progress text the user // is typing lives in a separate `*Input` string per field. This keeps a // display name containing a comma (e.g. "Doo, John") intact instead of @@ -518,7 +545,9 @@ export function EmailComposer({ const [ccInput, setCcInput] = useState(''); const [bccInput, setBccInput] = useState(''); const [subject, setSubject] = useState(initialData?.subject ?? getInitialSubject()); - const [body, setBody] = useState(initialData?.body ?? getInitialBody()); + const [body, setBody] = useState(() => + initialData?.body != null ? withSignatureEmbedded(initialData.body) : getInitialBody() + ); const [showCc, setShowCc] = useState(initialData?.showCc ?? getInitialCc().length > 0); const [showBcc, setShowBcc] = useState(initialData?.showBcc ?? false); // Committed recipients plus any not-yet-committed text the user has typed.