diff --git a/server/src/mock/data.ts b/server/src/mock/data.ts index a9bd8b6..162210a 100644 --- a/server/src/mock/data.ts +++ b/server/src/mock/data.ts @@ -122,7 +122,11 @@ export function addSignedEmail(o: { which: keyof typeof SIGNED_MESSAGES; from: [ hasAttachment: false, preview: body.slice(0, 120), textBody: [{ partId: "1", blobId: textBlob, size: body.length, name: null, type: "text/plain", charset: "utf-8", disposition: null, cid: null }], - htmlBody: [], + // `htmlBody` is derived (RFC 8621 4.1.4): a message with no HTML + // alternative still gets one, holding the text/plain part. Checked against + // Stalwart 0.16.21 on 2026-09-10 -- see hasHtmlAlternative() in the client, + // which reads the part's type rather than trusting this list to be empty. + htmlBody: [{ partId: "1", blobId: textBlob, size: body.length, name: null, type: "text/plain", charset: "utf-8", disposition: null, cid: null }], attachments: [], bodyValues: { "1": { value: body, isEncodingProblem: false, isTruncated: false } }, bodyStructure: { @@ -192,7 +196,8 @@ export function addEmail(o: { from: [string, string]; to?: string; subject: stri from: [{ name: o.from[0], email: o.from[1] }], to: [{ name: "Demo User", email: o.to ?? USER }], cc: null, bcc: null, replyTo: null, sender: null, subject: o.subject, hasAttachment: Boolean(o.attach), preview: text.slice(0, 120).replace(/\n/g, " "), textBody: [{ partId: "1", blobId: textBlob, size: text.length, name: null, type: "text/plain", charset: "utf-8", disposition: null, cid: null }], - htmlBody: o.html ? [{ partId: "2", blobId: htmlBlob, size: (o.styled ? STYLED_MARKETING_HTML : html).length, name: null, type: "text/html", charset: "utf-8", disposition: null, cid: null }] : [], + // No HTML alternative means `htmlBody` names the text part, not nothing. See addSignedEmail. + htmlBody: o.html ? [{ partId: "2", blobId: htmlBlob, size: (o.styled ? STYLED_MARKETING_HTML : html).length, name: null, type: "text/html", charset: "utf-8", disposition: null, cid: null }] : [{ partId: "1", blobId: textBlob, size: text.length, name: null, type: "text/plain", charset: "utf-8", disposition: null, cid: null }], attachments, bodyValues: { "1": { value: text, isEncodingProblem: false, isTruncated: false }, ...(o.html ? { "2": { value: o.styled ? STYLED_MARKETING_HTML : html, isEncodingProblem: false, isTruncated: false } } : {}) }, bodyStructure: { partId: null, blobId: null, size: 0, type: "multipart/mixed", name: null, charset: null, disposition: null, cid: null, subParts: [{ partId: "1", blobId: textBlob, size: text.length, type: "text/plain", name: null, charset: "utf-8", disposition: null, cid: null }, ...(o.html ? [{ partId: "2", blobId: htmlBlob, size: (o.styled ? STYLED_MARKETING_HTML : html).length, type: "text/html", name: null, charset: "utf-8", disposition: null, cid: null }] : []), ...attachments] }, diff --git a/web/src/lib/mail/remoteImages.ts b/web/src/lib/mail/remoteImages.ts new file mode 100644 index 0000000..aac0181 --- /dev/null +++ b/web/src/lib/mail/remoteImages.ts @@ -0,0 +1,65 @@ +import { unproxiedImageUrl } from "@/lib/text/html"; +import type { ImagePolicy } from "@/store/settings"; + +/** + * Whether a message's remote images may be fetched. + * + * The reader's decision, in one place, because the composer has to make the + * same one. Quoting a message into a reply renders it again — and a quote that + * fetched what the reader had declined would report the message read, and the + * address live, to whoever was counting. The tracking pixel does not care + * which window it loaded in. + */ +export function remoteImagesAllowed(opts: { + from: string | null | undefined; + policy: ImagePolicy; + trusted: string[]; + inContacts: boolean; + /** The reader pressed "Show images" on this message. */ + shown: boolean; +}): boolean { + if (opts.shown || opts.policy === "always") return true; + if (opts.trusted.includes((opts.from ?? "").toLowerCase())) return true; + return opts.policy === "contacts" && opts.inContacts; +} + +/** + * Point proxied images back at their own addresses, on the way out. + * + * Reading a message fetches its remote images through this server, so the + * sender learns nothing about the reader. Those URLs belong to this + * deployment, so a quote that kept them would reach the recipient as images + * only this server can serve -- broken for them, and a beacon back here for + * anyone who could load them (#412). + */ +export function unproxyImages(html: string): string { + if (!html.includes("/api/image?url=")) return html; + const doc = new DOMParser().parseFromString(html, "text/html"); + for (const img of Array.from(doc.querySelectorAll("img[src]"))) { + const real = unproxiedImageUrl(img.getAttribute("src") ?? ""); + if (real) img.setAttribute("src", real); + } + return doc.body.innerHTML; +} + +/** + * Put back the addresses of images that were blocked when the message was + * quoted, on the way out. + * + * Blocking keeps the original URL on the element (`data-ihm-remote`), so + * nothing was lost by not fetching it. The copy that leaves here should be the + * quote as its sender wrote it: the recipient's client decides for itself + * whether to load those images, the same as it would have with any other + * client's reply. + */ +export function restoreBlockedImages(html: string): string { + if (!html.includes("data-ihm-blocked")) return html; + const doc = new DOMParser().parseFromString(html, "text/html"); + for (const img of Array.from(doc.querySelectorAll("img[data-ihm-blocked]"))) { + const url = img.getAttribute("data-ihm-remote"); + if (url) img.setAttribute("src", url); + img.removeAttribute("data-ihm-blocked"); + img.removeAttribute("data-ihm-remote"); + } + return doc.body.innerHTML; +} diff --git a/web/src/lib/mailbox/__tests__/folderOrder.test.ts b/web/src/lib/mailbox/__tests__/folderOrder.test.ts index c21cf0b..faee3bf 100644 --- a/web/src/lib/mailbox/__tests__/folderOrder.test.ts +++ b/web/src/lib/mailbox/__tests__/folderOrder.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "vitest"; -import { canPlaceFolder, compareFolders, neighbour, placeFolder, siblingsOf } from "../folderOrder"; +import { canPlaceFolder, compareFolders, neighbour, placeFolder, siblingsOf, treeOrder } from "../folderOrder"; import type { Id, Mailbox } from "@/jmap/types"; const RIGHTS = { mayRename: true, mayCreateChild: true } as Mailbox["myRights"]; @@ -119,3 +119,23 @@ describe("neighbour", () => { expect(neighbour(hidden, "alpha", "up", (m) => m.isSubscribed)).toEqual({ targetId: "junk", placement: "before" }); }); }); + +describe("treeOrder", () => { + const ids = (all: Record) => treeOrder(all).map((m) => m.id); + + it("lists the tree the way the sidebar does, each folder followed by its subfolders", () => { + expect(ids(fresh)).toEqual(["inbox", "drafts", "sent", "junk", "trash", "alpha", "work", "clients", "zeta"]); + }); + + it("follows a saved order rather than A–Z", () => { + // #1 on GitLab: the move-to picker kept the old order after the sidebar changed. + const ordered = apply(fresh, { zeta: { sortOrder: 10 }, sent: { sortOrder: 20 }, alpha: { sortOrder: 30 }, drafts: { sortOrder: 40 }, junk: { sortOrder: 50 }, trash: { sortOrder: 60 }, work: { sortOrder: 70 } }); + expect(ids(ordered)).toEqual(["inbox", "zeta", "sent", "alpha", "drafts", "junk", "trash", "work", "clients"]); + }); + + it("still lists a folder the walk from the top can't reach", () => { + const looped = apply(fresh, { work: { parentId: "clients" } }); + expect(ids(looped)).toHaveLength(Object.keys(looped).length); + expect(ids(looped)).toEqual(expect.arrayContaining(["work", "clients"])); + }); +}); diff --git a/web/src/lib/mailbox/folderOrder.ts b/web/src/lib/mailbox/folderOrder.ts index b70e05a..46b9b3a 100644 --- a/web/src/lib/mailbox/folderOrder.ts +++ b/web/src/lib/mailbox/folderOrder.ts @@ -25,6 +25,37 @@ function roleRank(m: Mailbox): number { return m.role && m.role in ROLE_ORDER ? ROLE_ORDER[m.role]! : Number.MAX_SAFE_INTEGER; } +/** + * Every folder, parents before their children and siblings in + * `compareFolders` order: the sidebar's order with every folder expanded. + * Lists that show all folders at once, like the move-to picker, use this so a + * folder sits where the user dragged it rather than where A–Z would put it. + * + * A folder the walk from the top never reaches (a parent loop the server + * should not allow) is appended rather than dropped, so it can still be + * picked. + */ +export function treeOrder(mailboxes: Record): Mailbox[] { + const byParent = new Map(); + for (const m of Object.values(mailboxes)) { + const p = m.parentId && mailboxes[m.parentId] ? m.parentId : null; + byParent.set(p, [...(byParent.get(p) ?? []), m]); + } + for (const list of byParent.values()) list.sort(compareFolders); + const out: Mailbox[] = []; + const seen = new Set(); + const walk = (parent: Id | null) => { + for (const m of byParent.get(parent) ?? []) { + if (seen.has(m.id)) continue; + seen.add(m.id); + out.push(m); + walk(m.id); + } + }; + walk(null); + return out.concat(Object.values(mailboxes).filter((m) => !seen.has(m.id)).sort(compareFolders)); +} + /** Every folder under `parentId` (null: the top level), in list order. */ export function siblingsOf(mailboxes: Record, parentId: Id | null): Mailbox[] { return Object.values(mailboxes) diff --git a/web/src/lib/text/html.ts b/web/src/lib/text/html.ts index bb0d9a4..a2eb1ed 100644 --- a/web/src/lib/text/html.ts +++ b/web/src/lib/text/html.ts @@ -1,5 +1,5 @@ import DOMPurify from "dompurify"; -import { withBase } from "@/lib/basePath"; +import { BASE_PATH, withBase } from "@/lib/basePath"; export interface SanitizeOptions { /** Map of Content-ID (without angle brackets) → URL for inline images. */ @@ -158,6 +158,23 @@ export function proxiedImageUrl(url: string): string { return withBase(`/api/image?url=${encodeURIComponent(url)}`); } +/** + * The address a proxied image really points at, or null if this is not one. + * + * A proxied URL is this server's, so it is right for reading a message and + * wrong for sending one: a quote left this way would hand the recipient + * images that only load from inside this deployment (#412). + */ +export function unproxiedImageUrl(src: string): string | null { + const path = `${BASE_PATH}/api/image?url=`; + if (!src.startsWith(path)) return null; + try { + return decodeURIComponent(src.slice(path.length)) || null; + } catch { + return null; // Malformed escape: leave it alone rather than mangle it. + } +} + export function sanitizeEmailHtml(input: string, opts: SanitizeOptions = {}): SanitizeResult { ensureHooks(); let bodyStyle = ""; diff --git a/web/src/locales/de.ts b/web/src/locales/de.ts index db9fd98..3ada0e7 100644 --- a/web/src/locales/de.ts +++ b/web/src/locales/de.ts @@ -1406,6 +1406,8 @@ export const catalog: Catalog = { "Collapse all": "Alle einklappen", "Expand all": "Alle ausklappen", "Send now instead": "Stattdessen jetzt senden", + "This message is rich text": "Diese Nachricht ist formatierter Text", + "This message is plain text": "Diese Nachricht ist Nur-Text", "Switch to plain text": "Zu Nur-Text wechseln", "Switch to rich text": "Zu formatiertem Text wechseln", "{used} of {total} used": "{used} von {total} belegt", diff --git a/web/src/locales/es.ts b/web/src/locales/es.ts index 42b05f6..19b32ce 100644 --- a/web/src/locales/es.ts +++ b/web/src/locales/es.ts @@ -1379,6 +1379,8 @@ export const catalog: Catalog = { "Collapse all": "Contraer todo", "Expand all": "Expandir todo", "Send now instead": "Enviar ahora, sin programar", + "This message is rich text": "Este mensaje es texto enriquecido", + "This message is plain text": "Este mensaje es texto sin formato", "Switch to plain text": "Cambiar a texto sin formato", "Switch to rich text": "Cambiar a texto enriquecido", "{used} of {total} used": "{used} de {total} usados", diff --git a/web/src/locales/fr.ts b/web/src/locales/fr.ts index b8f8b13..3a37a94 100644 --- a/web/src/locales/fr.ts +++ b/web/src/locales/fr.ts @@ -1384,6 +1384,8 @@ export const catalog: Catalog = { "Collapse all": "Tout réduire", "Expand all": "Tout développer", "Send now instead": "Envoyer tout de suite", + "This message is rich text": "Ce message est en texte enrichi", + "This message is plain text": "Ce message est en texte brut", "Switch to plain text": "Passer en texte brut", "Switch to rich text": "Passer en texte enrichi", "{used} of {total} used": "{used} sur {total} utilisés", diff --git a/web/src/locales/ja.ts b/web/src/locales/ja.ts index 1c59378..517af6d 100644 --- a/web/src/locales/ja.ts +++ b/web/src/locales/ja.ts @@ -1387,6 +1387,8 @@ export const catalog: Catalog = { "Collapse all": "すべて折りたたむ", "Expand all": "すべて展開", "Send now instead": "予約をやめて今すぐ送信", + "This message is rich text": "このメールはリッチテキストです", + "This message is plain text": "このメールはプレーンテキストです", "Switch to plain text": "プレーンテキストに切り替え", "Switch to rich text": "リッチテキストに切り替え", "{used} of {total} used": "{total} 中 {used} を使用", diff --git a/web/src/locales/nl.ts b/web/src/locales/nl.ts index f619711..789f5b4 100644 --- a/web/src/locales/nl.ts +++ b/web/src/locales/nl.ts @@ -1379,6 +1379,8 @@ export const catalog: Catalog = { "Collapse all": "Alles samenvouwen", "Expand all": "Alles uitvouwen", "Send now instead": "Toch nu verzenden", + "This message is rich text": "Dit bericht is opgemaakte tekst", + "This message is plain text": "Dit bericht is platte tekst", "Switch to plain text": "Overschakelen naar platte tekst", "Switch to rich text": "Overschakelen naar opgemaakte tekst", "{used} of {total} used": "{used} van {total} gebruikt", diff --git a/web/src/locales/pt-BR.ts b/web/src/locales/pt-BR.ts index d9de982..c396e48 100644 --- a/web/src/locales/pt-BR.ts +++ b/web/src/locales/pt-BR.ts @@ -1382,6 +1382,8 @@ export const catalog: Catalog = { "Collapse all": "Recolher tudo", "Expand all": "Expandir tudo", "Send now instead": "Enviar agora mesmo", + "This message is rich text": "Esta mensagem está em texto formatado", + "This message is plain text": "Esta mensagem está em texto simples", "Switch to plain text": "Mudar para texto simples", "Switch to rich text": "Mudar para texto formatado", "{used} of {total} used": "{used} de {total} usados", diff --git a/web/src/locales/ru.ts b/web/src/locales/ru.ts index f645f1b..f4bbc00 100644 --- a/web/src/locales/ru.ts +++ b/web/src/locales/ru.ts @@ -1381,6 +1381,8 @@ export const catalog: Catalog = { "Collapse all": "Свернуть все", "Expand all": "Развернуть все", "Send now instead": "Отправить сейчас", + "This message is rich text": "Это письмо в формате HTML", + "This message is plain text": "Это письмо в виде простого текста", "Switch to plain text": "Переключиться на обычный текст", "Switch to rich text": "Переключиться на форматированный текст", "{used} of {total} used": "Использовано {used} из {total}", diff --git a/web/src/locales/uk.ts b/web/src/locales/uk.ts index 4b335c3..e1b615c 100644 --- a/web/src/locales/uk.ts +++ b/web/src/locales/uk.ts @@ -1375,6 +1375,8 @@ export const catalog: Catalog = { "Collapse all": "Згорнути все", "Expand all": "Розгорнути все", "Send now instead": "Надіслати зараз", + "This message is rich text": "Цей лист у форматі HTML", + "This message is plain text": "Цей лист у вигляді простого тексту", "Switch to plain text": "Перейти на звичайний текст", "Switch to rich text": "Перейти на форматований текст", "{used} of {total} used": "Використано {used} з {total}", diff --git a/web/src/locales/zh-Hans.ts b/web/src/locales/zh-Hans.ts index 628971c..85aada8 100644 --- a/web/src/locales/zh-Hans.ts +++ b/web/src/locales/zh-Hans.ts @@ -1386,6 +1386,8 @@ export const catalog: Catalog = { "Collapse all": "全部折叠", "Expand all": "全部展开", "Send now instead": "改为立即发送", + "This message is rich text": "这封邮件是富文本", + "This message is plain text": "这封邮件是纯文本", "Switch to plain text": "切换为纯文本", "Switch to rich text": "切换为富文本", "{used} of {total} used": "已使用 {used},共 {total}", diff --git a/web/src/store/__tests__/compose-email.test.ts b/web/src/store/__tests__/compose-email.test.ts index 44a46fa..1b574a4 100644 --- a/web/src/store/__tests__/compose-email.test.ts +++ b/web/src/store/__tests__/compose-email.test.ts @@ -18,7 +18,7 @@ function draft(over: Partial = {}): Draft { requestReceipt: false, priority: "normal", showCc: false, showBcc: false, showReplyTo: false, minimized: false, maximized: false, dirty: false, savedAt: null, - saving: false, sending: false, error: null, signatureHtml: "", replyMode: null, sendAt: null, + saving: false, sending: false, error: null, signatureHtml: "", replyMode: null, quoteHtml: "", quoteText: "", formatOffer: null, sendAt: null, ...over, }; } diff --git a/web/src/store/__tests__/quote-image-policy.test.ts b/web/src/store/__tests__/quote-image-policy.test.ts new file mode 100644 index 0000000..fb92444 --- /dev/null +++ b/web/src/store/__tests__/quote-image-policy.test.ts @@ -0,0 +1,98 @@ +import { beforeEach, describe, expect, it } from "vitest"; +import { buildEmailObject, useCompose } from "@/store/compose"; +import { useMail } from "@/store/mail"; +import { useContacts } from "@/store/contacts"; +import { DEFAULT_SETTINGS, useSettings } from "@/store/settings"; +import type { Email, EmailAddress, Identity } from "@/jmap/types"; + +/* + * Remote images in a quoted message (#410). + * + * Quoting renders the message a second time. The reply was fetching every + * remote image in it, whatever the reader had decided — so replying to a + * message whose images had been left blocked told the tracker the mail was + * read and the address live. The composer is a window like any other. + */ + +const PIXEL = "https://tracker.example/open.gif?id=42"; + +const MESSAGE = { + id: "m1", messageId: [""], subject: "Sale", references: [], inReplyTo: [], keywords: {}, + attachments: [], receivedAt: "2026-09-04T10:00:00Z", mailboxIds: {}, + from: [{ name: "Shop", email: "shop@example.com" }], to: [{ name: "John", email: "john@example.org" }], cc: [], + htmlBody: [{ partId: "2", type: "text/html" }], + textBody: [{ partId: "1", type: "text/plain" }], + bodyValues: { + "1": { value: "Sale on now", isEncodingProblem: false, isTruncated: false }, + "2": { value: `

Sale on now

`, isEncodingProblem: false, isTruncated: false }, + }, +} as unknown as Email; + +const IDENTITIES = [{ id: "i1", name: "John", email: "john@example.org", replyTo: null }] as unknown as Identity[]; + +/** + * Whether the draft will actually load the image. Allowed images go through + * the server's proxy where the deployment has one (#412), so the address is + * escaped inside an `/api/image` URL rather than sitting in `src` as it is. + */ +const fetched = (html: string) => html.includes(`/api/image?url=${encodeURIComponent(PIXEL)}`) || html.includes(`src="${PIXEL}"`); + +function replyDraft() { + useMail.setState({ + accountId: "a1", + identities: IDENTITIES as never, + getEmails: (async () => [MESSAGE]) as never, + defaultIdentity: (() => IDENTITIES[0]) as never, + loadIdentities: (async () => IDENTITIES) as never, + roleId: (() => null) as never, + }); + return useCompose.getState().reply(MESSAGE, "reply").then((key) => useCompose.getState().drafts.find((d) => d.key === key)!); +} + +beforeEach(() => { + useCompose.setState({ drafts: [], activeKey: null, pendingSends: {} }); + useMail.setState({ imagesShown: {} }); + useContacts.setState({ loaded: false } as never); + useSettings.setState({ settings: { ...DEFAULT_SETTINGS, imagePolicy: "ask", composeFormat: "html" } }); +}); + +describe("quoting a message whose images were not allowed", () => { + it("does not put a fetchable address in the draft", async () => { + const d = await replyDraft(); + expect(d.html).not.toContain(PIXEL.split("?")[0]! + '"'); + expect(d.html).toContain("data-ihm-blocked"); + // The src is what the browser would fetch; nothing else in the draft is. + expect(/]+src="https:/.test(d.html)).toBe(false); + }); + + it("keeps the address, so the sent copy is the quote as it was written", async () => { + const d = await replyDraft(); + expect(d.html).toContain(PIXEL); + const email = await buildEmailObject({ ...d, to: [{ name: null, email: "shop@example.com" }] as EmailAddress[] }, { forSend: true }); + const sent = JSON.stringify(email); + expect(sent).toContain(PIXEL); + expect(sent).not.toContain("data-ihm-blocked"); + }); + + it("fetches them once the reader has shown images on that message", async () => { + useMail.setState({ imagesShown: { m1: true } }); + const d = await replyDraft(); + expect(fetched(d.html)).toBe(true); + expect(d.html).not.toContain("data-ihm-blocked"); + }); + + it("fetches them when the policy is to show images always", async () => { + useSettings.setState((s) => ({ settings: { ...s.settings, imagePolicy: "always" } })); + expect(fetched((await replyDraft()).html)).toBe(true); + }); + + it("fetches them from a sender the reader trusts", async () => { + useSettings.setState((s) => ({ settings: { ...s.settings, trustedImageSenders: ["shop@example.com"] } })); + expect(fetched((await replyDraft()).html)).toBe(true); + }); + + it("leaves them blocked for a stranger when the policy is contacts only", async () => { + useSettings.setState((s) => ({ settings: { ...s.settings, imagePolicy: "contacts" } })); + expect((await replyDraft()).html).toContain("data-ihm-blocked"); + }); +}); diff --git a/web/src/store/__tests__/quote-image-proxy.test.ts b/web/src/store/__tests__/quote-image-proxy.test.ts new file mode 100644 index 0000000..0c21079 --- /dev/null +++ b/web/src/store/__tests__/quote-image-proxy.test.ts @@ -0,0 +1,99 @@ +import { beforeEach, describe, expect, it } from "vitest"; +import { buildEmailObject, useCompose } from "@/store/compose"; +import { useMail } from "@/store/mail"; +import { useContacts } from "@/store/contacts"; +import { useSession } from "@/store/session"; +import { DEFAULT_SETTINGS, useSettings } from "@/store/settings"; +import { unproxyImages } from "@/lib/mail/remoteImages"; +import type { Email, EmailAddress, Identity } from "@/jmap/types"; + +/* + * Remote images in a quote go through this server, and come back out pointing + * at their own addresses (#412). + * + * Reading a message proxies its images so the sender learns nothing about the + * reader. Quoting fetched them directly, which handed the same pixel the + * reader's IP and user agent. Proxying the quote is only half of it: those + * URLs belong to this deployment, so the copy that is sent has to carry the + * originals or the recipient gets images only this server can serve. + */ + +const IMAGE = "https://cdn.example/banner.png?id=7"; + +const MESSAGE = { + id: "m1", messageId: [""], subject: "Sale", references: [], inReplyTo: [], keywords: {}, + attachments: [], receivedAt: "2026-09-04T10:00:00Z", mailboxIds: {}, + from: [{ name: "Shop", email: "shop@example.com" }], to: [{ name: "John", email: "john@example.org" }], cc: [], + htmlBody: [{ partId: "2", type: "text/html" }], + textBody: [{ partId: "1", type: "text/plain" }], + bodyValues: { + "1": { value: "Sale on now", isEncodingProblem: false, isTruncated: false }, + "2": { value: `

Sale

`, isEncodingProblem: false, isTruncated: false }, + }, +} as unknown as Email; + +const IDENTITIES = [{ id: "i1", name: "John", email: "john@example.org", replyTo: null }] as unknown as Identity[]; + +function draftFor(mode: "reply" | "forward") { + useMail.setState({ + accountId: "a1", + identities: IDENTITIES as never, + getEmails: (async () => [MESSAGE]) as never, + defaultIdentity: (() => IDENTITIES[0]) as never, + loadIdentities: (async () => IDENTITIES) as never, + roleId: (() => null) as never, + }); + return useCompose.getState().reply(MESSAGE, mode).then((key) => useCompose.getState().drafts.find((d) => d.key === key)!); +} + +const proxy = (on: boolean) => useSession.setState({ session: { ihasmail: { imageProxy: on } } } as never); + +beforeEach(() => { + useCompose.setState({ drafts: [], activeKey: null, pendingSends: {} }); + useMail.setState({ imagesShown: {} }); + useContacts.setState({ loaded: false } as never); + // Images allowed, so the question is only how they are fetched. + useSettings.setState({ settings: { ...DEFAULT_SETTINGS, imagePolicy: "always", composeFormat: "html" } }); + proxy(true); +}); + +describe("images in a quote, while the reply is being written", () => { + it("are fetched through this server, as reading the message does", async () => { + const d = await draftFor("reply"); + expect(d.html).toContain("/api/image?url="); + expect(d.html).not.toContain(`src="${IMAGE}"`); + }); + + it("are fetched directly where the deployment has no proxy", async () => { + proxy(false); + const d = await draftFor("reply"); + expect(d.html).toContain(`src="${IMAGE}"`); + expect(d.html).not.toContain("/api/image?url="); + }); + + it("go through it on a forward too", async () => { + expect((await draftFor("forward")).html).toContain("/api/image?url="); + }); +}); + +describe("the copy that is sent", () => { + it("points at the image's own address, not at this server", async () => { + const d = await draftFor("reply"); + const sent = JSON.stringify(await buildEmailObject({ ...d, to: [{ name: null, email: "shop@example.com" }] as EmailAddress[] }, { forSend: true })); + expect(sent).toContain(IMAGE.replace(/&/g, "&")); + expect(sent).not.toContain("/api/image?url="); + }); + + it("restores a signature or template image that used the proxy as well", () => { + const logo = "https://cdn.example/logo.png"; + const html = `

Regards

`; + const out = unproxyImages(html); + expect(out).toContain(`src="${logo}"`); + expect(out).toContain('src="cid:x@1"'); + }); + + it("leaves everything else alone", () => { + const html = 'link'; + expect(unproxyImages(html)).toBe(html); + }); +}); diff --git a/web/src/store/__tests__/reply-addressing.test.ts b/web/src/store/__tests__/reply-addressing.test.ts index 437aac8..d3ee2bb 100644 --- a/web/src/store/__tests__/reply-addressing.test.ts +++ b/web/src/store/__tests__/reply-addressing.test.ts @@ -153,6 +153,30 @@ describe("a message of mine with nobody obvious to reply to", () => { const d = await draftFor({ ...MINE, to: [ME], cc: [] } as Email, "reply"); expect(addrs(d.to)).toEqual([ME.email]); }); + + it("answers the Reply-To rather than my own desk when nobody else is on it", async () => { + /* + * A contact form: the site mails itself, From and To both its own address, + * and the person who filled the form in is in Reply-To. From alone makes + * this look like mine, and the fallback used to reply to me (#415). + */ + const form = { ...MINE, to: [ME], cc: [], replyTo: [{ name: "Michael", email: "michael@example.com" }] } as Email; + const d = await draftFor(form, "reply"); + expect(addrs(d.to)).toEqual(["michael@example.com"]); + }); + + it("does the same on a reply all, without cc-ing myself", async () => { + const form = { ...MINE, to: [ME], cc: [], replyTo: [{ name: "Michael", email: "michael@example.com" }] } as Email; + const d = await draftFor(form, "replyAll"); + expect(addrs(d.to)).toEqual(["michael@example.com"]); + expect(d.cc).toEqual([]); + }); + + it("still prefers somebody I actually wrote to over my own Reply-To", async () => { + // The Cc is a person; the Reply-To is where answers to me belong. + const d = await draftFor({ ...MINE, to: [ME], replyTo: [{ name: null, email: "desk@example.org" }] } as Email, "reply"); + expect(addrs(d.to)).toEqual([BOB.email]); + }); }); describe("forwarding", () => { diff --git a/web/src/store/__tests__/reply-format-offer.test.ts b/web/src/store/__tests__/reply-format-offer.test.ts new file mode 100644 index 0000000..608d5d6 --- /dev/null +++ b/web/src/store/__tests__/reply-format-offer.test.ts @@ -0,0 +1,124 @@ +import { beforeEach, describe, expect, it } from "vitest"; +import { useCompose } from "@/store/compose"; +import { useMail } from "@/store/mail"; +import { DEFAULT_SETTINGS, useSettings } from "@/store/settings"; +import type { Email, Identity } from "@/jmap/types"; + +/* + * Offering to answer a message in the format it was written in (#407). + * + * The trap is `htmlBody`: RFC 8621 derives it, so a plain-text message has one + * too, holding its text/plain part. Reading that as "there is HTML" would + * offer a switch to rich text on every plain-text message, and never offer the + * switch to plain text where it is actually wanted. + */ + +const base = { + messageId: [""], subject: "Numbers", references: [], inReplyTo: [], + keywords: {}, attachments: [], receivedAt: "2026-09-04T10:00:00Z", mailboxIds: {}, + from: [{ name: "Ann", email: "ann@example.com" }], to: [{ name: "John", email: "john@example.org" }], cc: [], +}; + +/** A real multipart/alternative: two parts, one of them text/html. */ +const RICH = { + ...base, id: "m1", + htmlBody: [{ partId: "2", type: "text/html" }], + textBody: [{ partId: "1", type: "text/plain" }], + bodyValues: { "1": { value: "hi", isEncodingProblem: false, isTruncated: false }, "2": { value: "

hi

", isEncodingProblem: false, isTruncated: false } }, +} as unknown as Email; + +/** Plain text, as Stalwart returns it: both lists name the same text/plain part. */ +const PLAIN = { + ...base, id: "m2", + htmlBody: [{ partId: "1", type: "text/plain" }], + textBody: [{ partId: "1", type: "text/plain" }], + bodyValues: { "1": { value: "hi", isEncodingProblem: false, isTruncated: false } }, +} as unknown as Email; + +const IDENTITIES = [{ id: "i1", name: "John", email: "john@example.org", replyTo: null }] as unknown as Identity[]; + +function draftFor(email: Email, mode: "reply" | "replyAll" | "forward") { + useMail.setState({ + accountId: "a1", + identities: IDENTITIES as never, + getEmails: (async () => [email]) as never, + defaultIdentity: (() => IDENTITIES[0]) as never, + loadIdentities: (async () => IDENTITIES) as never, + roleId: (() => null) as never, + }); + return useCompose.getState().reply(email, mode).then((key) => useCompose.getState().drafts.find((d) => d.key === key)!); +} + +const composeIn = (format: "html" | "text") => useSettings.setState({ settings: { ...DEFAULT_SETTINGS, composeFormat: format } }); + +beforeEach(() => { + useCompose.setState({ drafts: [], activeKey: null }); + useSettings.setState({ settings: { ...DEFAULT_SETTINGS } }); +}); + +describe("answering a message written in the other format", () => { + it("offers rich text when a plain-text reply answers a rich message", async () => { + composeIn("text"); + const d = await draftFor(RICH, "reply"); + expect(d.format).toBe("text"); + expect(d.formatOffer).toBe("html"); + }); + + it("offers plain text when a rich reply answers a plain-text message", async () => { + composeIn("html"); + const d = await draftFor(PLAIN, "reply"); + expect(d.format).toBe("html"); + expect(d.formatOffer).toBe("text"); + }); + + it("offers nothing when the formats already agree", async () => { + composeIn("html"); + expect((await draftFor(RICH, "reply")).formatOffer).toBeNull(); + composeIn("text"); + expect((await draftFor(PLAIN, "reply")).formatOffer).toBeNull(); + }); + + it("reads the part's own type, not the derived htmlBody list", async () => { + // PLAIN has an htmlBody; it names the text/plain part. Offering a switch + // to rich text here would fire on every plain-text message there is. + composeIn("text"); + expect((await draftFor(PLAIN, "reply")).formatOffer).toBeNull(); + }); + + it("offers on a reply all and on a forward, where the same formatting is lost", async () => { + composeIn("text"); + expect((await draftFor(RICH, "replyAll")).formatOffer).toBe("html"); + expect((await draftFor(RICH, "forward")).formatOffer).toBe("html"); + }); + + it("carries both bodies either way, so switching has something to switch to", async () => { + composeIn("text"); + const d = await draftFor(RICH, "reply"); + expect(d.text).toContain("hi"); + expect(d.html).toContain("hi"); + }); + + it("keeps the quoted message in both formats, so a switch can restore it", async () => { + composeIn("text"); + const d = await draftFor(RICH, "reply"); + // The HTML quote is the original's markup, not the text one converted. + expect(d.quoteHtml).toContain("

hi

"); + expect(d.quoteHtml).toContain("ihm-quote"); + expect(d.quoteText).toContain("Ann"); + expect(d.text.endsWith(d.quoteText)).toBe(true); + }); + + it("quotes nothing on a message started from scratch", () => { + composeIn("text"); + const key = useCompose.getState().open(); + const d = useCompose.getState().drafts.find((x) => x.key === key)!; + expect(d.quoteHtml).toBe(""); + expect(d.quoteText).toBe(""); + }); + + it("makes no offer on a message started from scratch", () => { + composeIn("text"); + const key = useCompose.getState().open(); + expect(useCompose.getState().drafts.find((x) => x.key === key)!.formatOffer).toBeNull(); + }); +}); diff --git a/web/src/store/compose.ts b/web/src/store/compose.ts index 91a5a8b..d48776d 100644 --- a/web/src/store/compose.ts +++ b/web/src/store/compose.ts @@ -4,7 +4,8 @@ import type { Email, EmailAddress, EmailBodyPart, Id, Identity, SetResponse } fr import { formatFullDate, uid } from "@/lib/format"; import { formatAddress, parseMailto, sameAddress, uniqueAddresses } from "@/lib/address"; import { escapeHtml, htmlToText, quoteText, replySubject, textToHtml } from "@/lib/text/text"; -import { sanitizeEmailHtml, sanitizeEditorHtml } from "@/lib/text/html"; +import { hasHtmlAlternative, sanitizeEmailHtml, sanitizeEditorHtml } from "@/lib/text/html"; +import { remoteImagesAllowed, restoreBlockedImages, unproxyImages } from "@/lib/mail/remoteImages"; import { toast } from "@/ui/toast"; import { useMail, FULL_PROPS, BODY_PROPS } from "./mail"; import { useSession } from "./session"; @@ -13,6 +14,7 @@ import { formatScheduleTime, holdUntil } from "@/lib/schedule"; import { t as translate } from "@/lib/i18n"; import { BASE_PATH } from "@/lib/basePath"; import { settings } from "./settings"; +import { useContacts } from "./contacts"; import { emlFilename } from "@/lib/text/emlName"; import { fillPlaceholders, type PlaceholderContext } from "@/lib/templatePlaceholders"; import { shareBody, type SharedContent } from "@/lib/shareTarget"; @@ -76,6 +78,20 @@ export interface Draft { /** Original identity signature HTML currently embedded, to replace on identity switch. */ signatureHtml: string; replyMode: "reply" | "replyAll" | "forward" | null; + /** + * The quoted message as it was prepared in each format, kept so that + * switching format re-attaches the original rather than a conversion of + * whatever the other format flattened it into. Empty on a draft that quotes + * nothing. + */ + quoteHtml: string; + quoteText: string; + /** + * The format the message being answered was written in, when it is not the + * one this draft opened in (#407). The composer offers the switch; answering + * it either way, or dismissing it, clears this. + */ + formatOffer: "html" | "text" | null; mailboxIdOnSend?: Id | null; /** When set, hand the message to the server held until this instant. */ sendAt: number | null; @@ -146,11 +162,39 @@ function blankDraft(init: Partial = {}): Draft { error: null, signatureHtml: "", replyMode: null, + quoteHtml: "", + quoteText: "", + formatOffer: null, sendAt: null, ...init, }; } +/** + * Whether this message's remote images may be fetched into a composer. + * + * The same question the reader answered, asked with the same inputs: the + * policy, the trusted senders, whether the sender is a contact, and whether + * the reader pressed "Show images" on this message. + */ +/** Whether this deployment fetches remote images through its own server. */ +function imageProxyOn(): boolean { + return useSession.getState().session?.ihasmail?.imageProxy ?? true; +} + +function remoteImagesForMessage(email: Email): boolean { + const s = settings(); + const from = email.from?.[0]?.email; + const contacts = useContacts.getState(); + return remoteImagesAllowed({ + from, + policy: s.imagePolicy, + trusted: s.trustedImageSenders, + inContacts: Boolean(from && contacts.loaded && contacts.lookupByEmail(from)), + shown: Boolean(useMail.getState().imagesShown[email.id]), + }); +} + export function signatureBlock(identity: Identity | undefined, format: "html" | "text"): string { if (!identity) return ""; if (format === "text") return identity.textSignature ? `\n\n-- \n${identity.textSignature}` : ""; @@ -267,7 +311,7 @@ export const useCompose = create((set, get) => ({ showCc: Boolean(full.cc?.length), showBcc: Boolean(full.bcc?.length), subject: full.subject ?? "", - html: html ? sanitizeEmailHtml(html, { cidMap, allowRemote: true, dropStyleBlocks: true }).html : textToHtml(text).replace(/\n/g, "
"), + html: html ? sanitizeEmailHtml(html, { cidMap, allowRemote: remoteImagesForMessage(full), proxyRemote: imageProxyOn(), dropStyleBlocks: true }).html : textToHtml(text).replace(/\n/g, "
"), text: text || (html ? htmlToText(html) : ""), format: html ? "html" : settings().composeFormat, attachments, @@ -334,7 +378,7 @@ export const useCompose = create((set, get) => ({ showCc: Boolean(full.cc?.length), showBcc: Boolean(full.bcc?.length), subject: full.subject ?? "", - html: html ? sanitizeEmailHtml(html, { cidMap, allowRemote: true, dropStyleBlocks: true }).html : textToHtml(text).replace(/\n/g, "
"), + html: html ? sanitizeEmailHtml(html, { cidMap, allowRemote: remoteImagesForMessage(full), proxyRemote: imageProxyOn(), dropStyleBlocks: true }).html : textToHtml(text).replace(/\n/g, "
"), text: text || (html ? htmlToText(html) : ""), format: html ? "html" : settings().composeFormat, attachments, @@ -392,6 +436,19 @@ export const useCompose = create((set, get) => ({ // Addressed only to myself, or only in Cc: there is still somebody this // is a reply to, and an empty To is not it. if (!to.length) { to = cc.length ? cc : withoutOwn(full.cc ?? []); cc = []; } + /* + * Nobody but me on the message, and a Reply-To pointing somewhere that + * is not mine: that address is who this is really from. + * + * A contact form is the shape of it -- From and To are both the site's + * own mailbox, and the person who filled the form in is in Reply-To. + * The address test above calls that mine, correctly as far as it goes, + * and the fallback then addressed the reply to my own desk (#415). + * + * After the Cc, not before it: a message I really did send carries my + * own Reply-To, and somebody I actually wrote to beats it. + */ + if (!to.length) to = withoutOwn(full.replyTo ?? []); if (!to.length) to = uniqueAddresses([...(full.to ?? []), ...(full.cc ?? [])]); } else { to = uniqueAddresses(full.replyTo?.length ? full.replyTo : (full.from ?? [])); @@ -405,6 +462,13 @@ export const useCompose = create((set, get) => ({ const textPart = full.textBody?.[0]; const origHtml = htmlPart?.partId ? (full.bodyValues?.[htmlPart.partId]?.value ?? "") : ""; const origText = textPart?.partId ? (full.bodyValues?.[textPart.partId]?.value ?? "") : ""; + /* + * What the message being answered was really written in. `htmlBody` is + * derived, so its presence proves nothing -- hasHtmlAlternative() reads the + * part's own type. Getting this wrong would offer every plain-text message + * a switch to rich text it does not need. + */ + const origFormat = hasHtmlAlternative(htmlPart, origHtml) ? "html" : "text"; const accountId = mail.accountId!; const attachments: ComposeAttachment[] = []; const cidMap: Record = {}; @@ -415,9 +479,21 @@ export const useCompose = create((set, get) => ({ attachments.push({ id: uid("a"), name: a.name ?? "attachment", type: a.type, size: a.size, blobId: a.blobId, progress: 100, error: null, cid: a.cid ?? undefined, inline }); } } + /* + * Quoting renders the message a second time, so the reader's decision + * about its remote images applies here too: a quote that fetched what + * they declined would report the message read to whoever was counting + * (#410). Blocked images keep their address and get it back on the way + * out, so the recipient's copy is the quote as its sender wrote it. + */ + const allowRemote = remoteImagesForMessage(full); + // Fetched through this server while the reply is written, as reading the + // message does, and pointed back at their own addresses on the way out + // (#412). + const proxyRemote = imageProxyOn(); // Inline images are shown via their blob URLs in the editor and converted back to cid: at send time. const quotedHtmlBody = origHtml - ? sanitizeEmailHtml(origHtml, { cidMap, allowRemote: true, proxyRemote: false, dropStyleBlocks: true }).html + ? sanitizeEmailHtml(origHtml, { cidMap, allowRemote, proxyRemote, dropStyleBlocks: true }).html : textToHtml(origText).replace(/\n/g, "
"); const fromStr = escapeHtml((full.from ?? []).map(formatAddress).join(", ")); const date = formatFullDate(full.receivedAt); @@ -460,6 +536,9 @@ export const useCompose = create((set, get) => ({ relatedKeyword: mode === "forward" ? "$forwarded" : "$answered", signatureHtml: sigHtml, replyMode: mode, + quoteHtml, + quoteText: quoteTxt, + formatOffer: origFormat === s.composeFormat ? null : origFormat, }); set((st) => ({ drafts: [...st.drafts, d], activeKey: d.key })); return d.key; @@ -781,7 +860,9 @@ export async function buildEmailObject(d: Draft, opts: { forSend: boolean; mailb if (!ident) throw new Error(translate("No sending identity available")); const from: EmailAddress = { name: ident.name || null, email: ident.email }; - let html = d.format === "html" ? d.html : ""; + // Images blocked when the message was quoted keep their address; the copy + // that leaves carries it, and the recipient's client decides for itself. + let html = d.format === "html" ? unproxyImages(restoreBlockedImages(d.html)) : ""; const text = d.format === "html" ? htmlToText(d.html) : d.text; // Inline attachments shown via blob URLs in the editor → back to cid: references. diff --git a/web/src/store/mail/index.ts b/web/src/store/mail/index.ts index 7e71131..6a71dcb 100644 --- a/web/src/store/mail/index.ts +++ b/web/src/store/mail/index.ts @@ -78,6 +78,7 @@ function offerArchiveFolder(retry: () => Promise): void { export const useMail = create((set, get) => ({ accountId: null, + imagesShown: {}, mailboxes: {}, mailboxState: null, mailboxesLoaded: false, @@ -875,6 +876,10 @@ export const useMail = create((set, get) => ({ } }, + showImages(id) { + set((s) => ({ imagesShown: { ...s.imagesShown, [id]: true } })); + }, + select(ids, on) { set((s) => { const next = { ...s.selected }; diff --git a/web/src/store/mail/types.ts b/web/src/store/mail/types.ts index 751a677..c8a65aa 100644 --- a/web/src/store/mail/types.ts +++ b/web/src/store/mail/types.ts @@ -33,6 +33,12 @@ export interface ListState extends ListQuery { export interface MailState { accountId: Id | null; + /** + * Messages the reader pressed "Show images" on, this session. Kept here + * rather than in the message view because replying quotes the message into + * a second window, which has to honour the same decision. + */ + imagesShown: Record; mailboxes: Record; mailboxState: string | null; mailboxesLoaded: boolean; @@ -113,6 +119,8 @@ export interface MailState { saveVacation(patch: Partial): Promise; loadQuota(): Promise; + /** Remember that this message's remote images were allowed by hand. */ + showImages(id: Id): void; select(ids: Id[], on: boolean): void; clearSelection(): void; /** Refresh the per-label unread counts, in one request. */ diff --git a/web/src/styles/app.css b/web/src/styles/app.css index 8681f41..55a5eef 100644 --- a/web/src/styles/app.css +++ b/web/src/styles/app.css @@ -1584,6 +1584,9 @@ a.menu-item:hover { color: var(--fg); } .composer-head .title { flex: 1; font-weight: 600; white-space: nowrap; overflow: hidden; text-overflow: ellipsis; } .composer-head .status { color: var(--fg-faint); font-size: .8em; margin-right: 6px; white-space: nowrap; } .composer-body { display: flex; flex-direction: column; flex: 1; min-height: 0; } +/* An offer the draft makes about itself, above the editor: quiet, one line, and dismissible. */ +.composer-notice { display: flex; align-items: center; gap: 8px; padding: 6px 10px 6px 14px; background: var(--accent-soft); color: var(--accent-soft-fg); border-bottom: 1px solid var(--border); font-size: .85em; flex: 0 0 auto; } +.composer-notice span { flex: 1; min-width: 0; overflow: hidden; text-overflow: ellipsis; white-space: nowrap; } .composer-fields { flex: 0 0 auto; padding: 0 12px; } .composer-field { display: flex; align-items: center; gap: 8px; min-height: 40px; border-bottom: 1px solid var(--border); padding: 4px 0; } .composer-field > label { color: var(--fg-muted); width: 42px; flex: 0 0 auto; font-size: .92em; } diff --git a/web/src/views/compose/Composer.tsx b/web/src/views/compose/Composer.tsx index f81b5e4..a08f4f7 100644 --- a/web/src/views/compose/Composer.tsx +++ b/web/src/views/compose/Composer.tsx @@ -147,11 +147,26 @@ export function Composer({ draft }: { draft: Draft }) { patch({ sendAt: at.getTime() }); }; + /* + * Switching format converts what has been written, but the quoted message + * is not something this draft wrote: it was prepared in both formats when + * the reply opened. Converting the plain-text quote into HTML would hand + * back a flattened copy of a message that still exists in its original + * markup, so re-attach that instead, and keep only what the author typed + * above it. Where the quote can no longer be found -- edited, or a draft + * that quotes nothing -- convert the whole body as before. + */ const toggleFormat = () => { + // Whichever way the format is changed, the offer has been answered. if (d.format === "html") { - patch({ format: "text", text: htmlToText(d.html) }); + const at = d.quoteHtml ? d.html.indexOf('
') : -1; + const written = at >= 0 ? htmlToText(d.html.slice(0, at)) : htmlToText(d.html); + patch({ format: "text", text: at >= 0 ? written.replace(/\s+$/, "") + d.quoteText : written, formatOffer: null }); } else { - patch({ format: "html", html: textToHtml(d.text, { linkify: false, quoteColors: false }).replace(/\n/g, "
") }); + const keeps = Boolean(d.quoteText) && d.text.endsWith(d.quoteText); + const written = keeps ? d.text.slice(0, d.text.length - d.quoteText.length) : d.text; + const asHtml = textToHtml(written, { linkify: false, quoteColors: false }).replace(/\n/g, "
"); + patch({ format: "html", html: keeps ? asHtml + d.quoteHtml : asHtml, formatOffer: null }); } }; @@ -261,6 +276,22 @@ export function Composer({ draft }: { draft: Draft }) { )}
+ {/* + Replying in one format to a message written in the other loses + something either way: the formatting of a rich reply, or the plain + text somebody chose to write in. The draft opens in the format the + settings ask for, and this offers the other one for this message + only, rather than quietly overriding the setting (#407). + */} + {d.formatOffer && ( +
+ {d.formatOffer === "html" ? translate("This message is rich text") : translate("This message is plain text")} + + +
+ )} {d.format === "html" ? ( addFiles(key, files)} showToolbar={showToolbar} autoFocus={initialFocus === "body"} /> ) : ( diff --git a/web/src/views/compose/__tests__/format-offer-bar.test.tsx b/web/src/views/compose/__tests__/format-offer-bar.test.tsx new file mode 100644 index 0000000..8d8626e --- /dev/null +++ b/web/src/views/compose/__tests__/format-offer-bar.test.tsx @@ -0,0 +1,107 @@ +import { act } from "react"; +import { createRoot, type Root } from "react-dom/client"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { Composer } from "../Composer"; +import { useCompose, type Draft } from "@/store/compose"; +import { useMail } from "@/store/mail"; + +(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + +/** + * The bar the composer shows when the draft's format doesn't match the message + * it is answering (#407). A store test can say the offer was made; only the + * component can say that pressing it converts the body and puts the bar away. + */ + +window.matchMedia = ((q: string) => ({ matches: false, media: q, addEventListener() {}, removeEventListener() {} })) as unknown as typeof window.matchMedia; + +const QUOTE_HTML = '

On Friday, Ann wrote:

Look at this

'; +const QUOTE_TEXT = "\n\nOn Friday, Ann wrote:\n> Look at this"; + +const REPLY: Partial = { + key: "d1", replyMode: "reply", subject: "Re: Numbers", + format: "text", text: QUOTE_TEXT, html: `

${QUOTE_HTML}`, + quoteHtml: QUOTE_HTML, quoteText: QUOTE_TEXT, + formatOffer: "html", +}; + +describe("the format offer in the composer", () => { + let host: HTMLDivElement; + let root: Root; + const bar = () => document.querySelector(".composer-notice"); + const draft = () => useCompose.getState().drafts[0]!; + const button = (label: string) => Array.from(document.querySelectorAll(".composer-notice button")).find((b) => b.textContent === label || b.getAttribute("aria-label") === label)!; + + beforeEach(() => { + useMail.setState({ accountId: "a1", identities: [] as never }); + useCompose.setState({ drafts: [], activeKey: null, pendingSends: {} }); + const key = useCompose.getState().open(); + useCompose.getState().update(key, REPLY); + host = document.createElement("div"); + document.body.appendChild(host); + root = createRoot(host); + act(() => root.render()); + }); + afterEach(() => { act(() => root.unmount()); host.remove(); }); + + it("offers the message's own format, and says which it is", () => { + expect(bar()?.textContent).toContain("This message is rich text"); + expect(button("Switch to rich text")).toBeTruthy(); + }); + + it("switches this draft and puts the bar away", () => { + act(() => button("Switch to rich text").click()); + act(() => root.render()); + expect(draft().format).toBe("html"); + // The quoted reply came across, rather than the editor opening empty. + expect(draft().html).toContain("Ann wrote"); + expect(bar()).toBeNull(); + }); + + /* + * The message being quoted was prepared in both formats when the reply + * opened. Switching used to convert the plain-text body it had, handing + * back a flattened copy -- "> Look at this" -- of markup that still + * existed untouched on the draft. + */ + it("restores the original message, rather than converting the flattened quote", () => { + act(() => button("Switch to rich text").click()); + act(() => root.render()); + expect(draft().html).toContain("this"); + expect(draft().html).toContain("
"); + expect(draft().html).not.toContain("> Look at this"); + }); + + it("keeps what the author typed above the quote", () => { + useCompose.getState().update("d1", { text: `Thanks, that helps.${QUOTE_TEXT}` }); + act(() => root.render()); + act(() => button("Switch to rich text").click()); + act(() => root.render()); + expect(draft().html).toContain("Thanks, that helps."); + expect(draft().html).toContain("this"); + // Once only: the typed reply must not arrive with the quote doubled. + expect(draft().html.match(/On Friday, Ann wrote:/g)).toHaveLength(1); + }); + + it("goes back to plain text with the prepared quote, not a re-flattened one", () => { + // A rich draft answering a plain-text message: the offer runs the other way. + act(() => { + useCompose.getState().update("d1", { format: "html", html: `
Thanks.
${QUOTE_HTML}`, formatOffer: "text" }); + }); + act(() => root.render()); + act(() => button("Switch to plain text").click()); + act(() => root.render()); + expect(draft().format).toBe("text"); + expect(draft().text).toContain("Thanks."); + // The prepared plain-text quote, not HTML run through a converter. + expect(draft().text.endsWith(QUOTE_TEXT)).toBe(true); + expect(draft().text).not.toContain("
"); + }); + + it("dismisses without changing the format", () => { + act(() => button("Dismiss").click()); + act(() => root.render()); + expect(draft().format).toBe("text"); + expect(bar()).toBeNull(); + }); +}); diff --git a/web/src/views/mail/MailboxPicker.tsx b/web/src/views/mail/MailboxPicker.tsx index 43eece7..41ffeda 100644 --- a/web/src/views/mail/MailboxPicker.tsx +++ b/web/src/views/mail/MailboxPicker.tsx @@ -5,6 +5,7 @@ import { Dialog } from "@/ui/dialog"; import type { Id, Mailbox } from "@/jmap/types"; import { t } from "@/lib/i18n"; import { mailboxDisplayPath } from "@/lib/mailbox/mailboxName"; +import { treeOrder } from "@/lib/mailbox/folderOrder"; /** * @param need which right a folder has to grant to be worth offering. @@ -24,10 +25,11 @@ export function MailboxPicker({ title, onClose, onPick, exclude, need = "mayAddI const [q, setQ] = useState(""); const [active, setActive] = useState(0); const list = useMemo(() => { - const all = Object.values(mailboxes) + // The sidebar's order, not A–Z by path: a folder dragged into place has to + // be found in the same place here. + const all = treeOrder(mailboxes) .filter((m) => !exclude?.includes(m.id) && m.myRights[need] && (!allow || allow(m.id))) - .map((m) => ({ m, path: mailboxDisplayPath(m, mailboxes), pick: () => onPick(m.id) })) - .sort((a, b) => (a.m.role === "inbox" ? -1 : b.m.role === "inbox" ? 1 : a.path.localeCompare(b.path))); + .map((m) => ({ m, path: mailboxDisplayPath(m, mailboxes), pick: () => onPick(m.id) })); const rows: { m: Mailbox | null; path: string; pick: () => void }[] = root ? [{ m: null, path: root.label, pick: root.onPick }, ...all] : all; const ql = q.trim().toLowerCase(); return ql ? rows.filter((x) => x.path.toLowerCase().includes(ql)) : rows; diff --git a/web/src/views/mail/MessageView.tsx b/web/src/views/mail/MessageView.tsx index d9133df..438a246 100644 --- a/web/src/views/mail/MessageView.tsx +++ b/web/src/views/mail/MessageView.tsx @@ -19,6 +19,7 @@ import { llmOpinion } from "@/lib/llmOpinion"; import { LlmOpinionBanner, LlmOpinionDetail, llmBannerOpinion } from "./LlmOpinion"; import { formatFullDate, formatListDate, formatSize } from "@/lib/format"; import { displayName, domainOf, formatAddress } from "@/lib/address"; +import { remoteImagesAllowed } from "@/lib/mail/remoteImages"; import { EMAIL_BASE_CSS, TEXT_EMAIL_CSS, hasHtmlAlternative, htmlDeclaresColors, markKeptSurfaces, sanitizeEmailHtml } from "@/lib/text/html"; import { openableInTab, previewKind } from "@/lib/preview"; // Loaded when first opened: it is not needed to show mail, and it is not small. @@ -120,7 +121,12 @@ export const MessageView = memo(function MessageView({ email: e, expanded, wasUn /* Stable, so the body's click handler keeps its identity between renders. Passing an inline arrow here is what made the handler change on every render in the first place. */ - const showImages = useCallback(() => setAllowRemote(true), []); + const showImages = useCallback(() => { + setAllowRemote(true); + // Recorded for the composer: a reply quotes this message and must not + // fetch what the reader has not agreed to (#410). + useMail.getState().showImages(e.id); + }, [e.id]); const [filterOpen, setFilterOpen] = useState(false); const moreMenu = useMenu(); const [, navigate] = useLocation(); @@ -130,7 +136,7 @@ export const MessageView = memo(function MessageView({ email: e, expanded, wasUn const from = e.from?.[0]; const senderTrusted = settings.trustedImageSenders.includes((from?.email ?? "").toLowerCase()); const inContacts = useContacts((s) => Boolean(from && s.loaded && s.lookupByEmail(from.email))); - const remoteAllowed = allowRemote || settings.imagePolicy === "always" || senderTrusted || (settings.imagePolicy === "contacts" && inContacts); + const remoteAllowed = remoteImagesAllowed({ from: from?.email, policy: settings.imagePolicy, trusted: settings.trustedImageSenders, inContacts, shown: allowRemote }); const imageProxy = useSession((s) => s.session?.ihasmail?.imageProxy ?? true); const scheduled = useScheduled((s) => s.pending[e.id]); const receipt = useMemo(() => mdnDecision(e), [e]); diff --git a/web/src/views/mail/ThreadView.tsx b/web/src/views/mail/ThreadView.tsx index 585e3dc..46db4f6 100644 --- a/web/src/views/mail/ThreadView.tsx +++ b/web/src/views/mail/ThreadView.tsx @@ -228,7 +228,23 @@ export function ThreadView({ threadId, mailboxId, onBack, actions, onNavigate, h }, [messages, reply]); const subject = messages[0]?.subject || emails[thread?.emailIds[0] ?? ""]?.subject || "(no subject)"; - const rowIds = thread ? thread.emailIds.filter((id) => emails[id]) : []; + /* + * What the toolbar acts on: the messages the pane is showing, not the thread + * they belong to. + * + * With conversation view off, opening a message opens that message -- the + * list shows it alone, the pane renders it alone, and the buttons above it + * said so, because `anyUnread` and the rest already read `messages`. Only the + * ids handed to the action still named the whole thread, so Mark as unread, + * Move to, Report spam and Delete quietly took every message in it (#414). + * + * Same fallback as the pane's: an id naming nothing in this thread means the + * whole conversation, so the buttons keep matching what is on screen. + */ + const rowIds = useMemo(() => { + const loaded = thread ? thread.emailIds.filter((id) => emails[id]).map((id) => ({ id })) : []; + return visibleMessages(loaded, messageId).map((m) => m.id); + }, [thread, emails, messageId]); const anyUnread = messages.some((e) => !e.keywords.$seen); const anyStarred = messages.some((e) => e.keywords.$flagged); const inJunk = Boolean(mailboxId && mailboxes[mailboxId]?.role === "junk"); diff --git a/web/src/views/mail/__tests__/move-picker-order.test.tsx b/web/src/views/mail/__tests__/move-picker-order.test.tsx new file mode 100644 index 0000000..6d00d78 --- /dev/null +++ b/web/src/views/mail/__tests__/move-picker-order.test.tsx @@ -0,0 +1,62 @@ +import { act } from "react"; +import { createRoot, type Root } from "react-dom/client"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { MailboxPicker } from "../MailboxPicker"; +import { useMail } from "@/store/mail"; +import type { Mailbox, MailboxRole } from "@/jmap/types"; + +(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + +/** + * The move-to picker (v) lists folders in the sidebar's order (#1 on GitLab). + * + * It used to sort A–Z by path, so a folder dragged into place in the sidebar + * turned up somewhere else here. The ordering has its own tests in + * lib/mailbox; these check what the dialog actually shows. + */ + +window.matchMedia = ((q: string) => ({ matches: false, media: q, addEventListener() {}, removeEventListener() {} })) as unknown as typeof window.matchMedia; + +const rights = { mayReadItems: true, mayAddItems: true, mayRemoveItems: true, maySetSeen: true, maySetKeywords: true, mayCreateChild: true, mayRename: true, mayDelete: true, maySubmit: true }; +const box = (id: string, name: string, parentId: string | null, role: MailboxRole = null, sortOrder = 0): Mailbox => ({ + id, name, parentId, role, sortOrder, totalEmails: 0, unreadEmails: 0, totalThreads: 0, unreadThreads: 0, myRights: rights, isSubscribed: true, +}); + +/** Ordered by hand in the sidebar: Zeta dragged to the top, Alpha to the bottom. */ +const MAILBOXES = { + inbox: box("inbox", "Inbox", null, "inbox", 10), + zeta: box("zeta", "Zeta", null, null, 20), + sent: box("sent", "Sent", null, "sent", 30), + work: box("work", "Work", null, null, 40), + clients: box("clients", "Clients", "work"), + trash: box("trash", "Deleted Items", null, "trash", 50), + alpha: box("alpha", "Alpha", null, null, 60), +}; + +describe("the move-to picker", () => { + let host: HTMLDivElement; + let root: Root; + const rows = () => Array.from(document.querySelectorAll('[role="option"]')).map((r) => r.querySelector(".grow")?.textContent); + + function open(props: Partial[0]> = {}) { + act(() => root.render( {}} onPick={() => {}} {...props} />)); + } + + beforeEach(() => { + useMail.setState({ mailboxes: MAILBOXES, mailboxesLoaded: true }); + host = document.createElement("div"); + document.body.appendChild(host); + root = createRoot(host); + }); + afterEach(() => { act(() => root.unmount()); host.remove(); }); + + it("lists folders in the order they were dragged into, not A–Z", () => { + open(); + expect(rows()).toEqual(["Inbox", "Zeta", "Sent", "Work", "Work / Clients", "Deleted Items", "Alpha"]); + }); + + it("keeps that order for the folders left after excluding one", () => { + open({ exclude: ["work"] }); + expect(rows()).toEqual(["Inbox", "Zeta", "Sent", "Work / Clients", "Deleted Items", "Alpha"]); + }); +}); diff --git a/web/src/views/mail/__tests__/thread-toolbar-scope.test.tsx b/web/src/views/mail/__tests__/thread-toolbar-scope.test.tsx new file mode 100644 index 0000000..24cf3c0 --- /dev/null +++ b/web/src/views/mail/__tests__/thread-toolbar-scope.test.tsx @@ -0,0 +1,128 @@ +import { act } from "react"; +import { createRoot, type Root } from "react-dom/client"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { ThreadView } from "../ThreadView"; +import { useMail } from "@/store/mail"; +import type { ListActions } from "../MessageList"; +import type { Email, Id } from "@/jmap/types"; + +(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + +/* jsdom has neither of these, and the opening scroll uses both. */ +Element.prototype.scrollIntoView = () => {}; +globalThis.ResizeObserver ??= class { observe() {} unobserve() {} disconnect() {} } as unknown as typeof ResizeObserver; + +/* jsdom has no matchMedia, and the toolbar asks whether this is a phone. */ +window.matchMedia = ((q: string) => ({ matches: false, media: q, addEventListener() {}, removeEventListener() {} })) as unknown as typeof window.matchMedia; + +/* + * Reported from the inbox with conversation view off: marking a message unread + * from the list -- hover button, right-click menu -- touched that message, but + * the same action from the toolbar above the *opened* message marked every + * message in its thread. Move to, Report spam and Delete did it too (#414). + * + * The toolbar's labels were already right: "Mark as unread" read the messages + * on screen. Only the ids it handed the action named the whole thread. + */ + +const msg = (id: Id, subject: string): Email => + ({ + id, threadId: "t1", subject, mailboxIds: { inbox: true }, keywords: { $seen: true }, + from: [{ name: "Ann", email: "ann@example.com" }], to: [{ name: "Me", email: "me@example.org" }], + receivedAt: "2026-09-20T10:00:00Z", size: 10, blobId: "b1", preview: "hi", + htmlBody: [], textBody: [{ partId: "1", type: "text/plain" }], + bodyValues: { "1": { value: "hi", isEncodingProblem: false, isTruncated: false } }, + attachments: [], + }) as unknown as Email; + +const FIRST = msg("m1", "The question"); +const SECOND = msg("m2", "Re: The question"); + +function stubStore() { + useMail.setState({ + accountId: "a1", + threads: { t1: { id: "t1", emailIds: ["m1", "m2"] } } as never, + emails: { m1: FIRST, m2: SECOND } as never, + fullIds: { m1: true, m2: true } as never, + loadingThreads: {} as never, + mailboxes: { inbox: { id: "inbox", name: "Inbox", role: "inbox" } } as never, + loadThread: (async () => undefined) as never, + setOpenThread: (() => undefined) as never, + markRead: (async () => undefined) as never, + roleId: (() => null) as never, + }); +} + +describe("what the toolbar above an opened message acts on", () => { + let host: HTMLDivElement; + let root: Root; + let actions: ListActions; + + const show = async (messageId: Id | null) => { + await act(async () => { + root.render( + undefined} onNavigate={() => undefined} hasPrev={false} hasNext={false} + />, + ); + }); + }; + + /** The toolbar buttons carry their shortcut in the title, as the tooltips show. */ + const press = async (title: string) => { + const btn = [...host.querySelectorAll("button")].find((b) => b.title === title); + expect(btn, `no toolbar button titled ${title}`).toBeTruthy(); + await act(async () => btn!.click()); + }; + + beforeEach(() => { + stubStore(); + actions = { + archive: vi.fn(async () => undefined), trash: vi.fn(async () => undefined), + spam: vi.fn(async () => undefined), read: vi.fn(async () => undefined), + star: vi.fn(async () => undefined), move: vi.fn(async () => undefined), + label: vi.fn(async () => undefined), + } as unknown as ListActions; + host = document.createElement("div"); + document.body.appendChild(host); + root = createRoot(host); + }); + + afterEach(async () => { + await act(async () => root.unmount()); + host.remove(); + }); + + it("marks only the message that is open, not its thread", async () => { + await show("m1"); + await press("Mark as unread"); + expect(actions.read).toHaveBeenCalledWith(false, ["m1"]); + }); + + it("moves, reports and deletes only that message too", async () => { + await show("m1"); + await press("Move to (v)"); + await press("Report spam (!)"); + await press("Delete (#)"); + expect(actions.move).toHaveBeenCalledWith(["m1"]); + expect(actions.spam).toHaveBeenCalledWith(["m1"]); + expect(actions.trash).toHaveBeenCalledWith(["m1"]); + }); + + it("takes the whole thread when the pane is showing the whole thread", async () => { + // Conversation view on: no message singled out, and the toolbar is the + // conversation's toolbar. That is the behaviour this must not disturb. + await show(null); + await press("Mark as unread"); + expect(actions.read).toHaveBeenCalledWith(false, ["m1", "m2"]); + }); + + it("falls back to the thread when the open id names nothing in it", async () => { + // A link from somebody with conversation view on, or a stale `m` in the + // URL. The pane shows the conversation, so the toolbar acts on it. + await show("gone"); + await press("Mark as unread"); + expect(actions.read).toHaveBeenCalledWith(false, ["m1", "m2"]); + }); +});