From 79fde240ac2a8e705af50096cdbb78e7a26a92a9 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Wed, 4 Nov 2026 11:22:58 +0100 Subject: [PATCH] fix: stop the app crashing when Compose is pressed while a reply is docked --- .../mail-app-composer-replace.test.tsx | 174 ++++++++++++++++++ components/mail/mail-app.tsx | 13 +- 2 files changed, 184 insertions(+), 3 deletions(-) create mode 100644 components/mail/__tests__/mail-app-composer-replace.test.tsx diff --git a/components/mail/__tests__/mail-app-composer-replace.test.tsx b/components/mail/__tests__/mail-app-composer-replace.test.tsx new file mode 100644 index 000000000..f56066c13 --- /dev/null +++ b/components/mail/__tests__/mail-app-composer-replace.test.tsx @@ -0,0 +1,174 @@ +import React from 'react'; +import { act, fireEvent, render, screen, waitFor } from '@testing-library/react'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import type { Mailbox } from '@/lib/jmap/types'; +import { useAuthStore } from '@/stores/auth-store'; +import { useEmailStore } from '@/stores/email-store'; +import { useSettingsStore } from '@/stores/settings-store'; +import { usePluginStore } from '@/stores/plugin-store'; +import { useThemeStore } from '@/stores/theme-store'; +import { useToastStore } from '@/stores/toast-store'; +import { DEFAULT_SEARCH_FILTERS } from '@/lib/jmap/search-utils'; + +// One composer at a time (compose-p2-1, compose-p2-2). Another one opening +// while a composer is on screen closes that one the way its X does (save, +// "Draft saved"), then opens. It never crashes and never drops the text. + +const device = vi.hoisted(() => ({ current: { isMobile: false, isTablet: false, isDesktop: true } })); +// What the stub composer's close does: 'saves' closes and lets the next one +// open, 'fails' keeps the composer (a save the server refused). +const stub = vi.hoisted(() => ({ + close: 'none' as 'none' | 'saves' | 'fails', + closes: [] as string[], + mounts: [] as string[], +})); + +vi.mock('@/components/layout/mv-top-bar', () => ({ MvTopBar: () => null })); +vi.mock('@/components/layout/sidebar', () => ({ + Sidebar: ({ onCompose }: { onCompose: () => void }) => , +})); +// As the real composer: it hands its state (its own mode, untouched) to the +// host as it unmounts without an explicit close, and offers its close. +vi.mock('@/components/email/email-composer', async () => { + const { useEffect, useRef } = await import('react'); + return { + EmailComposer: ({ mode, onSaveState, onClose, requestCloseRef }: { + mode: string; + onSaveState: (data: Record) => void; + onClose: () => void; + requestCloseRef?: { current: unknown }; + }) => { + const closed = useRef(false); + useEffect(() => { + stub.mounts.push(mode); + // The old host moved the composer between dock and pane for ever + // (React error 185 in the browser); stop it here instead of hanging. + if (stub.mounts.length > 40) throw new Error('composer remount loop'); + if (requestCloseRef && stub.close !== 'none') { + requestCloseRef.current = (afterClose?: () => void) => { + stub.closes.push(mode); + if (stub.close === 'fails') return; + closed.current = true; + onClose(); + afterClose?.(); + }; + } + return () => { + if (requestCloseRef) requestCloseRef.current = null; + if (closed.current) return; + onSaveState({ to: '', cc: '', bcc: '', subject: '', body: '', mode, draftId: null, unedited: true }); + }; + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []); + return
stub composer
; + }, + }; +}); +vi.mock('@/components/search/mv-search-form', () => ({ + MvSearchForm: ({ onSearch }: { onSearch: () => void }) => , +})); +vi.mock('@/components/email/email-viewer', () => ({ + EmailViewer: ({ onReply, onReplyAll, onForward }: { onReply: () => void; onReplyAll: () => void; onForward: () => void }) => ( +
+ message viewer + + + +
+ ), +})); +vi.mock('@/components/layout/navigation-rail', () => ({ NavigationRail: () => null })); +vi.mock('@/components/layout/calendar-event-notification-toaster', () => ({ CalendarEventNotificationToaster: () => null })); +vi.mock('@/components/layout/share-notification-toaster', () => ({ ShareNotificationToaster: () => null })); +vi.mock('@/components/layout/mobile-header', () => ({ MobileHeader: () => null })); +vi.mock('@/components/layout/mv-side-panel', () => ({ MvSidePanel: () => null })); +vi.mock('@/components/email/message-list-tabs', () => ({ useMessageListPanelProps: (id: string) => ({ id }), MessageListTabs: () => null })); +vi.mock('@/components/search/search-box', () => ({ SearchBox: () => null })); +vi.mock('@/components/tour/tour-provider', () => ({ useTour: () => ({ activeTour: null, registerStep: vi.fn() }) })); +vi.mock('@/components/plugins/plugin-slot', () => ({ PluginSlot: () => null })); +vi.mock('@/hooks/use-plugin-slot-offers', () => ({ usePluginSlotOffers: () => [] })); +vi.mock('@/hooks/use-config', () => ({ useConfig: () => ({ appName: 'Mail' }) })); +vi.mock('@/hooks/use-identity-sync', () => ({ useIdentitySync: () => {} })); +vi.mock('@/hooks/use-pro-multi-account-identities', () => ({ useProMultiAccountIdentities: () => [] })); +vi.mock('@/hooks/use-pro-multi-account-mailboxes', () => ({ useProMultiAccountMailboxes: () => {} })); +vi.mock('@/hooks/use-wopi-status', () => ({ useWopiStatus: () => null, canWopiOpen: () => false })); +vi.mock('@/hooks/use-browser-navigation', () => ({ useBrowserNavigation: () => {} })); +vi.mock('@/hooks/use-media-query', () => ({ + useDeviceDetection: () => device.current, + useMediaQuery: () => false, + useIsDesktop: () => true, +})); +vi.mock('@/lib/push-bindings', () => ({ reconcilePushBindings: () => {}, releasePushBindings: () => {} })); +vi.mock('@/components/email/email-list', () => ({ + EmailList: () => null, +})); + +const { MailApp } = await import('../mail-app'); + +const inbox = { id: 'inbox', name: 'Inbox', role: 'inbox', totalEmails: 0, unreadEmails: 0 } as Mailbox; +const MESSAGE = { + id: 'm1', threadId: 't1', mailboxIds: { inbox: true }, keywords: { $seen: true }, + from: [{ name: 'Ann', email: 'ann@example.org' }], to: [{ email: 'a@example.org' }], + subject: 'Plans', receivedAt: '2026-11-01T10:00:00Z', preview: 'Hi', size: 10, + hasAttachment: false, textBody: [], htmlBody: [], bodyValues: {}, +} as never; + +beforeEach(() => { + device.current = { isMobile: false, isTablet: false, isDesktop: true }; + stub.close = 'none'; + stub.closes = []; + stub.mounts = []; + vi.stubGlobal('fetch', vi.fn().mockRejectedValue(new Error('Unexpected network request'))); + vi.stubGlobal('ResizeObserver', class { observe() {} unobserve() {} disconnect() {} }); + usePluginStore.setState({ initializePlugins: vi.fn().mockResolvedValue(undefined) }); + useThemeStore.setState({ syncServerThemes: vi.fn().mockResolvedValue(undefined) }); + const client = { + getAccountId: () => 'acc', getUsername: () => 'a@example.org', + getSieveAccountId: () => null, hasDelayedSend: () => false, getSession: () => null, + getEmail: vi.fn().mockResolvedValue(null), + }; + useAuthStore.setState({ + isAuthenticated: true, isLoading: false, client: client as never, activeAccountId: 'login-a', identities: [], + getClientForAccount: () => client as never, + getAllConnectedClients: () => new Map([['login-a', client]]) as never, + }); + useSettingsStore.setState({ enableUnifiedMailbox: false, interfaceLayout: 'default', mailLayout: 'split' } as never); + useEmailStore.setState({ + emails: [MESSAGE], selectedEmail: MESSAGE, selectedMailbox: 'inbox', selectedKeyword: null, mailboxes: [inbox], + viewingAccountId: null, accountMailboxes: {}, isUnifiedView: false, unifiedRole: null, + isScheduledView: false, isLoadingEmail: false, isLoading: false, searchQuery: '', + searchFilters: { ...DEFAULT_SEARCH_FILTERS }, searchMailboxId: '', isAdvancedSearchOpen: false, + selectedEmailKeys: new Set(), unreadSection: null, + fetchMailboxes: vi.fn().mockResolvedValue(undefined), fetchEmails: vi.fn().mockResolvedValue(undefined), + fetchQuota: vi.fn().mockResolvedValue(undefined), fetchTagCounts: vi.fn().mockResolvedValue(undefined), + refreshScheduledMetadata: vi.fn().mockResolvedValue(undefined), + } as never); + useToastStore.getState().clearToasts(); +}); +afterEach(() => { + vi.restoreAllMocks(); + vi.unstubAllGlobals(); + useToastStore.getState().clearToasts(); +}); + +const composerMode = () => screen.getByTestId('stub-composer').getAttribute('data-mode'); + +describe('Compose while a reply is docked (compose-p2-1)', () => { + for (const action of ['stub reply', 'stub forward']) { + it(`opens a new message in the pane after ${action}`, async () => { + render(); + fireEvent.click(await screen.findByText(action)); + const reply = await screen.findByTestId('stub-composer'); + expect(reply.closest('[data-reply-dock]')).not.toBeNull(); + + fireEvent.click(screen.getByText('stub compose')); + await waitFor(() => expect(composerMode()).toBe('compose')); + // The new message takes the pane, not the dock, and stays put. + expect(screen.getByTestId('stub-composer').closest('[data-reply-dock]')).toBeNull(); + const mounted = stub.mounts.length; + await act(async () => { await new Promise((r) => setTimeout(r, 20)); }); + expect(stub.mounts.length).toBe(mounted); + expect(composerMode()).toBe('compose'); + }); + } +}); diff --git a/components/mail/mail-app.tsx b/components/mail/mail-app.tsx index bdc888cfc..1ea598b03 100644 --- a/components/mail/mail-app.tsx +++ b/components/mail/mail-app.tsx @@ -304,7 +304,7 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { composerSessionRef.current = composerSessionId; const composerHostRef = useRef('pane'); // The compose session docked under its message (dockedReply). - const dockedSessionRef = useRef(null); + const [dockedSession, setDockedSession] = useState(null); const showComposerRef = useRef(showComposer); showComposerRef.current = showComposer; // Fields a plugin filled in through onBeforeCompose for the composer about @@ -4923,8 +4923,9 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { // anew when the message goes away (another folder opened meanwhile). const dockCandidate = !mvShell && !isMobile && showComposer && !isEmbedded && !floatingComposer && (pendingDraft?.mode ?? composerMode) !== 'compose'; - if (dockCandidate && selectedEmail && !selectedEmail.keywords?.$draft) dockedSessionRef.current = composerSessionId; - const dockedReply = dockCandidate && dockedSessionRef.current === composerSessionId; + const dockOnOpen = dockCandidate && Boolean(selectedEmail) && !selectedEmail?.keywords?.$draft; + if (dockOnOpen && dockedSession !== composerSessionId) setDockedSession(composerSessionId); + const dockedReply = dockCandidate && (dockOnOpen || dockedSession === composerSessionId); const composerHost = floatingComposer ? 'window' : mvInlineReply ? 'inline' : dockedReply ? 'dock' : 'pane'; composerHostRef.current = composerHost; const hasViewerContent = (showComposer && !floatingComposer) || Boolean(conversationThread) @@ -5275,6 +5276,12 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { sessionKey={`inline-${composerSessionId}`} sourceEmailId={answeredEmailId} onSaveState={(data) => { + // A composer whose session another one has replaced: its state + // belongs to no composer on screen. Taken as the new session's, + // it turned a new message into the old reply and moved it between + // the dock and the pane for ever (React error 185, compose-p2-1). + // Whatever opens a composer closes the old one first, which saves it. + if (composerSessionRef.current !== composerSessionId) return; setComposerPrefill(null); if (suppressComposerStateSaveSessionRef.current === composerSessionId) { suppressComposerStateSaveSessionRef.current = null;