From 18e493bcd19426357a653cc28352a8f233277742 Mon Sep 17 00:00:00 2001 From: John Coffey Date: Wed, 26 Aug 2026 12:00:16 -0700 Subject: [PATCH] Delete all spam, and call folders what the server calls them Junk Mail can now be emptied in one action, the way every other mail client offers it: a banner across the top of the folder, and an item in both the folder's right-click menu and the list's own menu. The messages are destroyed rather than moved to Deleted Items. Routing spam through the bin on its way out leaves you with the same problem in a different folder, and "delete all spam" means gone everywhere else. So the dialog says it before you commit, and there is no undo. emptyMailbox already did the hard part -- walking a folder a page at a time so it survives maxObjectsInSet, which a Deleted Items of 5192 once did not. All that changed is which folders it will accept. The guard stays in the store rather than living only in the menus, so a fourth caller cannot empty the Inbox by asking nicely. The three entry points share one helper, because three dialogs warning about a permanent deletion in three slightly different ways is how one of them ends up not warning at all. A folder with nothing in it offers the item greyed out rather than hiding it, so it is where you expect it to be next time. Folder naming is fixed in the same commit because it changed the same file, and because testing this is what surfaced it. Two problems, one cause: - The mock called its folders "Trash" and "Sent". Stalwart's defaults follow the Exchange convention -- "Deleted Items", "Sent Items" -- so anything built from a folder's name read differently against the mock than against a real server, and every screenshot in the README showed a folder list no user has. - Worse, and in shipping code: the undo toast took a hardcoded label in preference to the folder's actual name, so deleting a message announced "moved to Trash" on a server whose folder is called "Deleted Items", and reporting spam said "moved to Spam" where it is "Junk Mail". The one message whose job is saying where mail went was naming somewhere that does not exist. It now prefers the mailbox's own name and keeps the hardcoded word only as a fallback. Verified against the mock: the banner appears only in Junk and only with something to delete, the dialog counts and pluralises, the messages are destroyed and Deleted Items stays empty afterwards, the banner disappears once the folder is, the item greys out when empty, Archive is offered neither, and Trash still says "Empty Deleted Items". --- README.md | 1 + server/src/mock/index.ts | 13 ++++- web/src/lib/__tests__/emptyFolder.test.ts | 41 +++++++++++++++ web/src/lib/emptyFolder.tsx | 52 +++++++++++++++++++ web/src/store/__tests__/empty-mailbox.test.ts | 48 ++++++++++++++--- web/src/store/mail.ts | 24 ++++++--- web/src/styles/app.css | 2 +- web/src/views/mail/MailboxTree.tsx | 8 ++- web/src/views/mail/MessageList.tsx | 30 ++++++++--- 9 files changed, 188 insertions(+), 31 deletions(-) create mode 100644 web/src/lib/__tests__/emptyFolder.test.ts create mode 100644 web/src/lib/emptyFolder.tsx diff --git a/README.md b/README.md index 19450b7..cc2b698 100644 --- a/README.md +++ b/README.md @@ -65,6 +65,7 @@ seconds of downtime with nothing lost. - Gmail-style three-pane layout (reading pane right/bottom/off, **drag-to-resize splitter** in both orientations, quick layout switch in the list menu), conversation view with collapsed messages and "show quoted text", dense/cozy/comfortable density, light/dark/system themes plus **ihasmail** — the palette from ihasmail.org, and what a new account starts on — each with accent colours over the top - Virtualised, infinitely-scrolling message list; multi-select (click, ⇧-click, ⌃-click), drag & drop to folders, right-click context menus, hover actions, Gmail keyboard shortcuts (`j/k`, `e`, `#`, `r/a/f`, `g i`, `/`, `?` …) - Archive / delete / spam / star / mark read / move / labels (IMAP keywords with colours) with **Undo** +- **Delete all spam** — a banner across the top of Junk Mail, and an item in the folder's right-click menu and the list's own menu, that empties it in one action. The messages are destroyed rather than moved to Deleted Items, since routing spam through the bin leaves you with the same problem in another folder; the dialog says so before you commit, and there is no undo. Only Deleted Items and Junk Mail can be emptied this way — enforced in the store rather than merely hidden in the menus — and a folder that is already empty offers it greyed out - **"Filter messages like this…"** from the message context menu: creates a Sieve rule pre-filled from the sender/list (target folders can be created on the fly), and can **apply it immediately to the existing messages in the folder** (evaluated client-side, actions applied via JMAP) - Safe HTML rendering: DOMPurify sanitisation inside a Shadow DOM, **remote images blocked by default** with a per-sender allow-list and an optional **privacy image proxy** (like Gmail's) - Messages sit on a light card by default, untouched as the sender designed them. *Appearance › Apply the theme to messages too* lets them follow the app's light/dark theme instead — plain-text mail always does, and with the option on so does HTML mail that brings no colours of its own; mail that styles itself is still left alone diff --git a/server/src/mock/index.ts b/server/src/mock/index.ts index f90bfcf..386a6e1 100644 --- a/server/src/mock/index.ts +++ b/server/src/mock/index.ts @@ -44,12 +44,21 @@ const state = { n: 1 }; const nextState = () => String(state.n++); /* ---------- data ---------- */ +/* + * The names are Stalwart's own defaults, which follow the Exchange convention: + * "Deleted Items" and "Sent Items", not "Trash" and "Sent". The mock used the + * short forms, so anything built from a folder's name read differently here + * than in production -- "Empty Trash" against the mock, "Empty Deleted Items" + * against a real server -- and every screenshot in the README showed a folder + * list no user has. The role is what the client branches on; the name is only + * ever displayed, which is exactly why it has to look right. + */ const mailboxes: Obj[] = [ mb("inbox", "Inbox", "inbox"), mb("drafts", "Drafts", "drafts"), - mb("sent", "Sent", "sent"), + mb("sent", "Sent Items", "sent"), mb("junk", "Junk Mail", "junk"), - mb("trash", "Trash", "trash"), + mb("trash", "Deleted Items", "trash"), mb("archive", "Archive", "archive"), mb("work", "Work", null), mb("work-inv", "Invoices", null, "work"), diff --git a/web/src/lib/__tests__/emptyFolder.test.ts b/web/src/lib/__tests__/emptyFolder.test.ts new file mode 100644 index 0000000..9fd8d6b --- /dev/null +++ b/web/src/lib/__tests__/emptyFolder.test.ts @@ -0,0 +1,41 @@ +import { describe, expect, it } from "vitest"; +import { canEmpty, emptyLabel } from "@/lib/emptyFolder"; +import type { MailboxRole } from "@/jmap/types"; + +/** + * Emptying destroys everything in a folder in one action, with no undo and no + * trip through Deleted Items. Which folders may be emptied is therefore a + * safety property, not a presentation one — the store enforces it too, and + * these pin the half the menus decide. + */ + +describe("which folders may be emptied", () => { + it("allows exactly Deleted Items and Junk Mail", () => { + expect(canEmpty("trash")).toBe(true); + expect(canEmpty("junk")).toBe(true); + }); + + it("refuses folders holding mail someone meant to keep", () => { + const keep: MailboxRole[] = ["inbox", "archive", "sent", "drafts", "all", "flagged", "important", "subscribed"]; + for (const role of keep) expect(canEmpty(role), String(role)).toBe(false); + }); + + it("refuses a plain folder, which has no role at all", () => { + expect(canEmpty(null)).toBe(false); + expect(canEmpty(undefined)).toBe(false); + }); +}); + +describe("what the action is called", () => { + it("says what it does to spam, rather than naming the folder", () => { + // "Delete all spam" is what this is called everywhere else; "Empty Junk + // Mail" would be accurate and still leave people hunting for it. + expect(emptyLabel({ name: "Junk Mail", role: "junk" })).toBe("Delete all spam"); + expect(emptyLabel({ name: "Spam", role: "junk" })).toBe("Delete all spam"); + }); + + it("names the folder for Deleted Items, whatever the server calls it", () => { + expect(emptyLabel({ name: "Deleted Items", role: "trash" })).toBe("Empty Deleted Items"); + expect(emptyLabel({ name: "Trash", role: "trash" })).toBe("Empty Trash"); + }); +}); diff --git a/web/src/lib/emptyFolder.tsx b/web/src/lib/emptyFolder.tsx new file mode 100644 index 0000000..fd49d53 --- /dev/null +++ b/web/src/lib/emptyFolder.tsx @@ -0,0 +1,52 @@ +/** + * Emptying a folder, and asking first. + * + * There are three ways in — the folder's right-click menu, the list's own + * menu, and the banner across the top of Junk Mail — and they must not drift + * apart in what they warn about. A folder can only be emptied when it is one + * whose whole purpose is holding things you did not want: Deleted Items, or + * Junk Mail. + * + * The wording differs between them for a reason. Emptying Deleted Items is + * what anyone expects it to do. Emptying Junk Mail is the surprising one: the + * messages do not travel to Deleted Items on the way out, so there is no + * second chance to change your mind, and the dialog says so rather than + * leaving it to be discovered. + */ +import { confirmDialog } from "@/ui/dialog"; +import { useMail } from "@/store/mail"; +import type { Id, MailboxRole } from "@/jmap/types"; + +export interface EmptyTarget { + id: Id; + name: string; + role: MailboxRole; + totalEmails: number; +} + +/** Whether this folder is one that may be emptied at all. */ +export function canEmpty(role: MailboxRole | undefined | null): boolean { + return role === "trash" || role === "junk"; +} + +const plural = (n: number) => `${n.toLocaleString()} message${n === 1 ? "" : "s"}`; + +/** What the button or menu item is called, in the folder's own terms. */ +export function emptyLabel(target: Pick): string { + return target.role === "junk" ? "Delete all spam" : `Empty ${target.name}`; +} + +/** Ask, then empty. Resolves once the emptying has been attempted, or declined. */ +export async function confirmAndEmpty(target: EmptyTarget): Promise { + if (!canEmpty(target.role)) return; + const junk = target.role === "junk"; + const ok = await confirmDialog({ + title: junk ? `Delete all spam in “${target.name}”?` : `Empty “${target.name}”?`, + message: junk + ? `All ${plural(target.totalEmails)} will be deleted permanently. They do not go to Deleted Items first, so this cannot be undone.` + : `All ${plural(target.totalEmails)} will be permanently deleted.`, + confirmLabel: junk ? "Delete all spam" : "Empty folder", + danger: true, + }); + if (ok) await useMail.getState().emptyMailbox(target.id); +} diff --git a/web/src/store/__tests__/empty-mailbox.test.ts b/web/src/store/__tests__/empty-mailbox.test.ts index 1f9598e..dd7077e 100644 --- a/web/src/store/__tests__/empty-mailbox.test.ts +++ b/web/src/store/__tests__/empty-mailbox.test.ts @@ -12,6 +12,7 @@ import type { JmapSession } from "@/jmap/types"; */ const TRASH = "mbTrash"; +const JUNK = "mbJunk"; const MAX = 500; interface Call { @@ -21,13 +22,13 @@ interface Call { } /** A server that holds `count` messages and enforces MAX objects per call. */ -function server(count: number, opts: { refuseDestroy?: boolean } = {}) { +function server(count: number, opts: { refuseDestroy?: boolean; mailbox?: string } = {}) { const live = new Set(Array.from({ length: count }, (_, i) => `e${i}`)); const destroyBatches: number[] = []; const fetchMock = vi.fn(async (_url: string, init: RequestInit) => { const body = JSON.parse(init.body as string) as { methodCalls: [string, Record, string][] }; const methodResponses = body.methodCalls.map(([name, args, id]: [string, Record, string]) => { - if (name === "Email/query" && (args.filter as { inMailbox?: string })?.inMailbox === TRASH) { + if (name === "Email/query" && (args.filter as { inMailbox?: string })?.inMailbox === (opts.mailbox ?? TRASH)) { const limit = Math.min((args.limit as number) ?? 50, MAX); return [name, { accountId: "a1", queryState: "q", canCalculateChanges: false, position: 0, ids: [...live].slice(0, limit), total: live.size }, id]; } @@ -57,7 +58,16 @@ beforeEach(() => { primaryAccounts: {}, state: "s1", } as unknown as JmapSession; - useMail.setState({ accountId: "a1", mailboxes: { [TRASH]: { id: TRASH, role: "trash", name: "Deleted Items" } } as never, list: null, emails: {} }); + useMail.setState({ + accountId: "a1", + mailboxes: { + [TRASH]: { id: TRASH, role: "trash", name: "Deleted Items" }, + [JUNK]: { id: JUNK, role: "junk", name: "Junk Mail" }, + mbArchive: { id: "mbArchive", role: "archive", name: "Archive" }, + } as never, + list: null, + emails: {}, + }); useToasts.setState({ toasts: [] }); }); @@ -82,13 +92,35 @@ describe("emptyMailbox", () => { expect(messages()).toContain("Deleted 12 messages"); }); - it("refuses any folder that is not Deleted Items", async () => { - const s = server(5192); - useMail.setState({ mailboxes: { ...useMail.getState().mailboxes, mbJunk: { id: "mbJunk", role: "junk", name: "Junk" } } as never }); - await useMail.getState().emptyMailbox("mbJunk"); + /** + * Junk Mail is emptiable too, and the messages are destroyed rather than + * moved to Deleted Items — routing spam through the bin on its way out + * would leave the user with the same problem in a different folder. + */ + it("empties Junk Mail, destroying rather than moving to Deleted Items", async () => { + const s = server(1200, { mailbox: JUNK }); + await useMail.getState().emptyMailbox(JUNK); + expect(s.live.size).toBe(0); + expect(Math.max(...s.destroyBatches)).toBeLessThanOrEqual(MAX); + expect(messages()).toContain("Deleted 1200 messages"); + // Nothing was moved anywhere: every mutating call was a destroy. + const sets = s.fetchMock.mock.calls.flatMap(([, init]) => { + const body = JSON.parse((init as RequestInit).body as string) as { methodCalls: [string, Record, string][] }; + return body.methodCalls.filter(([n]) => n === "Email/set").map(([, a]) => a); + }); + expect(sets.length).toBeGreaterThan(0); + for (const a of sets) { + expect(Array.isArray(a.destroy)).toBe(true); + expect(a.update).toBeUndefined(); + } + }); + + it("refuses a folder that is neither Deleted Items nor Junk Mail", async () => { + const s = server(5192, { mailbox: "mbArchive" }); + await useMail.getState().emptyMailbox("mbArchive"); expect(s.destroyBatches).toEqual([]); expect(s.live.size).toBe(5192); - expect(messages()).toContain("Only Deleted Items can be emptied."); + expect(messages()).toContain("Only Deleted Items and Junk Mail can be emptied."); }); it("stops instead of looping when the server destroys nothing", async () => { diff --git a/web/src/store/mail.ts b/web/src/store/mail.ts index 002e852..8156cb8 100644 --- a/web/src/store/mail.ts +++ b/web/src/store/mail.ts @@ -447,7 +447,12 @@ export const useMail = create((set, get) => ({ try { await setEmails(accountId, update); if (!opts.silent) { - const name = opts.label ?? mailboxes[toMailboxId]?.name ?? "folder"; + // The folder's own name, because that is what the user is looking at + // in the sidebar. A hardcoded word here told people their mail had + // moved to "Trash" or "Spam" on a server whose folders are called + // "Deleted Items" and "Junk Mail" -- naming somewhere that does not + // exist, in the one message whose job is saying where it went. + const name = mailboxes[toMailboxId]?.name ?? opts.label ?? "folder"; toast.show(`${ids.length === 1 ? "Conversation" : `${ids.length} conversations`} moved to ${name}`, { action: { label: "Undo", @@ -506,7 +511,7 @@ export const useMail = create((set, get) => ({ const inTrash = ids.filter((id) => (trashId && emails[id]?.mailboxIds[trashId]) || (roleId("junk") && emails[id]?.mailboxIds[roleId("junk")!])); const toMove = ids.filter((id) => !inTrash.includes(id)); if (inTrash.length) await get().destroy(inTrash); - if (toMove.length && trashId) await get().move(toMove, trashId, { label: "Trash" }); + if (toMove.length && trashId) await get().move(toMove, trashId, { label: "Deleted Items" }); else if (toMove.length) await get().destroy(toMove); }, @@ -552,17 +557,22 @@ export const useMail = create((set, get) => ({ } catch { /* keyword may be rejected; still move */ } - await get().move(ids, target, { label: isSpam ? "Spam" : "Inbox" }); + await get().move(ids, target, { label: isSpam ? "Junk Mail" : "Inbox" }); }, async emptyMailbox(mailboxId) { const accountId = get().accountId; if (!accountId) return; // Emptying is permanent and covers the whole folder at once, so it is - // offered for Deleted Items alone. The menus hide it elsewhere; this is - // the guard that makes that true of the action itself. - if (mailboxId !== get().roleId("trash")) { - toast.error("Only Deleted Items can be emptied."); + // offered only for the two folders whose whole purpose is holding what you + // did not want. The menus hide it elsewhere; this is the guard that makes + // that true of the action itself, whatever calls it. + // + // Junk Mail is destroyed outright rather than moved to Deleted Items — + // there is no point routing spam through the bin on its way out, and it is + // what "delete all spam" means everywhere else. The dialogs say so. + if (mailboxId !== get().roleId("trash") && mailboxId !== get().roleId("junk")) { + toast.error("Only Deleted Items and Junk Mail can be emptied."); return; } // A folder can hold far more messages than the server will destroy in one diff --git a/web/src/styles/app.css b/web/src/styles/app.css index b4a4f7a..32da255 100644 --- a/web/src/styles/app.css +++ b/web/src/styles/app.css @@ -504,7 +504,7 @@ img { max-width: 100%; } .msg-row .msg-important { color: var(--warn); } .list-footer { padding: 12px; text-align: center; color: var(--fg-muted); font-size: .9em; } .list-hint { padding: 8px 12px; font-size: .85em; color: var(--fg-muted); background: var(--bg-sunken); border-bottom: 1px solid var(--border); display: flex; align-items: center; gap: 8px; } -.list-hint button { color: var(--link); font-weight: 600; } +.list-hint button { color: var(--link); font-weight: 600; white-space: nowrap; } .drag-ghost { position: fixed; top: -1000px; left: -1000px; padding: 8px 12px; background: var(--accent); color: var(--accent-fg); border-radius: 999px; font-weight: 600; box-shadow: var(--shadow-2); pointer-events: none; z-index: 5000; } /* Splitter between list and reading pane */ diff --git a/web/src/views/mail/MailboxTree.tsx b/web/src/views/mail/MailboxTree.tsx index 78217f8..28f1fd0 100644 --- a/web/src/views/mail/MailboxTree.tsx +++ b/web/src/views/mail/MailboxTree.tsx @@ -2,6 +2,7 @@ import { useMemo, useState, type DragEvent, type ReactNode } from "react"; import { Link, useLocation } from "wouter"; import { AlertOctagon, Archive, ChevronDown, Clock, ChevronRight, File, Folder, FolderPlus, Inbox, Mail, MoreVertical, Palette, Send, Star, Tag, Trash2, Plus, Pencil, Eye, EyeOff, CheckCheck, Eraser, Share2, X } from "lucide-react"; import { useMail } from "@/store/mail"; +import { canEmpty, confirmAndEmpty, emptyLabel } from "@/lib/emptyFolder"; import { isScheduledMailbox } from "@/store/scheduled"; import { useSettings } from "@/store/settings"; import type { Id, Mailbox } from "@/jmap/types"; @@ -328,10 +329,7 @@ function MailboxMenu({ mailbox: m, onClose, onCreateChild, onShare }: { mailbox: toast.error((err as Error).message); } }; - const empty = async () => { - const ok = await confirmDialog({ title: `Empty “${m.name}”?`, message: `All ${m.totalEmails} messages will be permanently deleted.`, confirmLabel: "Empty folder", danger: true }); - if (ok) await useMail.getState().emptyMailbox(m.id); - }; + const empty = () => confirmAndEmpty({ id: m.id, name: m.name, role: m.role, totalEmails: m.totalEmails }); const isSpecial = Boolean(m.role) && m.role !== "subscribed"; const color = folderColor(colors, m.id); const setColor = (c: string | null) => { @@ -372,7 +370,7 @@ function MailboxMenu({ mailbox: m, onClose, onCreateChild, onShare }: { mailbox: {color && } label="Use the default colour" onClick={() => setColor(null)} />} - {m.role === "trash" && } label="Empty folder" onClick={() => void empty()} danger />} + {canEmpty(m.role) && } label={emptyLabel(m)} onClick={() => void empty()} danger disabled={!m.totalEmails} />} } label="Delete folder" onClick={() => void remove()} danger disabled={isSpecial || !m.myRights.mayDelete} /> ); diff --git a/web/src/views/mail/MessageList.tsx b/web/src/views/mail/MessageList.tsx index 8b256bb..72de482 100644 --- a/web/src/views/mail/MessageList.tsx +++ b/web/src/views/mail/MessageList.tsx @@ -6,10 +6,10 @@ import { useMail, type ListState } from "@/store/mail"; import { dateTimeKey, useSettings } from "@/store/settings"; import type { Email, Id } from "@/jmap/types"; import { formatListDate } from "@/lib/format"; +import { canEmpty, confirmAndEmpty, emptyLabel } from "@/lib/emptyFolder"; import { displayName, shortName } from "@/lib/address"; import { Avatar, Empty, useIsMobile } from "@/ui/misc"; import { MenuItem, MenuSep, MenuTitle, Popover, useMenu } from "@/ui/popover"; -import { confirmDialog } from "@/ui/dialog"; import { useCompose } from "@/store/compose"; import { FilterFromMessageDialog } from "./FilterFromMessage"; @@ -74,8 +74,6 @@ export function MessageList({ title, list, openThreadId, focusId, setFocusId, on const selCount = Object.keys(selected).length; const mailbox = mailboxId ? mailboxes[mailboxId] : undefined; const isTrashOrJunk = mailbox?.role === "trash" || mailbox?.role === "junk"; - // Emptying in one action is for Deleted Items only; Junk is cleared by hand. - const isTrash = mailbox?.role === "trash"; const isDrafts = mailbox?.role === "drafts"; const rowHeight = twoLine ? (settings.density === "compact" ? 56 : settings.density === "comfortable" ? 78 : 66) : settings.density === "compact" ? 36 : settings.density === "comfortable" ? 52 : 44; @@ -200,16 +198,15 @@ export function MessageList({ title, list, openThreadId, focusId, setFocusId, on } label="Select all" onClick={selectAll} /> } label="Mark all as read" onClick={() => mailboxId && void useMail.getState().markMailboxRead(mailboxId)} disabled={!mailboxId} /> - {isTrash && ( + {mailbox && canEmpty(mailbox.role) && ( <> } - label={`Empty ${mailbox?.name}`} - onClick={async () => { - if (await confirmDialog({ title: `Empty ${mailbox?.name}?`, message: "All messages will be permanently deleted.", confirmLabel: "Empty", danger: true })) void useMail.getState().emptyMailbox(mailboxId!); - }} + label={emptyLabel(mailbox)} + disabled={!mailbox.totalEmails} + onClick={() => void confirmAndEmpty(mailbox)} /> )} @@ -223,6 +220,23 @@ export function MessageList({ title, list, openThreadId, focusId, setFocusId, on )} + {/* + Junk Mail's own banner, the way every other mail client offers it: + clearing spam is the one thing people come to this folder to do, and + making them find it in a menu is making them hunt for it. + + Only here, and only with something to delete. It says "permanently" + because that is the part worth knowing before clicking — these do not + pass through Deleted Items on the way out. + */} + {mailbox?.role === "junk" && !!mailbox.totalEmails && !selCount && ( +
+ + Deleting spam is permanent — it does not go to Deleted Items first. + + +
+ )}
{list?.loading && ids.length === 0 ? (