From 9247ef1ebe90ac75ff1b5387c9974cf636fcb304 Mon Sep 17 00:00:00 2001 From: Phillip Carter Date: Fri, 31 Jul 2026 15:59:17 -0700 Subject: [PATCH] Improve formwarding rendering --- src/app/compose/page.tsx | 50 +++++++++- src/app/smoke-tests/SmokeHarness.tsx | 45 +++++++++ src/app/smoke-tests/page.tsx | 1 + src/components/Composer.tsx | 47 ++++++--- src/lib/__tests__/compose.test.ts | 13 +++ src/lib/__tests__/composeHtml.test.ts | 46 ++++++++- src/lib/compose.ts | 10 ++ src/lib/composeHtml.ts | 135 +++++++++++++++++++++----- tests/smoke/mail.spec.ts | 20 ++++ 9 files changed, 322 insertions(+), 45 deletions(-) diff --git a/src/app/compose/page.tsx b/src/app/compose/page.tsx index 233a8fc..be93d00 100644 --- a/src/app/compose/page.tsx +++ b/src/app/compose/page.tsx @@ -14,7 +14,9 @@ import MobileBackButton from "@/components/MobileBackButton"; import { getJmapContext } from "@/lib/jmapServer"; import { visibleAttachments } from "@/lib/attachments"; import { + buildForwardedHtml, extractForwardedHtml, + replaceCidReferences, } from "@/lib/composeHtml"; import { sanitizeReaderHtml } from "@/lib/printHtml"; import type { EmailBodyPart } from "@/lib/types"; @@ -188,14 +190,54 @@ export default async function ComposePage({ searchParams }: Props) { } else if (mode === "forward") { title = "Forward"; initialSubject = fwdSubject(email.subject); - // Capture the original HTML body so the Composer can append it - // verbatim to the outgoing email — preserving images and formatting. + // Preserve downloadable files and embedded CID resources from the + // original message. CID references are rewritten to stable IDs used by + // the new outgoing multipart/related message. + const forwardedInlineParts = (email.attachments ?? []).filter((part) => + !!part.blobId && + !!part.cid && + part.type !== "text/calendar" && + part.disposition?.toLowerCase() !== "attachment" + ); + const cidReplacements = forwardedInlineParts.map((part, index) => { + const inlineId = `forwarded-${index + 1}`; + initialInlineImages.push({ + id: inlineId, + blobId: part.blobId!, + dataUrl: inlinePartUrl(part), + type: part.type, + }); + return { cid: part.cid!, url: `cid:${inlineId}@mail` }; + }); + initialAttachments = visibleAttachments(email.attachments).flatMap( + (part) => + part.blobId + ? [{ + id: `forwarded-${part.blobId}`, + name: part.name ?? "attachment", + size: part.size, + type: part.type, + blobId: part.blobId, + }] + : [], + ); + + // Keep the original rich message as a separate HTML fragment. The + // Markdown quote remains the text/plain fallback and is not duplicated + // into the outgoing HTML part. if (email.htmlBody?.length > 0) { const part = email.htmlBody[0]; if (part.partId && email.bodyValues?.[part.partId]) { - forwardedHtml = sanitizeReaderHtml( - email.bodyValues[part.partId].value, + const originalHtml = replaceCidReferences( + sanitizeReaderHtml(email.bodyValues[part.partId].value), + cidReplacements, ); + forwardedHtml = buildForwardedHtml(originalHtml, { + from: addrList(email.from), + to: addrList(email.to), + date: dateStr, + subject: email.subject ?? "", + }); } } // The markdown body carries the plain-text fallback (text/plain part diff --git a/src/app/smoke-tests/SmokeHarness.tsx b/src/app/smoke-tests/SmokeHarness.tsx index fffee93..207ac79 100644 --- a/src/app/smoke-tests/SmokeHarness.tsx +++ b/src/app/smoke-tests/SmokeHarness.tsx @@ -15,6 +15,7 @@ import { getUnreadFaviconDataUrl, } from "@/lib/browserTabIndicator"; import { prepareHtml } from "@/lib/emailHtml"; +import { buildForwardedHtml } from "@/lib/composeHtml"; import { dispatchUnreadCountEvent } from "@/lib/unreadCount"; import type { Email, EmailBodyPart } from "@/lib/types"; import type { MailPanelData } from "@/lib/jmap"; @@ -22,6 +23,7 @@ import type { MailPanelData } from "@/lib/jmap"; export type SmokePanel = | "inbox" | "reply" + | "forward" | "attachments" | "target" | "auto-sync" @@ -152,6 +154,31 @@ const darkRenderingDocument = prepareHtml( { colorMode: "dark" }, ); +const forwardedNewsletterHtml = buildForwardedHtml( + ` + + + + +
+

Retrieval Weekly

+

The latest research, presented in its original layout.

+ +
+ + `, + { + from: "Retrieval Weekly ", + to: "Phillip Carter ", + date: "July 31, 2026", + subject: "The latest in information retrieval", + }, +); + const navItems: Array<{ panel: SmokePanel; label: string }> = [ { panel: "inbox", label: "Inbox" }, { panel: "reply", label: "Reply" }, @@ -339,6 +366,24 @@ export default function SmokeHarness({ panel }: { panel: SmokePanel }) { /> )} + {panel === "forward" && ( + \n**To:** Phillip Carter \n**Date:** July 31, 2026 \n**Subject:** The latest in information retrieval\n\nRetrieval Weekly [ https://substack.com/redirect/very-long-tracking-link ]" + } + forwardedHtml={forwardedNewsletterHtml} + /> + )} + {panel === "attachments" && (

diff --git a/src/app/smoke-tests/page.tsx b/src/app/smoke-tests/page.tsx index df9e90b..4f731f9 100644 --- a/src/app/smoke-tests/page.tsx +++ b/src/app/smoke-tests/page.tsx @@ -8,6 +8,7 @@ interface Props { const smokePanels = new Set([ "inbox", "reply", + "forward", "attachments", "target", "auto-sync", diff --git a/src/components/Composer.tsx b/src/components/Composer.tsx index 11b8298..a446620 100644 --- a/src/components/Composer.tsx +++ b/src/components/Composer.tsx @@ -7,14 +7,16 @@ import { saveDraftAction, deleteDraftAction } from "@/app/compose/actions"; import { useToast } from "@/components/ToastProvider"; import { useNavigationGuard } from "@/components/NavigationGuardProvider"; import { - appendForwardedHtml, + combineEmailHtml, markQuotedReplyHtml, + replaceCidReferences, wrapComposePreviewHtml, wrapEmailHtml, } from "@/lib/composeHtml"; import { normalizeComposeMarkdown, htmlToPlainText, + markdownBeforeQuotedHistory, quotedSectionStart, applyIdentitySignature, } from "@/lib/compose"; @@ -375,13 +377,23 @@ export default function Composer({ useEffect(() => { let cancelled = false; (async () => { - const html = await marked.parse(normalizeComposeMarkdown(markdown)); + const htmlMarkdown = forwardedHtml + ? markdownBeforeQuotedHistory(markdown) + : markdown; + const html = await marked.parse(normalizeComposeMarkdown(htmlMarkdown)); const withImages = replacePlaceholders(html, (id) => { return inlineImages.find((img) => img.id === id)?.dataUrl ?? ""; }); - const composed = forwardedHtml - ? withImages + appendForwardedHtml(forwardedHtml) - : withImages; + const previewForward = forwardedHtml + ? replaceCidReferences( + forwardedHtml, + inlineImages.map(({ id, dataUrl }) => ({ + cid: `${id}@mail`, + url: dataUrl, + })), + ) + : undefined; + const composed = combineEmailHtml(withImages, previewForward); const body = inReplyToId ? markQuotedReplyHtml(composed) : composed; if (!cancelled) setPreview(wrapComposePreviewHtml(body)); })(); @@ -415,14 +427,15 @@ export default function Composer({ const existingDraftId = draftIdRef.current; const isFirstSave = !existingDraftId; try { - const rawHtml = await marked.parse(normalizeComposeMarkdown(markdown)); + const htmlMarkdown = forwardedHtml + ? markdownBeforeQuotedHistory(markdown) + : markdown; + const rawHtml = await marked.parse(normalizeComposeMarkdown(htmlMarkdown)); const htmlWithCids = replacePlaceholders( rawHtml, (id) => `cid:${id}@mail`, ); - const renderedBody = forwardedHtml - ? htmlWithCids + appendForwardedHtml(forwardedHtml) - : htmlWithCids; + const renderedBody = combineEmailHtml(htmlWithCids, forwardedHtml); const composedBody = inReplyToId ? markQuotedReplyHtml(renderedBody) : renderedBody; @@ -617,11 +630,17 @@ export default function Composer({ suppressDraftSideEffectsRef.current = true; cleanupPendingDraftsRef.current = false; try { - const rawHtml = await marked.parse(normalizeComposeMarkdown(markdown)); + const fullMarkdownHtml = await marked.parse( + normalizeComposeMarkdown(markdown), + ); + const htmlMarkdown = forwardedHtml + ? markdownBeforeQuotedHistory(markdown) + : markdown; + const rawHtml = forwardedHtml + ? await marked.parse(normalizeComposeMarkdown(htmlMarkdown)) + : fullMarkdownHtml; const htmlWithCids = replacePlaceholders(rawHtml, (id) => `cid:${id}@mail`); - const renderedBody = forwardedHtml - ? htmlWithCids + appendForwardedHtml(forwardedHtml) - : htmlWithCids; + const renderedBody = combineEmailHtml(htmlWithCids, forwardedHtml); const composedBody = inReplyToId ? markQuotedReplyHtml(renderedBody) : renderedBody; @@ -643,7 +662,7 @@ export default function Composer({ subject, // Drafts stay Markdown, while the text/plain transport is a clean, // readable fallback derived from the rendered representation. - textBody: htmlToPlainText(composedBody), + textBody: htmlToPlainText(fullMarkdownHtml), htmlBody: wrapEmailHtml(composedBody), inlineImages: inlineImagesRef.current.map(({ id, blobId, type }) => ({ id, diff --git a/src/lib/__tests__/compose.test.ts b/src/lib/__tests__/compose.test.ts index e94e015..1ed180b 100644 --- a/src/lib/__tests__/compose.test.ts +++ b/src/lib/__tests__/compose.test.ts @@ -9,6 +9,7 @@ import { htmlToPlainText, normalizeComposeMarkdown, quotedSectionStart, + markdownBeforeQuotedHistory, formatSignatureForSave, stripSignatureSeparator, applyIdentitySignature, @@ -293,3 +294,15 @@ describe("quotedSectionStart", () => { assert.equal(quotedSectionStart(markdown), "Reply\n\n".length); }); }); + +describe("markdownBeforeQuotedHistory", () => { + it("keeps the author's note while omitting the forwarded text fallback", () => { + const markdown = + "For your review.\n\n---\n\n**---------- Forwarded message ----------**\n\nA long plain-text original"; + assert.equal(markdownBeforeQuotedHistory(markdown), "For your review."); + }); + + it("keeps an ordinary message intact", () => { + assert.equal(markdownBeforeQuotedHistory("Hello\n\nWorld"), "Hello\n\nWorld"); + }); +}); diff --git a/src/lib/__tests__/composeHtml.test.ts b/src/lib/__tests__/composeHtml.test.ts index 5301a6a..f5b85a1 100644 --- a/src/lib/__tests__/composeHtml.test.ts +++ b/src/lib/__tests__/composeHtml.test.ts @@ -1,9 +1,11 @@ import assert from "node:assert/strict"; import { describe, it } from "node:test"; import { - appendForwardedHtml, + buildForwardedHtml, + combineEmailHtml, extractForwardedHtml, markQuotedReplyHtml, + replaceCidReferences, wrapComposePreviewHtml, wrapEmailHtml, } from "../composeHtml"; @@ -28,16 +30,50 @@ describe("wrapEmailHtml", () => { describe("forwarded draft HTML", () => { it("round-trips forwarded HTML through a saved draft", () => { - const forwarded = appendForwardedHtml( - "Original

Original message

", + const forwardedHtml = buildForwardedHtml( + "

Original message

", + { + from: "Sender ", + to: "Reader ", + date: "July 31, 2026", + subject: "Original subject", + }, ); - const draft = wrapEmailHtml(`

My note

${forwarded}`); - assert.equal(extractForwardedHtml(draft), "

Original message

"); + const draft = wrapEmailHtml( + combineEmailHtml("

My note

", forwardedHtml), + ); + assert.equal(extractForwardedHtml(draft), forwardedHtml); + assert.ok(draft.includes(".brand{color:#123456}")); + assert.ok(draft.includes('class="newsletter"')); + assert.ok(draft.includes("Original subject")); }); it("returns no forwarded body for an ordinary draft", () => { assert.equal(extractForwardedHtml(wrapEmailHtml("

Hello

")), undefined); }); + + it("keeps authored styles from changing the preserved original layout", () => { + const result = wrapEmailHtml( + combineEmailHtml( + "

My note

", + "
Original layout
", + ), + ); + assert.ok(result.includes(".mail-authored-content td")); + assert.ok(!result.includes(".mail-content td")); + }); + + it("rewrites embedded content IDs for the outgoing message and preview", () => { + const outgoing = replaceCidReferences( + '
', + [ + { cid: "hero@original", url: "cid:forwarded-1@mail" }, + { cid: "bg@original", url: "/api/download?blobId=2" }, + ], + ); + assert.ok(outgoing.includes('src="cid:forwarded-1@mail"')); + assert.ok(outgoing.includes("url(/api/download?blobId=2)")); + }); }); describe("markQuotedReplyHtml", () => { diff --git a/src/lib/compose.ts b/src/lib/compose.ts index ed75c77..20eef22 100644 --- a/src/lib/compose.ts +++ b/src/lib/compose.ts @@ -90,6 +90,16 @@ export function quotedSectionStart(markdown: string): number { return starts.length > 0 ? Math.min(...starts) : -1; } +/** + * Return only the author-controlled portion of a reply or forward. This is + * useful when a rich original message is supplied separately for the HTML + * transport and the Markdown quote exists solely as the text/plain fallback. + */ +export function markdownBeforeQuotedHistory(markdown: string): string { + const quoteStart = quotedSectionStart(markdown); + return (quoteStart >= 0 ? markdown.slice(0, quoteStart) : markdown).trimEnd(); +} + /** * Replace the managed signature in the editable portion of a composition. * Quoted reply/forward history is kept byte-for-byte after the signature. diff --git a/src/lib/composeHtml.ts b/src/lib/composeHtml.ts index dd12d7c..3c69fc5 100644 --- a/src/lib/composeHtml.ts +++ b/src/lib/composeHtml.ts @@ -5,31 +5,33 @@ const EMAIL_CONTENT_CSS = ` max-width: 680px; margin: 0 auto; padding: 24px; + } + .mail-authored-content { color: #172033; font-family: -apple-system, BlinkMacSystemFont, "Segoe UI", Arial, sans-serif; font-size: 15px; line-height: 1.65; overflow-wrap: anywhere; } - .mail-content > :first-child { margin-top: 0; } - .mail-content > :last-child { margin-bottom: 0; } - .mail-content h1 { font-size: 1.45em; line-height: 1.25; font-weight: 650; margin: 1.4em 0 0.5em; } - .mail-content h2 { font-size: 1.25em; line-height: 1.3; font-weight: 650; margin: 1.4em 0 0.5em; } - .mail-content h3 { font-size: 1.08em; line-height: 1.35; font-weight: 650; margin: 1.4em 0 0.5em; } - .mail-content p { margin: 0 0 1em; } - .mail-content a { color: #1d4ed8; text-decoration: underline; text-underline-offset: 2px; } - .mail-content ul, .mail-content ol { padding-left: 1.5em; margin: 0 0 1em; } - .mail-content li { margin: 0.2em 0; } - .mail-content hr { border: 0; border-top: 1px solid #e2e8f0; margin: 1.5em 0; } - .mail-content img { max-width: 100%; height: auto; display: block; margin: 1em 0; } - .mail-content code { + .mail-authored-content > :first-child { margin-top: 0; } + .mail-authored-content > :last-child { margin-bottom: 0; } + .mail-authored-content h1 { font-size: 1.45em; line-height: 1.25; font-weight: 650; margin: 1.4em 0 0.5em; } + .mail-authored-content h2 { font-size: 1.25em; line-height: 1.3; font-weight: 650; margin: 1.4em 0 0.5em; } + .mail-authored-content h3 { font-size: 1.08em; line-height: 1.35; font-weight: 650; margin: 1.4em 0 0.5em; } + .mail-authored-content p { margin: 0 0 1em; } + .mail-authored-content a { color: #1d4ed8; text-decoration: underline; text-underline-offset: 2px; } + .mail-authored-content ul, .mail-authored-content ol { padding-left: 1.5em; margin: 0 0 1em; } + .mail-authored-content li { margin: 0.2em 0; } + .mail-authored-content hr { border: 0; border-top: 1px solid #e2e8f0; margin: 1.5em 0; } + .mail-authored-content img { max-width: 100%; height: auto; display: block; margin: 1em 0; } + .mail-authored-content code { font-family: "SFMono-Regular", Consolas, "Liberation Mono", monospace; background: #f1f5f9; padding: 0.15em 0.35em; border-radius: 4px; font-size: 0.9em; } - .mail-content pre { + .mail-authored-content pre { box-sizing: border-box; max-width: 100%; padding: 14px 16px; @@ -39,15 +41,15 @@ const EMAIL_CONTENT_CSS = ` overflow-x: auto; white-space: pre-wrap; } - .mail-content pre code { padding: 0; border: 0; background: transparent; } - .mail-content blockquote { + .mail-authored-content pre code { padding: 0; border: 0; background: transparent; } + .mail-authored-content blockquote { margin: 1em 0; padding: 0 0 0 14px; border-left: 2px solid #cbd5e1; color: #64748b; } - .mail-content table { max-width: 100%; border-collapse: collapse; } - .mail-content th, .mail-content td { padding: 6px 8px; border: 1px solid #e2e8f0; text-align: left; } + .mail-authored-content table { max-width: 100%; border-collapse: collapse; } + .mail-authored-content th, .mail-authored-content td { padding: 6px 8px; border: 1px solid #e2e8f0; text-align: left; } `; const FORWARDED_HTML_START = ""; @@ -63,9 +65,99 @@ function extractBodyContent(html: string): string { .trim(); } -export function appendForwardedHtml(originalHtml: string): string { - const content = extractBodyContent(originalHtml); - return `
${FORWARDED_HTML_START}${content}${FORWARDED_HTML_END}
`; +function extractHeadStyles(html: string): string { + const head = html.match(/]*>([\s\S]*?)<\/head>/i)?.[1] ?? ""; + return (head.match(/]*>[\s\S]*?<\/style>/gi) ?? []).join("\n"); +} + +function extractBodyPresentation(html: string): { className: string; style: string } { + const attributes = html.match(/]*)>/i)?.[1] ?? ""; + const className = attributes.match(/\bclass\s*=\s*(["'])(.*?)\1/i)?.[2] ?? ""; + const style = attributes.match(/\bstyle\s*=\s*(["'])(.*?)\1/i)?.[2] ?? ""; + return { className, style }; +} + +function escapeHtml(value: string): string { + return value + .replace(/&/g, "&") + .replace(//g, ">") + .replace(/"/g, """) + .replace(/'/g, "'"); +} + +export interface ForwardedMessageHeaders { + from: string; + to: string; + date: string; + subject: string; +} + +/** Build the preserved original-message fragment used by rich HTML forwards. */ +export function buildForwardedHtml( + originalHtml: string, + headers: ForwardedMessageHeaders, +): string { + const originalStyles = extractHeadStyles(originalHtml); + const originalBody = extractBodyContent(originalHtml) + .replaceAll(FORWARDED_HTML_START, "") + .replaceAll(FORWARDED_HTML_END, ""); + const bodyPresentation = extractBodyPresentation(originalHtml); + const classAttribute = bodyPresentation.className + ? ` class="${escapeHtml(bodyPresentation.className)}"` + : ""; + const styleAttribute = bodyPresentation.style + ? ` style="${escapeHtml(bodyPresentation.style)}"` + : ""; + + return `${originalStyles} +
+
---------- Forwarded message ----------
+
From: ${escapeHtml(headers.from)}
+
To: ${escapeHtml(headers.to)}
+
Date: ${escapeHtml(headers.date)}
+
Subject: ${escapeHtml(headers.subject)}
+
+
${originalBody}
`; +} + +/** Replace CID references without otherwise rewriting the original HTML. */ +export function replaceCidReferences( + html: string, + replacements: ReadonlyArray<{ cid: string; url: string }>, +): string { + const byCid = new Map( + replacements.map(({ cid, url }) => [normalizeCid(cid), url]), + ); + if (byCid.size === 0) return html; + + return html.replace(/cid:([^\s"'()<>]+)/gi, (match, cid: string) => { + return byCid.get(normalizeCid(cid)) ?? match; + }); +} + +function normalizeCid(value: string): string { + let decoded = value; + try { + decoded = decodeURIComponent(value); + } catch { + // Preserve malformed identifiers exactly as received. + } + return decoded.replace(/^cid:/i, "").replace(/^<|>$/g, "").trim().toLowerCase(); +} + +export function appendForwardedHtml(forwardedHtml: string): string { + return `
${FORWARDED_HTML_START}${forwardedHtml}${FORWARDED_HTML_END}
`; +} + +export function combineEmailHtml( + authoredHtml: string, + forwardedHtml?: string, +): string { + const authored = `
${authoredHtml}
`; + return forwardedHtml + ? `${authored}${appendForwardedHtml(forwardedHtml)}` + : authored; } export function extractForwardedHtml(draftHtml: string): string | undefined { @@ -112,7 +204,7 @@ export function wrapEmailHtml(body: string): string { -
+
${body}
@@ -150,7 +242,6 @@ export function wrapComposePreviewHtml(body: string): string { overflow: hidden; } ${EMAIL_CONTENT_CSS} - [data-forwarded-email="true"] { color: #475569; } @media (prefers-color-scheme: dark) { :root { --canvas: #020617; --border: #334155; } /* The preview intentionally remains a white email surface. That is what diff --git a/tests/smoke/mail.spec.ts b/tests/smoke/mail.spec.ts index 53316e4..7bb5dc4 100644 --- a/tests/smoke/mail.spec.ts +++ b/tests/smoke/mail.spec.ts @@ -417,6 +417,26 @@ test("keeps reply history intact while editing Markdown", async ({ page }) => { await expect(editor).toHaveValue(/The revised plan is ready for review\./); }); +test("preserves the original rich layout when forwarding HTML mail", async ({ + page, +}) => { + await page.goto("/smoke-tests?panel=forward"); + await page.getByRole("button", { name: "Preview" }).click(); + + const preview = page.frameLocator('iframe[title="Email preview"]'); + const newsletter = preview.locator("#preserved-newsletter"); + await expect(newsletter).toBeVisible(); + await expect(newsletter).toHaveCSS("background-color", "rgb(238, 242, 255)"); + await expect(newsletter).toHaveCSS("border-top-width", "2px"); + await expect(preview.getByText("Read the paper")).toBeVisible(); + await expect( + preview.getByText("https://substack.com/redirect/very-long-tracking-link"), + ).toHaveCount(0); + await expect( + preview.getByText("---------- Forwarded message ----------"), + ).toBeVisible(); +}); + test("switches the signature with the sending identity", async ({ page }) => { await page.goto("/smoke-tests?panel=reply"); -- 2.51.2