From 9ff14f57814664a2258ab2933fb632f2ed319ef4 Mon Sep 17 00:00:00 2001 From: dealerweb Date: Mon, 10 Aug 2026 13:23:37 +0200 Subject: [PATCH 1/2] fix: route mailto composer replacement through the close guard Opening a mailto: link while the composer held unsaved text used to replace the session outright and drop the text on the floor. The mailto entry point now hands a follow-up to the composer's dirty-aware close handler, so the "Save or discard draft?" dialog runs first: save and discard continue into the mailto draft, cancel leaves the current draft untouched. When a failed save hands the text back to the continue-draft slot (#702), that rescue wins and the follow-up is dropped. --- .../__tests__/composer-close-guard.test.tsx | 277 ++++++++++++++++++ components/email/email-composer.tsx | 48 ++- components/mail/mail-app.tsx | 28 +- components/pro/pro-compose-tab-body.tsx | 2 +- 4 files changed, 341 insertions(+), 14 deletions(-) create mode 100644 components/email/__tests__/composer-close-guard.test.tsx diff --git a/components/email/__tests__/composer-close-guard.test.tsx b/components/email/__tests__/composer-close-guard.test.tsx new file mode 100644 index 000000000..84bdc1f37 --- /dev/null +++ b/components/email/__tests__/composer-close-guard.test.tsx @@ -0,0 +1,277 @@ +import { render, screen, fireEvent, waitFor, act, within } from '@testing-library/react'; +import { describe, it, expect, vi } from 'vitest'; +import React from 'react'; +import { EmailComposer } from '../email-composer'; + +// ─── Heavy component mocks (mirrors reply-addressing.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' }], + 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 }; +}); + +const updateSetting = vi.fn(); + +vi.mock('@/stores/settings-store', () => { + const state = { + timeFormat: '24h', + plainTextMode: false, + subAddressDelimiter: '+', + autoSelectReplyIdentity: true, + attachmentReminderEnabled: false, + attachmentReminderKeywords: [], + emptySubjectWarningEnabled: true, + sendDelaySeconds: 0, + signaturePosition: 'above_quote', + signatureSeparatorEnabled: false, + requestReadReceiptDefault: false, + addTrustedSender: () => {}, + trustedSendersAddressBook: null, + updateSetting: (...args: unknown[]) => updateSetting(...args), + }; + 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: () => {} }, + 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, + sanitizeEmailHtml: (v: string) => v, + parseHtmlSafely: (html: string) => new DOMParser().parseFromString(html, 'text/html'), +})); + +vi.mock('@/lib/email-threading', () => ({ + computeReplyThreadingHeaders: () => ({ inReplyTo: [], references: [] }), +})); +vi.mock('@/lib/signature-utils', () => ({ + appendPlainTextSignature: (body: string) => body, + getPlainTextSignature: () => '', +})); +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 ──────────────────────────────────────────────────────────────────── + +/** + * A host that replaces the compose session - a mailto: click, say - must not + * drop text the user has not sent. It hands a follow-up to requestCloseRef and + * the composer runs it only once it has really closed, so "Save" and "Discard" + * continue into the new session while "Cancel" leaves the draft standing. + */ + +const DIRTY_DRAFT = { + to: 'bob@example.com', + cc: '', + bcc: '', + subject: 'Half-written', + body: '

Still typing

', + showCc: false, + showBcc: false, + selectedIdentityId: 'id-me', + subAddressTag: '', + mode: 'compose' as const, + draftId: null, +}; + +function renderWithCloseRef(props: Record = {}) { + const requestCloseRef = { current: null } as React.MutableRefObject<((afterClose?: () => void) => void) | null>; + const afterClose = vi.fn(); + const onClose = vi.fn(); + render( + , + ); + // Typing is what makes the draft dirty; initialData alone is the baseline. + fireEvent.change(screen.getByDisplayValue('Half-written'), { + target: { value: 'Half-written more' }, + }); + return { requestCloseRef, afterClose, onClose }; +} + +describe('composer close guard with a follow-up', () => { + it('asks before replacing an unsaved draft instead of dropping it', async () => { + const { requestCloseRef, afterClose } = renderWithCloseRef(); + + act(() => requestCloseRef.current!(afterClose)); + + expect(await screen.findByText('close_draft_title')).toBeInTheDocument(); + expect(afterClose).not.toHaveBeenCalled(); + }); + + it('runs the follow-up after discarding', async () => { + const { requestCloseRef, afterClose, onClose } = renderWithCloseRef(); + + act(() => requestCloseRef.current!(afterClose)); + const dialog = await screen.findByRole('alertdialog'); + fireEvent.click(within(dialog).getByText('discard')); + + await waitFor(() => expect(afterClose).toHaveBeenCalledTimes(1)); + // The host's own close runs first, so the new session starts from a clean slate. + expect(onClose).toHaveBeenCalledTimes(1); + expect(onClose.mock.invocationCallOrder[0]).toBeLessThan(afterClose.mock.invocationCallOrder[0]); + }); + + it('drops the follow-up when the user cancels', async () => { + const { requestCloseRef, afterClose, onClose } = renderWithCloseRef(); + + act(() => requestCloseRef.current!(afterClose)); + const dialog = await screen.findByRole('alertdialog'); + fireEvent.click(within(dialog).getByText('cancel')); + + expect(afterClose).not.toHaveBeenCalled(); + expect(onClose).not.toHaveBeenCalled(); + + // A later, unrelated close must not resurrect the abandoned follow-up. + act(() => requestCloseRef.current!()); + const reopened = await screen.findByRole('alertdialog'); + fireEvent.click(within(reopened).getByText('discard')); + + await waitFor(() => expect(onClose).toHaveBeenCalledTimes(1)); + expect(afterClose).not.toHaveBeenCalled(); + }); + + it('closes straight through when there is nothing to lose', async () => { + const requestCloseRef = { current: null } as React.MutableRefObject<((afterClose?: () => void) => void) | null>; + const afterClose = vi.fn(); + render(); + + act(() => requestCloseRef.current!(afterClose)); + + await waitFor(() => expect(afterClose).toHaveBeenCalledTimes(1)); + expect(screen.queryByText('close_draft_title')).not.toBeInTheDocument(); + }); + + it('runs the follow-up after saving the draft', async () => { + const { requestCloseRef, afterClose } = renderWithCloseRef(); + + act(() => requestCloseRef.current!(afterClose)); + const dialog = await screen.findByRole('alertdialog'); + fireEvent.click(within(dialog).getByText('save')); + + await waitFor(() => expect(afterClose).toHaveBeenCalledTimes(1)); + }); +}); diff --git a/components/email/email-composer.tsx b/components/email/email-composer.tsx index 2ed1ea435..6dd65f4e7 100644 --- a/components/email/email-composer.tsx +++ b/components/email/email-composer.tsx @@ -155,8 +155,12 @@ interface EmailComposerProps { * handler shows the unsaved-changes dialog when the draft is dirty, so a * host (e.g. the Pro tab bar's close button) can route an external close * request through the same guard instead of discarding silently. + * + * A host replacing this session with another one (e.g. a mailto: click) + * can pass a follow-up that runs once the composer has actually closed. + * Cancelling the dialog drops it, so the draft simply stays put. */ - requestCloseRef?: React.MutableRefObject<(() => void) | null>; + requestCloseRef?: React.MutableRefObject<((afterClose?: () => void) => void) | null>; onDiscardDraft?: (draftId: string) => void; onSaveState?: (data: ComposerDraftData) => void; className?: string; @@ -575,7 +579,7 @@ export function EmailComposer({ const closeDialogRef = useFocusTrap({ isActive: showCloseDialog, - onEscape: () => setShowCloseDialog(false), + onEscape: () => dismissCloseDialog(), restoreFocus: true, }); @@ -2100,6 +2104,29 @@ export function EmailComposer({ handleSend({ delayedUntil: new Date(scheduleValue).toISOString() }); }; + // A host can hand a follow-up to requestCloseRef - "close this session, then + // do that". It runs only once the composer really closes, never when the + // guard dialog is cancelled, and never before onClose has had its say. + const afterCloseRef = useRef<(() => void) | null>(null); + + const emitClose = () => { + onClose?.(); + const afterClose = afterCloseRef.current; + afterCloseRef.current = null; + // A save that the server refused resets explicitCloseRef so the unmount + // stash hands the text back to the host's continue-draft slot (#702). The + // draft is still alive in that case, so a follow-up that would replace it + // must not run - the rescue outranks the request that triggered it. + if (explicitCloseRef.current) { + afterClose?.(); + } + }; + + const dismissCloseDialog = () => { + afterCloseRef.current = null; + setShowCloseDialog(false); + }; + const cleanClose = () => { sendCancelledRef.current = true; explicitCloseRef.current = true; @@ -2107,7 +2134,7 @@ export function EmailComposer({ clearTimeout(saveTimeoutRef.current); } stateRef.current = { to: '', cc: '', bcc: '', subject: '', body: '', showCc: false, showBcc: false, selectedIdentityId: null, subAddressTag: '', draftId: null, fromOverrideEnabled: false, fromOverrideEmail: '', fromOverrideName: '' }; - onClose?.(); + emitClose(); }; const handleSaveDraftAndClose = async () => { @@ -2134,7 +2161,7 @@ export function EmailComposer({ stateRef.current = { to: '', cc: '', bcc: '', subject: '', body: '', showCc: false, showBcc: false, selectedIdentityId: null, subAddressTag: '', draftId: null, fromOverrideEnabled: false, fromOverrideEmail: '', fromOverrideName: '' }; } } finally { - onClose?.(); + emitClose(); } }; @@ -2149,10 +2176,11 @@ export function EmailComposer({ onDiscardDraft(draftId); } stateRef.current = { to: '', cc: '', bcc: '', subject: '', body: '', showCc: false, showBcc: false, selectedIdentityId: null, subAddressTag: '', draftId: null, fromOverrideEnabled: false, fromOverrideEmail: '', fromOverrideName: '' }; - onClose?.(); + emitClose(); }; - const handleClose = () => { + const handleClose = (afterClose?: () => void) => { + afterCloseRef.current = afterClose ?? null; if (isDirtyRef.current) { setShowCloseDialog(true); } else { @@ -2244,7 +2272,7 @@ export function EmailComposer({ {/* Header - mobile: clean bar with close/send, desktop: title bar */}
-
@@ -2798,7 +2826,7 @@ export function EmailComposer({