From a73ac09035b1ae155ea4c7ce372155c53bf8402d Mon Sep 17 00:00:00 2001 From: VasilisNtovolos Date: Mon, 19 Oct 2026 15:43:38 +0300 Subject: [PATCH] fix: route conversation blobs by originating message --- components/email/email-viewer.tsx | 18 +- components/email/thread-conversation-view.tsx | 52 +++- .../mail/__tests__/blob-routing.test.tsx | 270 +++++++++++++++++- components/mail/mail-app.tsx | 137 +++++---- stores/email-store.ts | 8 +- 5 files changed, 404 insertions(+), 81 deletions(-) diff --git a/components/email/email-viewer.tsx b/components/email/email-viewer.tsx index d179d93eb..46ba234d9 100644 --- a/components/email/email-viewer.tsx +++ b/components/email/email-viewer.tsx @@ -107,7 +107,7 @@ import { useDeviceDetection } from "@/hooks/use-media-query"; import { useIsPaneScoped } from "@/hooks/use-pane-context"; import { useAuthStore } from "@/stores/auth-store"; import { useAccountStore } from "@/stores/account-store"; -import { useEmailStore } from "@/stores/email-store"; +import { useEmailStore, resolveEmailBlobContext, type EmailBlobContext } from "@/stores/email-store"; import { useThemeStore } from "@/stores/theme-store"; import { EmailIdentityBadge } from "./email-identity-badge"; import { UnsubscribeBanner } from "./unsubscribe-banner"; @@ -129,7 +129,7 @@ import { parseTnef, isTnefAttachment } from "@/lib/tnef"; import { debug } from "@/lib/debug"; import { findVerificationCode, verificationCodeBodyText } from "@/lib/verification-code"; import { VerificationCodeChip } from "./verification-code-chip"; -import { ConversationEmailBody, useConversationExternalContent, useMarkReadWhenShown } from "./thread-conversation-view"; +import { ConversationEmailBody, type ConversationAttachmentHandler, useConversationExternalContent, useMarkReadWhenShown } from "./thread-conversation-view"; import type { TnefAttachment } from "@/lib/tnef"; import { PluginSlot } from "@/components/plugins/plugin-slot"; import { PluginActions } from "@/components/plugins/plugin-actions"; @@ -154,7 +154,7 @@ const SUB_MENU_ITEM_SELECTOR = '[role="menuitem"],[role="menuitemcheckbox"],[rol interface EmailViewerProps { email: Email | null; /** Captured message routing supplied by the standard mail UI. */ - blobSource?: { client: IJMAPClient | null; accountId?: string }; + blobSource?: EmailBlobContext; isLoading?: boolean; onReply?: (draftText?: string) => void; onReplyAll?: () => void; @@ -165,7 +165,7 @@ interface EmailViewerProps { onToggleStar?: () => void; onMarkAsRead?: (emailId: string, read: boolean) => void; onSetTag?: (emailId: string, tagId: string | null) => void; - onDownloadAttachment?: (blobId: string, name: string, type?: string, forceDownload?: boolean) => void; + onDownloadAttachment?: (blobId: string, name: string, type?: string, forceDownload?: boolean, source?: EmailBlobContext) => void; onQuickReply?: (body: string) => Promise; onMarkAsSpam?: () => void; onUndoSpam?: () => void; @@ -791,6 +791,7 @@ export function MvExpandedConversationCard({ onMarkRead, onReply, onDownloadAttachment, + blobSource, compact, }: { email: Email; @@ -800,7 +801,8 @@ export function MvExpandedConversationCard({ onLoadBody?: (email: Email) => void; onMarkRead?: (email: Email) => void; onReply?: (email: Email) => void; - onDownloadAttachment?: (blobId: string, name: string, type?: string) => void; + onDownloadAttachment?: ConversationAttachmentHandler; + blobSource?: EmailBlobContext; }) { const t = useTranslations('email_viewer'); const externalContent = useConversationExternalContent(); @@ -883,6 +885,7 @@ export function MvExpandedConversationCard({ onAllowExternal={() => externalContent.allow(email.id)} onTrustSender={externalContent.trustSender(email)} onDownloadAttachment={onDownloadAttachment} + blobSource={blobSource} className={cn("pb-4", compact ? "px-4" : "ps-[80px] pe-6")} bodyStyle={{ paddingBlock: '0.25rem 0.5rem' }} /> @@ -1215,7 +1218,10 @@ export function EmailViewer({ onLoadBody={onLoadConversationEmail} onMarkRead={onMarkConversationEmailRead} onReply={onReplyConversationEmail} - onDownloadAttachment={onDownloadAttachment} + blobSource={target.sourceClientAccountId || target.sourceAccountId + ? resolveEmailBlobContext(target, client) + : blobSource ?? resolveEmailBlobContext(target, client)} + onDownloadAttachment={(blobId, name, type, source) => onDownloadAttachment?.(blobId, name, type, false, source)} compact={mvMobileViewer} /> ); diff --git a/components/email/thread-conversation-view.tsx b/components/email/thread-conversation-view.tsx index 73ec423c1..44c5c590f 100644 --- a/components/email/thread-conversation-view.tsx +++ b/components/email/thread-conversation-view.tsx @@ -39,6 +39,7 @@ import { import { useTranslations } from "next-intl"; import { useSettingsStore } from "@/stores/settings-store"; import { useContactStore } from "@/stores/contact-store"; +import { useEmailStore, resolveEmailBlobContext, type EmailBlobContext } from "@/stores/email-store"; import { useAuthStore } from "@/stores/auth-store"; import { isFilePreviewable, toInertBlob } from "@/lib/file-preview"; import { useWopiStatus, canWopiOpen } from "@/hooks/use-wopi-status"; @@ -51,6 +52,10 @@ import { usePluginRenderedBody } from "@/hooks/use-plugin-rendered-body"; import { emailToReadView } from "@/lib/plugin-projection"; import { AttachmentPluginMenu, ReaderAboveQuoteSlot } from "./reader-plugin-surfaces"; +export type ConversationAttachmentHandler = ( + blobId: string, name: string, type: string | undefined, source: EmailBlobContext, +) => void; + interface ThreadConversationViewProps { thread: ThreadGroup; emails: Email[]; @@ -59,7 +64,9 @@ interface ThreadConversationViewProps { onReply?: (email: Email) => void; onReplyAll?: (email: Email) => void; onForward?: (email: Email) => void; - onDownloadAttachment?: (blobId: string, name: string, type?: string) => void; + onDownloadAttachment?: ConversationAttachmentHandler; + /** Routing captured when this conversation was opened. */ + blobSource?: EmailBlobContext; onMarkAsRead?: (emailId: string, read: boolean) => void; /** Role of the folder the conversation was opened from, for plugin actions limited to some folders. */ mailboxRole?: string | null; @@ -176,6 +183,7 @@ export function ThreadConversationView({ onReplyAll, onForward, onDownloadAttachment, + blobSource, onMarkAsRead, mailboxRole, }: ThreadConversationViewProps) { @@ -274,6 +282,7 @@ export function ThreadConversationView({ onReplyAll={onReplyAll ? () => onReplyAll(email) : undefined} onForward={onForward ? () => onForward(email) : undefined} onDownloadAttachment={onDownloadAttachment} + blobSource={blobSource} onMarkAsRead={onMarkAsRead} mailboxRole={mailboxRole} /> @@ -296,7 +305,9 @@ interface EmailCardProps { onReply?: () => void; onReplyAll?: () => void; onForward?: () => void; - onDownloadAttachment?: (blobId: string, name: string, type?: string) => void; + onDownloadAttachment?: ConversationAttachmentHandler; + /** Routing captured when this conversation was opened. */ + blobSource?: EmailBlobContext; onMarkAsRead?: (emailId: string, read: boolean) => void; mailboxRole?: string | null; } @@ -330,6 +341,7 @@ function EmailCard({ onReplyAll, onForward, onDownloadAttachment, + blobSource, onMarkAsRead, mailboxRole, }: EmailCardProps) { @@ -412,6 +424,7 @@ function EmailCard({ onAllowExternal={onAllowExternal} onTrustSender={onTrustSender} onDownloadAttachment={onDownloadAttachment} + blobSource={blobSource} /> @@ -479,7 +492,9 @@ interface ConversationEmailBodyProps { allowExternal: boolean; onAllowExternal: () => void; onTrustSender?: () => void; - onDownloadAttachment?: (blobId: string, name: string, type?: string) => void; + onDownloadAttachment?: ConversationAttachmentHandler; + /** Routing captured when this conversation was opened. */ + blobSource?: EmailBlobContext; /** Padding around the body; the phone card's density padding by default. */ bodyStyle?: React.CSSProperties; className?: string; @@ -496,6 +511,7 @@ export function ConversationEmailBody({ onAllowExternal, onTrustSender, onDownloadAttachment, + blobSource, bodyStyle, className, }: ConversationEmailBodyProps) { @@ -515,8 +531,18 @@ export function ConversationEmailBody({ [showVerificationCodes, email.id, email.subject, email.bodyValues], ); const [hasBlockedContent, setHasBlockedContent] = useState(false); - const [cidBlobUrls, setCidBlobUrls] = useState>({}); - const { client } = useAuthStore(useShallow((state) => ({ client: state.client }))); + const { client } = useAuthStore(useShallow((state) => ({ + client: state.client, revision: state.connectedAccountsRevision, + }))); + const fallbackSource = useEmailStore(useShallow(() => resolveEmailBlobContext(email, client))); + const source = blobSource ?? fallbackSource; + const { client: blobClient, accountId } = source; + // Local CID URLs belong to a particular client/owner/message/part list. + // Hide the previous set during render, before the new fetch's effect runs. + const cidRequest = useMemo(() => ({ blobClient, accountId, emailId: email.id, attachments: email.attachments }), + [blobClient, accountId, email.id, email.attachments]); + const [cidResult, setCidResult] = useState<{ request: typeof cidRequest; urls: Record } | null>(null); + const cidBlobUrls = useMemo(() => cidResult?.request === cidRequest ? cidResult.urls : {}, [cidResult, cidRequest]); // A crypto plugin's decrypted body (PGP, S/MIME) replaces the message's own. const pluginBody = usePluginRenderedBody(email); // reader-above-quote: the body the quote was collapsed in, once it is on screen. @@ -530,14 +556,14 @@ export function ConversationEmailBody({ // Fetch inline CID images with authentication to prevent browser auth dialogs useEffect(() => { - if (!client || !email?.attachments) { - setCidBlobUrls({}); + if (!blobClient || !cidRequest.attachments) { + setCidResult(null); return; } - const cidAttachments = email.attachments.filter(att => att.cid && att.blobId); + const cidAttachments = cidRequest.attachments.filter(att => att.cid && att.blobId); if (cidAttachments.length === 0) { - setCidBlobUrls({}); + setCidResult(null); return; } @@ -551,7 +577,7 @@ export function ConversationEmailBody({ try { // Re-type sender-declared script-bearing parts before they become a // blob: URL in our origin (GHSA-xvjh-v9c6-qcvc). - const objectUrl = URL.createObjectURL(toInertBlob(await client!.fetchBlob(att.blobId, att.name || 'inline', att.type))); + const objectUrl = URL.createObjectURL(toInertBlob(await blobClient!.fetchBlob(att.blobId, att.name || 'inline', att.type, accountId))); if (!cancelled) { urls[cidValue] = objectUrl; objectUrls.push(objectUrl); @@ -563,7 +589,7 @@ export function ConversationEmailBody({ } })); if (!cancelled) { - setCidBlobUrls(urls); + setCidResult({ request: cidRequest, urls }); } } @@ -573,7 +599,7 @@ export function ConversationEmailBody({ cancelled = true; objectUrls.forEach(url => URL.revokeObjectURL(url)); }; - }, [client, email?.id, email?.attachments]); + }, [cidRequest, blobClient, accountId]); // Sanitize and prepare email HTML content const emailContent = useMemo(() => { @@ -867,7 +893,7 @@ export function ConversationEmailBody({ - -)} })); +vi.mock('@/components/email/email-list', async () => { + const { ThreadListItem } = await import('@/components/email/thread-list-item'); + const { groupEmailsByThread } = await import('@/lib/thread-utils'); + return { EmailList: ({ emails, onEmailSelect, onOpenAttachment, onOpenConversation }: { + emails: Email[]; + onOpenConversation: (thread: ThreadGroup) => void; + onEmailSelect: (email: Email) => void; + onOpenAttachment: (email: Email, attachment: NonNullable[number]) => void; + }) => device.mobile ? ( +
{groupEmailsByThread(emails).map(thread => ( + {}} onEmailSelect={onEmailSelect} onOpenConversation={onOpenConversation} /> + ))}
+ ) : ( +
{emails.map((email, i) =>
+ + +
)}
+ ) }; +}); vi.mock('@/components/files/file-preview-modal', () => ({ FilePreviewModal: ({ onDownload, getFileContent }: { onDownload: () => void; getFileContent: () => Promise; }) =>
})); const { MailApp } = await import('../mail-app'); +const { ConversationEmailBody } = await import('@/components/email/thread-conversation-view'); +const { EmailViewer } = await import('@/components/email/email-viewer'); const sharedMailbox = { id: 'nrichmond:inbox', originalId: 'inbox', name: 'Shared', isShared: true, accountId: 'nrichmond' } as Mailbox; const primaryMailbox = { id: 'inbox', name: 'Inbox', role: 'inbox' } as Mailbox; @@ -90,6 +111,8 @@ let other: ReturnType; const asClient = (c: ReturnType) => c as unknown as IJMAPClient; beforeEach(() => { + device.mobile = false; + useUIStore.setState({ isMobile: false }); vi.stubGlobal('fetch', vi.fn().mockRejectedValue(new Error('Unexpected network request'))); usePluginStore.setState({ initializePlugins: vi.fn().mockResolvedValue(undefined) }); useThemeStore.setState({ syncServerThemes: vi.fn().mockResolvedValue(undefined) }); @@ -104,7 +127,7 @@ beforeEach(() => { getClientForAccount: (id) => (id === 'login-primary' ? asClient(primary) : id === 'login-other' ? asClient(other) : undefined) as never, getAllConnectedClients: () => new Map([['login-primary', asClient(primary)], ['login-other', asClient(other)]]) as never, }); - useSettingsStore.setState({ enableUnifiedMailbox: false, mailAttachmentAction: 'download', mailLayout: 'split', attachmentImagePreviewsEnabled: false, postExportAction: 'none', attachmentDownloadTemplate: '{filename}', filenameLowercase: false } as never); + useSettingsStore.setState({ interfaceLayout: 'default', markAsReadDelay: -1, enableUnifiedMailbox: false, mailAttachmentAction: 'download', mailLayout: 'split', attachmentImagePreviewsEnabled: false, postExportAction: 'none', attachmentDownloadTemplate: '{filename}', filenameLowercase: false } as never); useEmailStore.setState({ emails: [message()], selectedEmail: message(), selectedMailbox: sharedMailbox.id, mailboxes: [primaryMailbox, sharedMailbox], viewingAccountId: null, accountMailboxes: {}, @@ -352,3 +375,230 @@ describe('standard mail UI blob routing', () => { await waitFor(() => expect(primary.fetchBlob).toHaveBeenCalledWith('same-blob', 'inline.png', 'image/png', 'nrichmond')); }); }); + + +// Exercise real thread-row clicks, MailApp conversation loading, the card body, +// and JMAP URL/authentication. Only unrelated shell UI and responses are mocked. +async function conversationClients() { + const a = await connectDownloadClient('https://primary.example.com', 'primary-login', 'postal12345'); + const b = await connectDownloadClient('https://secondary.example.com', 'secondary-login', 'secondary-primary'); + useAuthStore.setState({ client: a, + getClientForAccount: id => id === 'login-primary' ? a : id === 'login-other' ? b : undefined, + getAllConnectedClients: () => new Map([['login-primary', a], ['login-other', b]]), + }); + const fetchMock = vi.mocked(fetch); + fetchMock.mockClear(); + fetchMock.mockImplementation(async () => new Response('blob bytes', { + status: 200, headers: { 'Content-Type': 'image/png' }, + })); + vi.spyOn(HTMLAnchorElement.prototype, 'click').mockImplementation(() => {}); + return { a, b, fetchMock }; +} +const cardAttachment = { partId: '3', blobId: 'same-blob', name: 'card.png', type: 'image/png', size: 50 }; +function conversationMessage(overrides: Partial = {}) { + return message({ id: 'card', subject: 'Routing conversation', attachments: [cardAttachment], ...overrides }); +} +function inlineMessage(overrides: Partial = {}) { + return conversationMessage({ + htmlBody: [{ partId: '1', blobId: 'html-blob', type: 'text/html', size: 30 }], textBody: [], + bodyValues: { '1': { value: '', isTruncated: false, isEncodingProblem: false } }, + attachments: [{ ...cardAttachment, cid: 'test-image' }], ...overrides, + }); +} +function expectBlobRequest(url: string, login: string, index = 0) { + const calls = vi.mocked(fetch).mock.calls; + expect(String(calls[index][0])).toBe(url); + expect(calls[index][1]?.headers).toEqual(expect.objectContaining({ + Authorization: `Basic ${btoa(`${login}:test-password`)}`, + })); +} +async function openPhoneConversation(ref: Email, threadClient: JMAPClient, selected: Email | null = null, unified = true) { + device.mobile = true; + useUIStore.setState({ isMobile: true }); + const older = { ...ref, id: 'older', receivedAt: '2025-12-31T00:00:00Z', attachments: [] }; + // Email/get/Thread/get results have no UI source stamps. + const raw = (email: Email) => ({ ...email, sourceClientAccountId: undefined, sourceAccountId: undefined }); + const threadFetch = vi.spyOn(threadClient, 'getThreadEmails').mockResolvedValue([raw(ref), raw(older)]); + useEmailStore.setState({ emails: [ref, older], selectedEmail: selected, + selectedMailbox: unified ? '__unified_inbox__' : sharedMailbox.id, + isUnifiedView: unified, unifiedRole: unified ? 'inbox' : null, + }); + render(); + fireEvent.click(await screen.findByText('Routing conversation')); + await screen.findByRole('button', { name: 'email_viewer.back_to_list' }); + await waitFor(() => expect(threadFetch).toHaveBeenCalled()); + await screen.findByText('threads.messages_other'); + return threadFetch; +} + +describe('conversation blob routing', () => { + it.each([ + { source: 'direct shared folder', login: 'login-primary', owner: 'nrichmond', unified: false }, + { source: 'secondary login and shared owner', login: 'login-other', owner: 'nrichmond', unified: true }, + { source: 'primary account', login: 'login-primary', owner: 'postal12345', unified: true }, + ])('routes phone CID images for $source', async ({ login, owner, unified }) => { + const { a, b, fetchMock } = await conversationClients(); + const ref = inlineMessage(unified ? { sourceClientAccountId: login, sourceAccountId: owner } : {}); + const threadFetch = await openPhoneConversation(ref, login === 'login-other' ? b : a, null, unified); + expect(threadFetch).toHaveBeenCalledWith('thread', unified ? owner : 'nrichmond'); + await waitFor(() => expect(fetchMock).toHaveBeenCalledTimes(1)); + expectBlobRequest(`https://${login === 'login-other' ? 'secondary' : 'primary'}.example.com/download/${owner}/same-blob/card.png?accept=image%2Fpng`, login === 'login-other' ? 'secondary-login' : 'primary-login'); + }); + + it('routes a direct shared-folder conversation through the browsing login', async () => { + const { b, fetchMock } = await conversationClients(); + useEmailStore.setState({ viewingAccountId: 'login-other', accountMailboxes: { 'login-other': [sharedMailbox] } }); + await openPhoneConversation(inlineMessage(), b, null, false); + await waitFor(() => expect(fetchMock).toHaveBeenCalledTimes(1)); + expectBlobRequest('https://secondary.example.com/download/nrichmond/same-blob/card.png?accept=image%2Fpng', 'secondary-login'); + }); + + it.each([ + { selection: 'null', login: 'login-other', owner: 'nrichmond' }, + { selection: 'unrelated', login: 'login-other', owner: 'nrichmond' }, + { selection: 'null', login: 'login-primary', owner: 'postal12345' }, + ])('downloads the phone card on $login with $selection selectedEmail', async ({ selection, login, owner }) => { + const { a, b, fetchMock } = await conversationClients(); + const selected = selection === 'null' ? null : message({ id: 'unrelated', subject: 'Other message', sourceClientAccountId: 'login-primary', sourceAccountId: 'unrelated-owner' }); + await openPhoneConversation(conversationMessage({ sourceClientAccountId: login, sourceAccountId: owner }), login === 'login-other' ? b : a, selected); + fireEvent.click(await screen.findByRole('button', { name: /card.png/ })); + await waitFor(() => expect(fetchMock).toHaveBeenCalledTimes(1)); + expectBlobRequest(`https://${login === 'login-other' ? 'secondary' : 'primary'}.example.com/download/${owner}/same-blob/card.png?accept=image%2Fpng`, login === 'login-other' ? 'secondary-login' : 'primary-login'); + }); + + it.each([false, true])('retains phone preview routing after navigation (direct shared=%s)', async direct => { + const { a, b, fetchMock } = await conversationClients(); + useSettingsStore.setState({ mailAttachmentAction: 'preview' }); + const ref = conversationMessage(direct ? {} : { sourceClientAccountId: 'login-other', sourceAccountId: 'nrichmond' }); + await openPhoneConversation(ref, direct ? a : b, null, !direct); + fireEvent.click(await screen.findByRole('button', { name: /card.png/ })); + await screen.findByText('Preview fetch'); + act(() => { + useEmailStore.setState({ selectedMailbox: 'inbox', selectedEmail: message({ sourceClientAccountId: 'login-primary', sourceAccountId: 'other-owner' }) }); + useAuthStore.setState({ client: b, activeAccountId: 'login-other' }); + }); + fireEvent.click(screen.getByText('Preview fetch')); + fireEvent.click(screen.getByText('Preview download')); + await waitFor(() => expect(fetchMock).toHaveBeenCalledTimes(2)); + for (const index of [0, 1]) expectBlobRequest(`https://${direct ? 'primary' : 'secondary'}.example.com/download/nrichmond/same-blob/card.png?accept=image%2Fpng`, direct ? 'primary-login' : 'secondary-login', index); + }); + + it.each([false, true])('routes Mountain View expanded sibling CID and attachment (mobile=%s)', async mobile => { + const { b, fetchMock } = await conversationClients(); + device.mobile = mobile; + useUIStore.setState({ isMobile: mobile }); + useSettingsStore.setState({ interfaceLayout: 'mountain-view' }); + const sibling = inlineMessage({ attachments: [...inlineMessage().attachments!, { ...cardAttachment, name: 'download.png', partId: '4' }], id: 'sibling', from: [{ name: 'Older sender', email: 'older@example.com' }], sourceClientAccountId: 'login-other', sourceAccountId: 'nrichmond' }); + const open = conversationMessage({ id: 'open', attachments: [], receivedAt: '2026-01-02T00:00:00Z', sourceClientAccountId: 'login-other', sourceAccountId: 'nrichmond' }); + const download = vi.fn(); + const { container } = render(); + fireEvent.click(await screen.findByText('Older sender')); + await waitFor(() => expect(fetchMock).toHaveBeenCalledTimes(1)); + expectBlobRequest('https://secondary.example.com/download/nrichmond/same-blob/card.png?accept=image%2Fpng', 'secondary-login'); + const card = container.querySelector('[data-mv-conversation-expanded]') as HTMLElement; + fireEvent.click(within(card).getByRole('button', { name: /download.png/ })); + expect(download).toHaveBeenCalledWith('same-blob', 'download.png', 'image/png', false, expect.objectContaining({ client: b, accountId: 'nrichmond' })); + }); + + it('downloads overlapping card blob ids from successive phone conversations', async () => { + const { a, b, fetchMock } = await conversationClients(); + device.mobile = true; + useUIStore.setState({ isMobile: true }); + const one = conversationMessage({ subject: 'First conversation', sourceClientAccountId: 'login-primary', sourceAccountId: 'owner-a' }); + const two = conversationMessage({ subject: 'Second conversation', sourceClientAccountId: 'login-other', sourceAccountId: 'owner-b' }); + const rows = [one, { ...one, id: 'older' }, two, { ...two, id: 'older' }]; + vi.spyOn(a, 'getThreadEmails').mockResolvedValue([one]); + vi.spyOn(b, 'getThreadEmails').mockResolvedValue([two]); + useEmailStore.setState({ emails: rows, selectedEmail: null, selectedMailbox: '__unified_inbox__', isUnifiedView: true, unifiedRole: 'inbox' }); + render(); + fireEvent.click(await screen.findByText('First conversation')); + fireEvent.click(await screen.findByRole('button', { name: /card.png/ })); + await waitFor(() => expect(fetchMock).toHaveBeenCalledTimes(1)); + fireEvent.click(screen.getByRole('button', { name: 'email_viewer.back_to_list' })); + fireEvent.click(screen.getByText('Second conversation')); + fireEvent.click(await screen.findByRole('button', { name: /card.png/ })); + await waitFor(() => expect(fetchMock).toHaveBeenCalledTimes(2)); + expectBlobRequest('https://primary.example.com/download/owner-a/same-blob/card.png?accept=image%2Fpng', 'primary-login', 0); + expectBlobRequest('https://secondary.example.com/download/owner-b/same-blob/card.png?accept=image%2Fpng', 'secondary-login', 1); + }); + + it.each(['owner', 'client'] as const)('scopes Mountain View pending loads when only the %s changes', async changed => { + const { a, b, fetchMock } = await conversationClients(); + useSettingsStore.setState({ interfaceLayout: 'mountain-view' }); + const nextOwner = changed === 'owner' ? 'next-owner' : 'nrichmond'; + const nextMailbox = { ...sharedMailbox, id: 'next-inbox', accountId: nextOwner }; + const open = conversationMessage({ id: 'open', attachments: [], receivedAt: '2026-01-02T00:00:00Z' }); + const sibling = (label: string) => conversationMessage({ + id: 'sibling', keywords: {}, from: [{ name: label, email: 'older@example.com' }], + }); + const getThreadA = vi.spyOn(a, 'getThread').mockResolvedValue({ id: 'thread', emailIds: ['sibling', 'open'] }); + const getThreadB = vi.spyOn(b, 'getThread').mockResolvedValue({ id: 'thread', emailIds: ['sibling', 'open'] }); + vi.spyOn(a, 'getSomeEmails').mockImplementation(async (_ids, owner) => [sibling(owner === nextOwner && changed === 'owner' ? 'Next sender' : 'First sender'), { ...open }]); + vi.spyOn(b, 'getSomeEmails').mockResolvedValue([sibling('Next sender'), { ...open }]); + useEmailStore.setState({ emails: [open], selectedEmail: open, selectedMailbox: sharedMailbox.id, + mailboxes: [sharedMailbox, nextMailbox], accountMailboxes: { 'login-other': [sharedMailbox, nextMailbox] }, + }); + const { container } = render(); + await screen.findByText('First sender'); + expect(fetchMock).not.toHaveBeenCalled(); + act(() => useEmailStore.setState({ + selectedMailbox: nextMailbox.id, selectedEmail: { ...open }, + viewingAccountId: changed === 'client' ? 'login-other' : null, + })); + await waitFor(() => expect(container.querySelector('[data-mv-conversation-expanded]')?.textContent).toContain('Next sender')); + const card = container.querySelector('[data-mv-conversation-expanded]') as HTMLElement; + fireEvent.click(within(card).getByRole('button', { name: /card.png/ })); + await waitFor(() => expect(fetchMock).toHaveBeenCalledTimes(1)); + expect(changed === 'client' ? getThreadB : getThreadA).toHaveBeenLastCalledWith('thread', nextOwner); + expectBlobRequest(`https://${changed === 'client' ? 'secondary' : 'primary'}.example.com/download/${nextOwner}/same-blob/card.png?accept=image%2Fpng`, changed === 'client' ? 'secondary-login' : 'primary-login'); + }); + + it('does not fetch a disconnected stamped source through another login', async () => { + const { fetchMock } = await conversationClients(); + render( {}} />); + await act(async () => {}); + expect(fetchMock).not.toHaveBeenCalled(); + }); + + it('isolates equal email/blob/CID ids across clients and owners and discards late CID results', async () => { + await conversationClients(); + let finishFirst!: (blob: Blob) => void; + const firstClient = asClient(primary); + const secondClient = asClient(other); + const first = vi.spyOn(firstClient, 'fetchBlob').mockImplementation(() => new Promise(resolve => { finishFirst = resolve; })); + const second = vi.spyOn(secondClient, 'fetchBlob').mockResolvedValue(new Blob(['second'], { type: 'image/png' })); + useAuthStore.setState({ client: firstClient as never, + getClientForAccount: id => (id === 'login-primary' ? firstClient : secondClient) as never, + }); + const attachments = inlineMessage().attachments; + const one = inlineMessage({ attachments, sourceClientAccountId: 'login-primary', sourceAccountId: 'owner-a' }); + const two = { ...one, sourceClientAccountId: 'login-other', sourceAccountId: 'owner-b' }; + vi.mocked(URL.createObjectURL).mockReturnValueOnce('blob:second').mockReturnValueOnce('blob:late'); + const body = (email: Email) => {}} />; + const { container, rerender, unmount } = render(body(one)); + await waitFor(() => expect(first).toHaveBeenCalled()); + rerender(body(two)); + await waitFor(() => expect(second).toHaveBeenCalledWith('same-blob', 'card.png', 'image/png', 'owner-b')); + await waitFor(() => expect(container.querySelector('iframe')?.getAttribute('srcdoc')).toContain('blob:second')); + await act(async () => finishFirst(new Blob(['late'], { type: 'image/png' }))); + expect(container.querySelector('iframe')?.getAttribute('srcdoc')).toContain('blob:second'); + expect(URL.revokeObjectURL).toHaveBeenCalledWith('blob:late'); + unmount(); + expect(URL.revokeObjectURL).toHaveBeenCalledWith('blob:second'); + }); + + it('clears resolved CID URLs immediately when only the owning account changes', async () => { + const { a, fetchMock } = await conversationClients(); + vi.mocked(URL.createObjectURL).mockReturnValue('blob:owner-a'); + const first = inlineMessage({ sourceClientAccountId: 'login-primary', sourceAccountId: 'owner-a' }); + const body = (email: Email) => {}} />; + const { container, rerender } = render(body(first)); + await waitFor(() => expect(container.querySelector('iframe')?.getAttribute('srcdoc')).toContain('blob:owner-a')); + vi.spyOn(a, 'fetchBlob').mockImplementation(() => new Promise(() => {})); + rerender(body({ ...first, sourceAccountId: 'owner-b' })); + expect(container.querySelector('iframe')?.getAttribute('srcdoc')).not.toContain('blob:owner-a'); + expect(fetchMock).toHaveBeenCalledTimes(1); + expect(a.fetchBlob).toHaveBeenLastCalledWith('same-blob', 'card.png', 'image/png', 'owner-b'); + expect(URL.revokeObjectURL).toHaveBeenCalledWith('blob:owner-a'); + }); +}); diff --git a/components/mail/mail-app.tsx b/components/mail/mail-app.tsx index 0ac5dc1a7..d7210505a 100644 --- a/components/mail/mail-app.tsx +++ b/components/mail/mail-app.tsx @@ -35,7 +35,7 @@ import type { UnifiedAccountClient } from "@/lib/unified-mailbox"; import { connectedAccountsGrew } from "@/lib/unified-mailbox"; import { messagePriorityFromHeaders, type MessagePriority } from "@/lib/message-priority"; import { KeyboardShortcutsModal } from "@/components/keyboard-shortcuts-modal"; -import { useEmailStore, pendingInitialData, buildUnifiedAccountClients, invalidateUnifiedMailboxes, captureViewToken, ArchiveMailboxNotFoundError, ensureArchiveMailbox, resolveSelectedEmails, resolveUnstampedEmailAccountId, resolveEmailBlobContext, emptyFolderMovesToTrash, bulkDeleteIsPermanent, shouldConfirmBulkDelete, removalFolder, canRemoveFromFolder } from "@/stores/email-store"; +import { useEmailStore, pendingInitialData, buildUnifiedAccountClients, invalidateUnifiedMailboxes, captureViewToken, ArchiveMailboxNotFoundError, ensureArchiveMailbox, resolveSelectedEmails, resolveUnstampedEmailAccountId, resolveEmailBlobContext, type EmailBlobContext, emptyFolderMovesToTrash, bulkDeleteIsPermanent, shouldConfirmBulkDelete, removalFolder, canRemoveFromFolder } from "@/stores/email-store"; import { useRemoveFromFolder } from "@/hooks/use-remove-from-folder"; import { groupSearchScopeFolders, SEARCH_SCOPE_ALL_FOLDERS } from "@/lib/search-scope-folders"; import { toast } from "@/stores/toast-store"; @@ -233,6 +233,8 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { const [isResizing, setIsResizing] = useState(false); const dragStartWidth = useRef(0); // Mobile conversation view state + const [conversationBlobSource, setConversationBlobSource] = useState(); + const conversationOpenSeqRef = useRef(0); const [conversationThread, setConversationThread] = useState(null); // Mountain View writes replies in place under the conversation until the // user pops one out into the docked window. @@ -242,11 +244,17 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { }, [showComposer]); const handleMvPopOut = useCallback(() => setMvReplyPoppedOut(true), []); // Mountain View: the open message's whole conversation, light properties only. - const [mvConversation, setMvConversation] = useState(null); + const [mvConversationResult, setMvConversationResult] = useState<{ + emails: Email[]; source: EmailBlobContext; threadId: string; + } | null>(null); // The thread whose conversation is being fetched, so the viewer waits for it // instead of showing the message alone first. (#1223) const [mvConversationLoading, setMvConversationLoading] = useState(null); - const mvConversationLoadRef = useRef<{ key: string; startedAt: number; promise: Promise } | null>(null); + const mvConversationLoadRef = useRef<{ + client: NonNullable; accountId?: string; + threadId: string; emailId: string; startedAt: number; promise: Promise; + } | null>(null); + const mvConversationOpenSeqRef = useRef(0); const [conversationEmails, setConversationEmails] = useState([]); const [isLoadingConversation, setIsLoadingConversation] = useState(false); const [rateLimitSecondsLeft, setRateLimitSecondsLeft] = useState(null); @@ -768,6 +776,8 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { // to return to a thread, we silently skip - back navigation always works. if ((state.threadId ?? null) !== ctx.conversationThreadId) { if (state.threadId === null) { + ++conversationOpenSeqRef.current; + setConversationBlobSource(undefined); setConversationThread(null); setConversationEmails([]); } @@ -1784,9 +1794,14 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { // Conversation. One fetch serves both layouts: mobile opens the dedicated // conversation view, desktop opens the newest message of the thread (which // is what clicking a thread in the list does there). + const openSeq = ++conversationOpenSeqRef.current; + const source: EmailBlobContext = { + client: activeClient, clientAccountId: useAuthStore.getState().activeAccountId ?? undefined, + }; setLoadingEmail(true); try { const threadEmails = await activeClient.getThreadEmails(link.id); + if (openSeq !== conversationOpenSeqRef.current) return; const [group] = groupEmailsByThread(threadEmails); if (!group) { toast.error(t('deep_link.thread_not_found')); @@ -1794,6 +1809,7 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { } if (isMobile) { setConversationThread(group); + setConversationBlobSource(source); setConversationEmails(threadEmails); setActiveView('viewer'); } else { @@ -2833,6 +2849,8 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { if (!archived) return; if (conversationThread && conversationThread.threadKey === threadKeyFor(emailToArchive)) { + ++conversationOpenSeqRef.current; + setConversationBlobSource(undefined); setConversationThread(null); setConversationEmails([]); } @@ -3685,8 +3703,8 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { return load; }, []); - const handleDownloadAttachment = async (blobId: string, name: string, type?: string, forceDownload?: boolean) => { - const { blobClient, accountId, clientAccountId } = resolveBlobSource(selectedEmail); + const handleDownloadAttachment = async (blobId: string, name: string, type?: string, forceDownload?: boolean, source?: EmailBlobContext) => { + const { client: blobClient, accountId, clientAccountId } = source ?? resolveEmailBlobContext(selectedEmail, client); if (!blobClient) return; try { @@ -3706,6 +3724,9 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { } }; + const handleDownloadConversationAttachment = (blobId: string, name: string, type: string | undefined, source: EmailBlobContext) => + handleDownloadAttachment(blobId, name, type, false, source); + const handlePreviewAttachmentDownload = useCallback(async () => { const c = previewAttachment?.client; if (!c || !previewAttachment) return; @@ -3890,26 +3911,31 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { const mvConversationThreadId = mvConversationView && !cursorOnly ? (selectedEmail?.threadId ?? null) : null; const mvConversationRef = mvConversationView && !cursorOnly ? selectedEmail : null; const mvConversationRoute = (ref: Email, fallback: NonNullable) => { - const route = resolveThreadRoute({ isUnifiedView, ref, mailboxes: viewMailboxes, selectedMailbox }); - const threadClient = route.clientAccountId - ? (useAuthStore.getState().getClientForAccount(route.clientAccountId) ?? fallback) - : fallback; - return { threadClient, accountId: route.accountId }; + const source = resolveEmailBlobContext(ref, fallback); + return { threadClient: source.client, accountId: source.accountId }; }; + const mvBlobSource = resolveEmailBlobContext(mvConversationRef, client); + // Do not render the previous owner's cards while the replacement loads. + const mvConversation = mvConversationResult?.source.client === mvBlobSource.client + && mvConversationResult?.source.accountId === mvBlobSource.accountId + && mvConversationResult?.threadId === mvConversationThreadId + ? mvConversationResult.emails : null; // Fetches the conversation around `ref`, oldest first. Opening a message // starts this next to the message fetch, and the effect below picks up that // same request rather than sending a second one. const loadMvConversation = (ref: Email, fallback: NonNullable): Promise => { - const key = `${ref.sourceClientAccountId ?? ''}:${ref.sourceAccountId ?? ''}:${ref.threadId}:${ref.id}`; + const { threadClient, accountId } = mvConversationRoute(ref, fallback); + if (!threadClient) return Promise.resolve(null); const pending = mvConversationLoadRef.current; - if (pending?.key === key && Date.now() - pending.startedAt < 10_000) return pending.promise; - const route = mvConversationRoute(ref, fallback); - const threadClient = route.threadClient; + // Bare ids overlap across shared owners and across connected clients. + if (pending?.client === threadClient && pending.accountId === accountId + && pending.threadId === ref.threadId && pending.emailId === ref.id + && Date.now() - pending.startedAt < 10_000) return pending.promise; const promise = (async () => { - const thread = await threadClient.getThread(ref.threadId, route.accountId); + const thread = await threadClient.getThread(ref.threadId, accountId); const ids = thread?.emailIds ?? []; if (ids.length < 2) return null; - const emails = await threadClient.getSomeEmails(ids, route.accountId); + const emails = await threadClient.getSomeEmails(ids, accountId); for (const e of emails) { // Carry the open message's source stamps, so opening one of these // in a merged view reaches the right account. @@ -3922,37 +3948,45 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { .filter((e) => e.id === ref.id || !e.keywords?.$draft) .sort((a, b) => a.receivedAt.localeCompare(b.receivedAt)); })().catch(() => null); - mvConversationLoadRef.current = { key, startedAt: Date.now(), promise }; + mvConversationLoadRef.current = { client: threadClient, accountId, threadId: ref.threadId, emailId: ref.id, startedAt: Date.now(), promise }; return promise; }; useEffect(() => { + ++mvConversationOpenSeqRef.current; if (!mvConversationThreadId || !client || !mvConversationRef) { - setMvConversation(null); + setMvConversationResult(null); setMvConversationLoading(null); return; } let cancelled = false; + setMvConversationResult(null); setMvConversationLoading(mvConversationThreadId); void loadMvConversation(mvConversationRef, client).then((conversation) => { if (cancelled) return; - setMvConversation(conversation); + setMvConversationResult(conversation ? { + emails: conversation, source: mvBlobSource, threadId: mvConversationThreadId, + } : null); setMvConversationLoading(null); }); - return () => { cancelled = true; }; + return () => { cancelled = true; ++mvConversationOpenSeqRef.current; }; // eslint-disable-next-line react-hooks/exhaustive-deps -- keyed on the open message, not on every list refresh - }, [mvConversationThreadId, mvConversationRef?.id, client]); + }, [mvConversationThreadId, mvConversationRef?.id, mvBlobSource.client, mvBlobSource.accountId, client]); const patchMvConversation = (id: string, patch: (e: Email) => Email) => - setMvConversation((prev) => prev?.map((e) => (e.id === id ? patch(e) : e)) ?? prev); + setMvConversationResult(prev => prev ? { + ...prev, emails: prev.emails.map(e => e.id === id ? patch(e) : e), + } : null); // An unfolded conversation message needs its body: the thread fetch above // only carries list properties. const handleLoadConversationEmail = async (target: Email): Promise => { if (!client) return null; const { threadClient, accountId } = mvConversationRoute(target, client); + if (!threadClient) return null; + const openSeq = mvConversationOpenSeqRef.current; const fetched = await threadClient.getEmail(target.id, accountId); - if (!fetched) return null; + if (!fetched || openSeq !== mvConversationOpenSeqRef.current) return null; const full: Email = { ...fetched, accountId: target.accountId, @@ -3982,6 +4016,7 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { return; } const { threadClient, accountId } = mvConversationRoute(target, client); + if (!threadClient) return; await threadClient.markAsRead(target.id, true, accountId); void emailHooks.onEmailReadStateChange.emit({ emailIds: [target.id], read: true }); } @@ -4138,7 +4173,8 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { // Fetch the full content try { - const { client: fetchClient, accountId } = resolveEmailBlobContext(listEmail ?? null, client); + const source = resolveEmailBlobContext(listEmail ?? null, client); + const { client: fetchClient, accountId } = source; if (!fetchClient) { console.warn('[mail-app] No connected client for email source'); if (openSeq === emailOpenSeqRef.current) abandonOpenEmail(previousEmail); @@ -4180,8 +4216,10 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { conversationPrefetch, new Promise((resolve) => setTimeout(() => resolve(undefined), MV_CONVERSATION_WAIT_MS)), ]); - if (conversation !== undefined) setMvConversation(conversation); if (openSeq !== emailOpenSeqRef.current) return; + if (conversation !== undefined) setMvConversationResult(conversation ? { + emails: conversation, source, threadId: fullEmail.threadId, + } : null); } // One write: the message, and the end of its loading state. finishOpenEmail(fullEmail); @@ -4202,6 +4240,8 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { // The OS / hardware back button is still wired through popstate → handleNavRestore. const handleMobileBack = () => { if (conversationThread) { + ++conversationOpenSeqRef.current; + setConversationBlobSource(undefined); setConversationThread(null); setConversationEmails([]); } @@ -4226,42 +4266,36 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { // Handle opening conversation view on mobile const handleOpenConversation = async (thread: ThreadGroup) => { if (!client) return; + const source = resolveEmailBlobContext(thread.latestEmail, client); + // Never fetch a stamped thread through an unrelated login. + if (!source.client) return; + const openSeq = ++conversationOpenSeqRef.current; - // The view marks the thread read; keep its row where it was tapped. useEmailStore.getState().holdListRow(thread.threadKey); setConversationThread(thread); + setConversationBlobSource(source); + setConversationEmails([]); setIsLoadingConversation(true); setActiveView("viewer"); try { - // The thread may belong to another account: in unified/aggregate views via - // the email's own source stamp (#281), and in a directly-browsed shared - // folder via the selected mailbox's owner (#814). Either way the fetch has - // to name that account - thread ids are per-account, so an unscoped fetch - // resolves to an unrelated thread in the active one. - const ref = thread.emails?.[0]; - const route = resolveThreadRoute({ isUnifiedView, ref, mailboxes: viewMailboxes, selectedMailbox }); - const threadClient = route.clientAccountId - ? (useAuthStore.getState().getClientForAccount(route.clientAccountId) ?? client) - : client; - const emails = await threadClient.getThreadEmails(thread.threadId, route.accountId); - // Re-stamp the source reference so conversation actions (reply/move/…) - // resolve to the right account; the fetched objects don't carry it. - if (isUnifiedView && ref) { - for (const e of emails) { - e.accountId = ref.accountId; - e.accountLabel = ref.accountLabel; - e.sourceClientAccountId = ref.sourceClientAccountId; - e.sourceAccountId = ref.sourceAccountId; - } - } - setConversationEmails(emails); + const emails = await source.client.getThreadEmails(thread.threadId, source.accountId); + if (openSeq !== conversationOpenSeqRef.current) return; + // Preserve UI identity stamps; direct-folder messages stay unstamped. + const ref = thread.latestEmail; + setConversationEmails(emails.map(e => ({ + ...e, + accountId: ref.accountId, + accountLabel: ref.accountLabel, + sourceClientAccountId: ref.sourceClientAccountId, + sourceAccountId: ref.sourceAccountId, + }))); } catch (error) { + if (openSeq !== conversationOpenSeqRef.current) return; console.error('Failed to fetch thread emails:', error); - // Fall back to thread.emails setConversationEmails(thread.emails); } finally { - setIsLoadingConversation(false); + if (openSeq === conversationOpenSeqRef.current) setIsLoadingConversation(false); } }; @@ -5209,7 +5243,8 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { onReply={handleConversationReply} onReplyAll={handleConversationReplyAll} onForward={handleConversationForward} - onDownloadAttachment={handleDownloadAttachment} + blobSource={conversationBlobSource} + onDownloadAttachment={handleDownloadConversationAttachment} onMarkAsRead={async (emailId, read) => { if (client) { const target = conversationEmails.find((e) => e.id === emailId); diff --git a/stores/email-store.ts b/stores/email-store.ts index 181e358bc..cd4a71c5e 100644 --- a/stores/email-store.ts +++ b/stores/email-store.ts @@ -1043,10 +1043,16 @@ function findListRow(rows: ListRow[], key: string): number { return own !== -1 ? own : rows.findIndex((row) => row.members.includes(key)); } +export interface EmailBlobContext { + client: IJMAPClient | null; + accountId?: string; + clientAccountId?: string; +} + export function resolveEmailBlobContext( email: Pick | null, passedClient: IJMAPClient | null, -): { client: IJMAPClient | null; accountId?: string; clientAccountId?: string } { +): EmailBlobContext { const state = useEmailStore.getState(); const auth = useAuthStore.getState(); const clientAccountId = email?.sourceClientAccountId ?? state.viewingAccountId ?? undefined;