From 8a09ba573448248ee24522089d46f953b4ff607b Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Sat, 31 Oct 2026 01:50:03 +0100 Subject: [PATCH] fix: leave a bounce's delivery report out of list chips and forwards --- .../email/__tests__/attachment-chips.test.tsx | 12 + .../composer-forward-bounce.test.tsx | 229 ++++++++++++++++++ components/email/attachment-chips.tsx | 6 +- components/email/email-composer.tsx | 14 +- components/email/email-viewer.tsx | 44 +--- components/email/thread-conversation-view.tsx | 7 +- lib/attachment-name.ts | 48 ++++ lib/attachment-visibility.ts | 17 ++ 8 files changed, 327 insertions(+), 50 deletions(-) create mode 100644 components/email/__tests__/composer-forward-bounce.test.tsx create mode 100644 lib/attachment-name.ts diff --git a/components/email/__tests__/attachment-chips.test.tsx b/components/email/__tests__/attachment-chips.test.tsx index 77f292c13..414269ab8 100644 --- a/components/email/__tests__/attachment-chips.test.tsx +++ b/components/email/__tests__/attachment-chips.test.tsx @@ -36,6 +36,18 @@ describe("realAttachments", () => { expect(realAttachments([att({ name: undefined })])).toEqual([]); }); + it("drops the report part of a bounce, which the reader does not list either", () => { + // A forwarded bounce: both parts named "attachment" by the forward, but + // only the original message is something the reader shows. + const original = att({ partId: "3", blobId: "b3", name: "attachment", type: "message/rfc822" }); + const out = realAttachments([ + att({ partId: "2", blobId: "b2", name: "attachment", type: "message/delivery-status" }), + original, + att({ partId: "4", blobId: "b4", name: "receipt", type: "Message/Disposition-Notification" }), + ]); + expect(out).toEqual([original]); + }); + it("keeps genuine attachments in order", () => { const a = att({ partId: "1", blobId: "b1", name: "a.pdf" }); const b = att({ partId: "2", blobId: "b2", name: "b.xlsx", type: "application/vnd.ms-excel" }); diff --git a/components/email/__tests__/composer-forward-bounce.test.tsx b/components/email/__tests__/composer-forward-bounce.test.tsx new file mode 100644 index 000000000..88486dafb --- /dev/null +++ b/components/email/__tests__/composer-forward-bounce.test.tsx @@ -0,0 +1,229 @@ +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 { useAuthStore } from '@/stores/auth-store'; + +// ─── 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 }), +})); + +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 }; +}); + +vi.mock('@/stores/settings-store', () => { + const state = { + timeFormat: '24h', + plainTextMode: false, + subAddressDelimiter: '+', + autoSelectReplyIdentity: true, + attachmentReminderEnabled: false, + attachmentReminderKeywords: [], + emptySubjectWarningEnabled: true, + sendDelaySeconds: 0, + externalContentPolicy: 'block', + isSenderTrusted: () => false, + signaturePosition: 'above_quote', + signatureSeparatorEnabled: false, + 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 }; +}); + +vi.mock('@/stores/toast-store', () => ({ + toast: { info: () => {}, error: () => {}, success: () => {} }, +})); + +vi.mock('@/lib/plugin-hooks', () => ({ + identityHooks: { + onSignatureRender: { size: 0, transform: async (value: unknown) => value }, + onIdentitySelect: { size: 0, emit: async () => {} }, + }, + templateHooks: { + onTemplateRender: { size: 0, transform: async (value: unknown) => value }, + onTemplateApply: { size: 0, emit: async () => {} }, + }, + 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, interceptOutcome: async () => 'allowed' }, + onComposeSend: { takeOver: async () => 'default' }, + 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: () => '', + plainTextBodyHasSignature: () => false, + plainTextBodyWithoutSignature: (body: string) => body, +})); +vi.mock('@/lib/sub-addressing', () => ({ generateSubAddress: () => '' })); +vi.mock('@/lib/debug', () => ({ debug: { log: () => {}, warn: () => {}, error: () => {} } })); +vi.mock('@/lib/template-utils', () => ({ substitutePlaceholders: (s: string) => s })); + +// ─── Tests ──────────────────────────────────────────────────────────────────── + +/** + * A bounce carries its delivery status (message/delivery-status) and the + * original message (message/rfc822) as parts without a name. The reader hides + * the first and shows the second as "Email.eml"; a forward used to send both + * as files called "attachment", which the recipient's list showed as two + * nameless chips next to the one the reader listed. + */ +describe('forwarding a bounce', () => { + afterEach(() => { + useAuthStore.setState({ client: null }); + vi.clearAllMocks(); + }); + + it('leaves the delivery status behind and names the original message like the reader does', async () => { + useAuthStore.setState({ + client: { + hasDelayedSend: () => false, + supportsPrincipals: () => false, + getMaxDelayedSend: () => 0, + } as never, + }); + const onSend = vi.fn(); + + render( + + ); + + expect(screen.getByText('Email.eml')).toBeInTheDocument(); + expect(screen.queryByText('attachment')).not.toBeInTheDocument(); + + fireEvent.paste(screen.getByPlaceholderText('to_placeholder'), { + clipboardData: { getData: () => 'bob@example.com;' }, + }); + fireEvent.click(screen.getAllByTestId('composer-send')[0]); + await waitFor(() => expect(onSend).toHaveBeenCalledTimes(1)); + + const sent = onSend.mock.calls[0][0] as { attachments?: Array<{ blobId: string; name: string; type: string }> }; + expect(sent.attachments).toEqual([ + expect.objectContaining({ blobId: 'blob-orig', name: 'Email.eml', type: 'message/rfc822' }), + ]); + }); +}); diff --git a/components/email/attachment-chips.tsx b/components/email/attachment-chips.tsx index 770259429..65e7caf3b 100644 --- a/components/email/attachment-chips.tsx +++ b/components/email/attachment-chips.tsx @@ -4,6 +4,7 @@ import { useEffect, useState, type ReactNode } from "react"; import { FileText, FileSpreadsheet, FileImage, FileVideo, FileAudio, FileArchive, File as FileIcon } from "@/components/icons"; import { cn } from "@/lib/utils"; import { stripBidiControls } from "@/lib/bidi"; +import { isReportPart } from "@/lib/attachment-visibility"; import type { Attachment, Email } from "@/lib/jmap/types"; import type { LoadListAttachments } from "@/lib/list-attachments"; @@ -12,12 +13,13 @@ import type { LoadListAttachments } from "@/lib/list-attachments"; * pixels, spacers a couple of hundred bytes each — none of which anyone * wants to download. Only parts the sender actually attached belong in * the list, so anything marked inline or referenced by a Content-ID is - * dropped. + * dropped, and so are the report parts of a bounce or read receipt, which + * the reader does not list either. */ export function realAttachments(attachments?: Attachment[]): Attachment[] { if (!attachments?.length) return []; return attachments.filter( - (a) => a.disposition !== "inline" && !a.cid && !!a.name, + (a) => a.disposition !== "inline" && !a.cid && !!a.name && !isReportPart(a.type), ); } diff --git a/components/email/email-composer.tsx b/components/email/email-composer.tsx index a1771da5b..bc8f02382 100644 --- a/components/email/email-composer.tsx +++ b/components/email/email-composer.tsx @@ -142,7 +142,8 @@ import { composeTextStyle, composeTextStyleCss } from "@/components/email/font-s import { buildMentionCandidates, type MentionCandidate } from "@/lib/recipient-mentions"; import { describeRecipientChanges, recipientDisplayName, type RecipientChange, type RecipientFields } from "@/lib/recipient-announcements"; import { announce as announceStatus } from "@/lib/announce"; -import { normalizeCid } from "@/lib/attachment-visibility"; +import { isReportPart, normalizeCid } from "@/lib/attachment-visibility"; +import { unnamedAttachmentName } from "@/lib/attachment-name"; import { RichTextEditor } from "@/components/email/rich-text-editor"; import { isRecipientMentionActive } from "@/components/email/recipient-mention"; import type { Editor } from "@tiptap/react"; @@ -953,7 +954,7 @@ export function EmailComposer({ return initialData.attachments .filter(att => !!att.blobId) .map(att => ({ - name: att.name || 'attachment', + name: att.name || unnamedAttachmentName(att.type), type: att.type || 'application/octet-stream', size: att.size, blobId: att.blobId, @@ -979,8 +980,13 @@ export function EmailComposer({ embeddedCids.has(normalizeCid(att.cid)) || (att.disposition === 'inline' && (att.type || '').startsWith('image/')) ))) + // A bounce's delivery status or a receipt's disposition is no file + // anyone attached; the reader does not list it, so it stays behind. + .filter(att => !isReportPart(att.type)) + // A part without a name goes out under the one the reader showed + // ("Email.eml" for the original message of a bounce). .map(att => ({ - name: att.name || 'attachment', + name: att.name || unnamedAttachmentName(att.type), type: att.type || 'application/octet-stream', size: att.size, blobId: att.blobId, @@ -2702,7 +2708,7 @@ export function EmailComposer({ // Uploads that finished during the save are not in this version. if (!att.blobId || att.uploading || !savedBlobIds.has(att.blobId)) return att; const idx = freshParts.findIndex((p, i) => - !claimed.has(i) && (p.name || 'attachment') === att.name && p.size === att.size); + !claimed.has(i) && (p.name || unnamedAttachmentName(p.type)) === att.name && p.size === att.size); if (idx === -1) return att; claimed.add(idx); return { ...att, blobId: freshParts[idx].blobId, fromDraftPart: true }; diff --git a/components/email/email-viewer.tsx b/components/email/email-viewer.tsx index 5e82d2d40..5716dfbaa 100644 --- a/components/email/email-viewer.tsx +++ b/components/email/email-viewer.tsx @@ -16,12 +16,12 @@ import { ReadAloudBar } from "./read-aloud-bar"; import { attachmentIcon } from "./attachment-chips"; import { ReaderViewDialog } from "./reader-view-dialog"; import { ReadingMask } from "./reading-mask"; -import { collectReferencedCids, isEmbeddedInBody, normalizeCid, rewriteCidReferences } from "@/lib/attachment-visibility"; +import { collectReferencedCids, isEmbeddedInBody, isReportPart, normalizeCid, rewriteCidReferences } from "@/lib/attachment-visibility"; +import { getAttachmentDisplayName } from "@/lib/attachment-name"; import { collapsePlainTextQuotes, setupQuoteCollapse } from "@/lib/quote-collapse"; import { DARK_PENDING_CLASS, applyEmailDarkMode, emailColorScheme, emailDarkModeCss, emailHasNativeDarkMode as htmlHasNativeDarkMode, resolveColorSchemeMedia } from "@/lib/email-dark-mode"; import { fitEmailBodyWidth, type FitOptions } from "@/lib/email-fit-width"; import { hasFullBleedCanvas } from "@/lib/email-full-bleed"; -import { stripBidiControls } from "@/lib/bidi"; import { withBasePath } from "@/lib/browser-navigation"; import { buildContactsPath } from "@/lib/deep-links"; import { Button } from "@/components/ui/button"; @@ -278,44 +278,6 @@ interface EmailViewerProps { className?: string; } -const MIME_TYPE_LABELS: Record = { - 'application/pdf': 'Document.pdf', - 'application/zip': 'Archive.zip', - 'application/x-zip-compressed': 'Archive.zip', - 'application/gzip': 'Archive.gz', - 'application/x-rar-compressed': 'Archive.rar', - 'application/x-7z-compressed': 'Archive.7z', - 'application/msword': 'Document.doc', - 'application/vnd.openxmlformats-officedocument.wordprocessingml.document': 'Document.docx', - 'application/vnd.ms-excel': 'Spreadsheet.xls', - 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet': 'Spreadsheet.xlsx', - 'application/vnd.ms-powerpoint': 'Presentation.ppt', - 'application/vnd.openxmlformats-officedocument.presentationml.presentation': 'Presentation.pptx', - 'text/plain': 'Text.txt', - 'text/html': 'Document.html', - 'text/csv': 'Data.csv', - 'application/json': 'Data.json', - 'application/xml': 'Data.xml', - 'application/octet-stream': 'Attachment', - 'message/rfc822': 'Email.eml', -}; - -const getAttachmentDisplayName = (name: string | null | undefined, mimeType?: string): string => { - // Without directional overrides, which would show `report‮fdp.exe` - // as `reportexe.pdf` (the download name is cleaned separately). - if (name) return stripBidiControls(name); - if (mimeType) { - const label = MIME_TYPE_LABELS[mimeType.toLowerCase()]; - if (label) return label; - const sub = mimeType.split('/')[1]; - if (sub) { - const clean = sub.replace(/^x-/, '').replace(/^vnd\./, ''); - return `Attachment.${clean}`; - } - } - return 'Attachment'; -}; - // Helper function to format recipients with contextual display const _formatRecipients = ( recipients: Array<{ name?: string; email: string }> | undefined, @@ -2629,7 +2591,7 @@ export function EmailViewer({ .filter(att => !(hideInlineImageAttachments && isEmbeddedInBody(att, bodyCids))) // Hide machine-readable report parts (MDN read-receipts, DSN bounce // reports). These are required MIME parts, not real user attachments. - .filter(att => att.type !== 'message/disposition-notification' && att.type !== 'message/delivery-status') + .filter(att => !isReportPart(att.type)) .map((attachment, index) => ({ id: attachment.blobId || `${attachment.name || 'attachment'}-${index}`, name: attachment.name || null, diff --git a/components/email/thread-conversation-view.tsx b/components/email/thread-conversation-view.tsx index 7233532b4..3c3706fb5 100644 --- a/components/email/thread-conversation-view.tsx +++ b/components/email/thread-conversation-view.tsx @@ -16,7 +16,7 @@ import { BookingCalendarActions } from "./structured-mail/booking-calendar-actio import { useStructuredMail } from "./structured-mail/use-structured-mail"; import { attachmentIcon } from "./attachment-chips"; import { ExternalSenderChip } from "./external-sender-chip"; -import { collectReferencedCids, isEmbeddedInBody, normalizeCid, rewriteCidReferences } from "@/lib/attachment-visibility"; +import { collectReferencedCids, isEmbeddedInBody, isReportPart, normalizeCid, rewriteCidReferences } from "@/lib/attachment-visibility"; import { collapsePlainTextQuotes, setupQuoteCollapse } from "@/lib/quote-collapse"; import { fitEmailBodyWidth } from "@/lib/email-fit-width"; import { mailLanguage } from "@/lib/mail-language"; @@ -754,9 +754,10 @@ export function ConversationEmailBody({ // Parts the body embeds via cid: stay out of the attachment row while the // user hides inline images - the desktop viewer's rule, shared through - // lib/attachment-visibility.ts so the two views cannot drift apart. + // lib/attachment-visibility.ts so the two views cannot drift apart. The + // report parts of a bounce or read receipt never show. const visibleAttachments = useMemo(() => { - const attachments = email.attachments ?? []; + const attachments = (email.attachments ?? []).filter(att => !isReportPart(att.type)); if (!hideInlineImageAttachments) return attachments; const bodyCids = collectReferencedCids(getRenderableHtmlBody(email)); return attachments.filter(att => !isEmbeddedInBody(att, bodyCids)); diff --git a/lib/attachment-name.ts b/lib/attachment-name.ts new file mode 100644 index 000000000..3a4281061 --- /dev/null +++ b/lib/attachment-name.ts @@ -0,0 +1,48 @@ +import { stripBidiControls } from "@/lib/bidi"; + +const MIME_TYPE_LABELS: Record = { + 'application/pdf': 'Document.pdf', + 'application/zip': 'Archive.zip', + 'application/x-zip-compressed': 'Archive.zip', + 'application/gzip': 'Archive.gz', + 'application/x-rar-compressed': 'Archive.rar', + 'application/x-7z-compressed': 'Archive.7z', + 'application/msword': 'Document.doc', + 'application/vnd.openxmlformats-officedocument.wordprocessingml.document': 'Document.docx', + 'application/vnd.ms-excel': 'Spreadsheet.xls', + 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet': 'Spreadsheet.xlsx', + 'application/vnd.ms-powerpoint': 'Presentation.ppt', + 'application/vnd.openxmlformats-officedocument.presentationml.presentation': 'Presentation.pptx', + 'text/plain': 'Text.txt', + 'text/html': 'Document.html', + 'text/csv': 'Data.csv', + 'application/json': 'Data.json', + 'application/xml': 'Data.xml', + 'application/octet-stream': 'Attachment', + 'message/rfc822': 'Email.eml', +}; + +/** + * The name the reader shows for a part that has none, from its type: the + * original message a bounce carries reads "Email.eml". A forward sends such + * a part under this name, so the recipient sees what the sender saw. + */ +export function unnamedAttachmentName(mimeType?: string | null): string { + if (mimeType) { + const label = MIME_TYPE_LABELS[mimeType.toLowerCase()]; + if (label) return label; + const sub = mimeType.split('/')[1]; + if (sub) { + const clean = sub.replace(/^x-/, '').replace(/^vnd\./, ''); + return `Attachment.${clean}`; + } + } + return 'Attachment'; +} + +export function getAttachmentDisplayName(name: string | null | undefined, mimeType?: string | null): string { + // Without directional overrides, which would show `report‮fdp.exe` + // as `reportexe.pdf` (the download name is cleaned separately). + if (name) return stripBidiControls(name); + return unnamedAttachmentName(mimeType); +} diff --git a/lib/attachment-visibility.ts b/lib/attachment-visibility.ts index bcd05c4b0..f0f0178c5 100644 --- a/lib/attachment-visibility.ts +++ b/lib/attachment-visibility.ts @@ -57,6 +57,23 @@ export function collectReferencedCids(html: string | null | undefined): Readonly return cids; } +/** + * Machine-readable report parts: the delivery status of a bounce (RFC 3464), + * the disposition of a read receipt (RFC 8098) and their RFC 6533 UTF-8 + * forms. A multipart/report needs them, but they are not files anyone + * attached, so no view lists them and a forward leaves them out. + */ +const REPORT_PART_TYPES: ReadonlySet = new Set([ + 'message/delivery-status', + 'message/disposition-notification', + 'message/global-delivery-status', + 'message/global-disposition-notification', +]); + +export function isReportPart(type: string | null | undefined): boolean { + return REPORT_PART_TYPES.has((type || '').toLowerCase()); +} + export interface AttachmentPartLike { cid?: string | null; type?: string | null;