From 767d4b9f9f7d4e6be93e767a1a9b8d1039c86c12 Mon Sep 17 00:00:00 2001 From: lucletoffe <15689941+lucletoffe@users.noreply.github.com> Date: Wed, 14 Oct 2026 08:34:57 +0200 Subject: [PATCH] fix: keep a re-opened draft's attachments on live blobs across saves and sends A draft's attachments and inline images are MIME parts of that draft, and their blobIds die with the version that holds them. Each save creates the next version and then destroys the previous one (#849), but the composer kept sending the blobIds it was seeded with. The first save worked, every later save failed with "blobId ... does not exist on this server", and so did the send. This is the second half of webmail #849, which the webmail fixed in 5d2b937a but which was never ported here. - saveDraftVersion() reads the new version's attachment parts back in the same request (Email/get on the creation id "#draft"; Stalwart 0.16 does not resolve a result reference into Email/set "created"). createDraft() keeps its signature on top of it. - The composer maps each held blobId to the matching part of the latest version (inline by Content-ID, the rest by name and size) and sends that. Uploads are re-pointed too, since an unreferenced upload expires. The map lives in a ref, so the dirty snapshot does not change on every save. - Opening a draft with inline images no longer looks like an edit. The hydration swapped the editor body for its normalised DOM HTML but compared it with the raw server HTML, so the autosave replaced the draft about two seconds after it opened. That destroyed the opened version, and a plain "open, then Send" failed with the same blobNotFound. Tests: unit tests for the matching and rebasing, a fake-server test that reproduces the failure without the rebase, and an opt-in live Stalwart test (verified against Stalwart 0.16.22). Co-Authored-By: Claude Opus 5.5 (1M context) --- src/api/__tests__/email-draft-blobs.test.ts | 138 ++++++++++++++++++++ src/api/__tests__/live-stalwart.test.ts | 43 ++++++ src/api/email.ts | 53 +++++++- src/lib/__tests__/draft-part-blobs.test.ts | 100 ++++++++++++++ src/lib/draft-part-blobs.ts | 108 +++++++++++++++ src/screens/ComposeScreen.tsx | 40 +++++- 6 files changed, 471 insertions(+), 11 deletions(-) create mode 100644 src/api/__tests__/email-draft-blobs.test.ts create mode 100644 src/lib/__tests__/draft-part-blobs.test.ts create mode 100644 src/lib/draft-part-blobs.ts diff --git a/src/api/__tests__/email-draft-blobs.test.ts b/src/api/__tests__/email-draft-blobs.test.ts new file mode 100644 index 00000000..6c1d699d --- /dev/null +++ b/src/api/__tests__/email-draft-blobs.test.ts @@ -0,0 +1,138 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +vi.mock('../jmap-client', () => ({ + jmapClient: { + accountId: 'acc-1', + request: vi.fn(), + getMaxCallsInRequest: vi.fn(() => 16), + getMaxObjectsInGet: vi.fn(() => 500), + getMaxObjectsInSet: vi.fn(() => 500), + }, +})); + +import { jmapClient } from '../jmap-client'; +import { saveDraftVersion, type OutgoingAttachment, type OutgoingEmail } from '../email'; +import { initialDraftPartBlobState, liveBlobId, rebaseDraftPartBlobs } from '../../lib/draft-part-blobs'; + +const mockRequest = jmapClient.request as ReturnType; + +/** + * A tiny server with Stalwart's blob semantics, as measured on 0.16.22 (2026-10-14): + * an email's part blobIds are derived from that email and stop resolving the + * moment it is destroyed ("blobId … does not exist on this server"); creating + * an email copies the referenced bytes into new parts with new blobIds. + */ +function fakeStalwart() { + let seq = 0; + const emails = new Map>(); + const live = (blobId: string) => [...emails.values()].some((parts) => parts.some((p) => p.blobId === blobId)); + const seed = (id: string, parts: OutgoingAttachment[]) => { + emails.set(id, parts.map((p) => ({ blobId: p.blobId, name: p.name, size: p.size ?? 0, cid: p.cid, type: p.type }))); + }; + mockRequest.mockImplementation(async (calls: Array<[string, Record, string]>) => { + const out: Array<[string, unknown, string]> = []; + let createdId: string | undefined; + for (const [method, args, callId] of calls) { + if (method === 'Email/set' && args.create) { + const create = args.create.draft; + const dead = (create.attachments ?? []).find((a: { blobId: string }) => !live(a.blobId)); + if (dead) { + out.push(['Email/set', { notCreated: { draft: { + type: 'blobNotFound', description: `blobId ${dead.blobId} does not exist on this server.`, + } } }, callId]); + continue; + } + createdId = `e${++seq}`; + emails.set(createdId, (create.attachments ?? []).map((a: OutgoingAttachment, i: number) => ({ + blobId: `${createdId}-part${i}`, name: a.name, size: a.size ?? 0, cid: a.cid ? `<${a.cid}>` : undefined, type: a.type, + }))); + out.push(['Email/set', { created: { draft: { id: createdId } } }, callId]); + } else if (method === 'Email/set' && args.destroy) { + for (const id of args.destroy) emails.delete(id); + out.push(['Email/set', { destroyed: args.destroy }, callId]); + } else if (method === 'Email/get') { + if (!createdId || args.ids?.[0] !== '#draft') { + out.push(['error', { type: 'invalidResultReference' }, callId]); + continue; + } + out.push(['Email/get', { list: [{ id: createdId, attachments: emails.get(createdId) }] }, callId]); + } + } + return { methodResponses: out }; + }); + return { seed, emails }; +} + +const IMG_A: OutgoingAttachment = { blobId: 'd0-part-a', name: 'schema-a.png', size: 136574, type: 'image/png', disposition: 'inline', cid: 'a@undust.co' }; +const IMG_B: OutgoingAttachment = { blobId: 'd0-part-b', name: 'schema-b.png', size: 175155, type: 'image/png', disposition: 'inline', cid: 'b@undust.co' }; + +const draftWith = (attachments: OutgoingAttachment[]): OutgoingEmail => ({ + from: [{ email: 'me@undust.co' }], + to: [{ email: 'test@example.invalid' }], + subject: 'RE: draft with two inline images', + htmlBody: '

x

', + textBody: 'x', + attachments, +}); + +beforeEach(() => { + vi.clearAllMocks(); +}); + +describe('saveDraftVersion', () => { + it('reads the new version\'s parts back in the same request, by creation id', async () => { + fakeStalwart().seed('d0', [IMG_A]); + const saved = await saveDraftVersion(draftWith([IMG_A]), 'drafts', 'd0'); + const calls = mockRequest.mock.calls[0][0]; + expect(calls.map((c: [string]) => c[0])).toEqual(['Email/set', 'Email/get']); + expect(calls[1][1].ids).toEqual(['#draft']); + expect(saved.id).toBe('e1'); + expect(saved.attachments.map((a) => a.blobId)).toEqual(['e1-part0']); + }); + + it('destroys the previous version only after the new one exists', async () => { + fakeStalwart().seed('d0', [IMG_A]); + await saveDraftVersion(draftWith([IMG_A]), 'drafts', 'd0'); + const destroyCall = mockRequest.mock.calls[1][0][0]; + expect(destroyCall[0]).toBe('Email/set'); + expect(destroyCall[1].destroy).toEqual(['d0']); + }); + + it('still returns the id when the read-back fails', async () => { + mockRequest.mockResolvedValueOnce({ methodResponses: [ + ['Email/set', { created: { draft: { id: 'e9' } } }, '0'], + ['error', { type: 'serverFail' }, '1'], + ] }); + const saved = await saveDraftVersion(draftWith([]), 'drafts'); + expect(saved).toEqual({ id: 'e9', attachments: [] }); + }); +}); + +describe('re-opened draft with inline images: save, save again, send', () => { + // What ComposeScreen does: buildOutgoing maps each held blobId through the + // state, saveDraftOnce rebases it on the parts read back. + async function editSession(rebase: boolean) { + const server = fakeStalwart(); + server.seed('d0', [IMG_A, IMG_B]); // the draft as deposited (IMAP APPEND or an earlier save) + let state = initialDraftPartBlobState(); + let draftId = 'd0'; + const outgoing = () => draftWith([IMG_A, IMG_B].map((a) => ({ ...a, blobId: liveBlobId(state, a.blobId) }))); + for (let i = 0; i < 2; i++) { + const sent = outgoing(); + const saved = await saveDraftVersion(sent, 'drafts', draftId); + draftId = saved.id; + if (rebase) state = rebaseDraftPartBlobs(state, sent.attachments ?? [], saved.attachments); + } + return { server, draftId }; + } + + it('reproduces the bug without the rebase: the second save references the destroyed draft\'s parts', async () => { + await expect(editSession(false)).rejects.toThrow('blobId d0-part-a does not exist on this server.'); + }); + + it('keeps every save (and so the send, which builds the same attachments) on live blobs', async () => { + const { server, draftId } = await editSession(true); + expect([...server.emails.keys()]).toEqual([draftId]); + expect(server.emails.get(draftId)!.map((p) => p.name)).toEqual(['schema-a.png', 'schema-b.png']); + }); +}); diff --git a/src/api/__tests__/live-stalwart.test.ts b/src/api/__tests__/live-stalwart.test.ts index dbdb939c..197b7bcb 100644 --- a/src/api/__tests__/live-stalwart.test.ts +++ b/src/api/__tests__/live-stalwart.test.ts @@ -22,6 +22,8 @@ import { getFullEmails, getThreadsHeaders, createDraft, + saveDraftVersion, + destroyEmails, sendEmail, markAsSpam, undoSpam, @@ -33,6 +35,9 @@ import { EMAIL_FULL_PROPERTIES, } from '../email'; import { getIdentities } from '../identity'; +import { uploadBytes } from '../blob'; +import { initialDraftPartBlobState, liveBlobId, rebaseDraftPartBlobs } from '../../lib/draft-part-blobs'; +import type { OutgoingAttachment } from '../email'; import { getCalendars, queryEvents, createEvent, deleteEvents } from '../calendar'; import { getAddressBooks, createContact, deleteContacts } from '../contacts'; @@ -192,6 +197,44 @@ live('live Stalwart', () => { await destroyEmails([id]); }, 60_000); + it('keeps a re-opened draft with an inline image saveable across saves (#849 part blobs)', async () => { + // A 1x1 PNG uploaded once; D0 stands for the draft as it sits on the server. + const png = Uint8Array.from(atob('iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mP8z8BQDwAEhQGAhKmMIQAAAABJRU5ErkJggg=='), (c) => c.charCodeAt(0)); + const up = await uploadBytes(png, 'image/png'); + const base = { + from: [{ email: USER }], + to: [{ email: 'test@example.invalid' }], + subject: '[TEST bulwark-blob — supprimable]', + htmlBody: '

live

', + textBody: 'live', + }; + const img = (blobId: string): OutgoingAttachment => ({ + blobId, type: 'image/png', name: 'img1.png', size: png.byteLength, disposition: 'inline', cid: 'img1@live.test', + }); + const d0 = await saveDraftVersion({ ...base, attachments: [img(up.blobId)] }, drafts); + const created = [d0.id]; + try { + // Re-open D0: the composer holds D0's part blobId. + const seeded = d0.attachments[0].blobId; + let state = initialDraftPartBlobState(); + let draftId = d0.id; + for (let i = 0; i < 2; i++) { + const sent = [img(liveBlobId(state, seeded))]; + const saved = await saveDraftVersion({ ...base, attachments: sent }, drafts, draftId); + created.push(saved.id); + draftId = saved.id; + state = rebaseDraftPartBlobs(state, sent, saved.attachments); + } + // Without the rebase the seeded part blob is gone with D0. + await expect(saveDraftVersion({ ...base, attachments: [img(seeded)] }, drafts)) + .rejects.toThrow(/does not exist|blobNotFound/); + const [last] = await getFullEmails([draftId]); + expect(last.attachments?.[0]?.cid).toContain('img1@live.test'); + } finally { + await destroyEmails(created).catch(() => undefined); + } + }, 90_000); + it('sends through Drafts and files into Sent only after submission (#188)', async () => { const identities = await getIdentities(); expect(identities.length).toBeGreaterThan(0); diff --git a/src/api/email.ts b/src/api/email.ts index a02943da..dda49a24 100644 --- a/src/api/email.ts +++ b/src/api/email.ts @@ -1679,31 +1679,64 @@ export async function sendEmail( }; } +/** A saved draft version and the attachment parts the server built for it. */ +export interface SavedDraftVersion { + id: string; + /** + * Attachment and inline parts of the new version, with their blobIds. A + * part's blobId only lives as long as this version: re-opened drafts must + * send these (not the previous version's) on their next save or send. + * Empty when the server did not answer the read-back. + */ + attachments: Attachment[]; +} + +const DRAFT_PART_BODY_PROPERTIES = ['partId', 'blobId', 'size', 'name', 'type', 'cid', 'disposition']; + /** * Save a draft into the Drafts folder (`$draft` + `$seen`). When `previousDraftId` * is given it is destroyed only AFTER the replacement was created (#849): a * combined create+destroy would delete the last good copy when the create - * fails (e.g. blobNotFound on a re-opened draft's attachments). Returns the - * new draft id. + * fails (e.g. blobNotFound on a re-opened draft's attachments). The new + * version's attachment parts are read back in the same request, so the + * composer can re-point its draft-part blobIds before the old version (and + * its part blobs) is destroyed. */ -export async function createDraft( +export async function saveDraftVersion( email: OutgoingEmail, draftsMailboxId: string, previousDraftId?: string, accountIdOverride?: string, -): Promise { +): Promise { const accountId = accountIdOverride ?? jmapClient.accountId; const emailCreate = buildEmailCreate(email); emailCreate.mailboxIds = { [draftsMailboxId]: true }; emailCreate.keywords = { $seen: true, $draft: true }; const res = await jmapClient.request([ ['Email/set', { accountId, create: { draft: emailCreate } }, '0'], + // `#draft` is the creation id (RFC 8620 §5.3). Stalwart 0.16 does not + // resolve a result reference into an Email/set `created` map + // (invalidResultReference), but it does resolve the creation id. + ['Email/get', { + accountId, + ids: ['#draft'], + properties: ['id', 'attachments'], + bodyProperties: DRAFT_PART_BODY_PROPERTIES, + }, '1'], ]); const body = requireMethodResult(res, '0', 'Email/set'); const notCreated = body.notCreated?.draft as { description?: string; type?: string } | undefined; if (notCreated) throw new Error(notCreated.description || notCreated.type || 'Failed to save draft'); const id = body.created?.draft?.id as string | undefined; if (!id) throw new Error('Draft save returned no id'); + let attachments: Attachment[] = []; + try { + const got = requireMethodResult(res, '1', 'Email/get'); + const saved = (got.list as Array<{ id: string; attachments?: Attachment[] }> | undefined)?.find((e) => e.id === id); + attachments = (saved?.attachments ?? []).filter((a) => !!a.blobId); + } catch (err) { + console.warn('[email] could not read back the saved draft parts:', err); + } if (previousDraftId && previousDraftId !== id) { try { await destroyEmails([previousDraftId], accountId); @@ -1711,7 +1744,17 @@ export async function createDraft( console.warn('[email] failed to destroy previous draft version:', err); } } - return id; + return { id, attachments }; +} + +/** {@link saveDraftVersion} for callers that only need the new draft id. */ +export async function createDraft( + email: OutgoingEmail, + draftsMailboxId: string, + previousDraftId?: string, + accountIdOverride?: string, +): Promise { + return (await saveDraftVersion(email, draftsMailboxId, previousDraftId, accountIdOverride)).id; } export interface ScheduledEmail { diff --git a/src/lib/__tests__/draft-part-blobs.test.ts b/src/lib/__tests__/draft-part-blobs.test.ts new file mode 100644 index 00000000..d8b9b467 --- /dev/null +++ b/src/lib/__tests__/draft-part-blobs.test.ts @@ -0,0 +1,100 @@ +import { describe, it, expect } from 'vitest'; +import { + initialDraftPartBlobState, liveBlobId, matchDraftParts, rebaseDraftPartBlobs, withSnapshotBody, +} from '../draft-part-blobs'; +import type { OutgoingAttachment } from '../../api/email'; + +const inline = (blobId: string, cid: string, name = 'img.png', size = 10): OutgoingAttachment => ({ + blobId, cid, name, size, type: 'image/png', disposition: 'inline', +}); +const file = (blobId: string, name: string, size: number): OutgoingAttachment => ({ + blobId, name, size, type: 'application/pdf', disposition: 'attachment', +}); + +describe('matchDraftParts', () => { + it('pairs inline parts by Content-ID, brackets and case aside', () => { + const pairs = matchDraftParts( + [inline('old-a', 'a@x'), inline('old-b', 'b@x')], + [ + { blobId: 'new-b', cid: '', name: 'img.png', size: 10 }, + { blobId: 'new-a', cid: '', name: 'img.png', size: 10 }, + ], + ); + expect(pairs).toEqual(new Map([['old-a', 'new-a'], ['old-b', 'new-b']])); + }); + + it('pairs file attachments by name and size, claiming each new part once', () => { + const pairs = matchDraftParts( + [file('old-1', 'q.pdf', 5), file('old-2', 'q.pdf', 5), file('old-3', 'r.pdf', 7)], + [ + { blobId: 'new-r', name: 'r.pdf', size: 7 }, + { blobId: 'new-q1', name: 'q.pdf', size: 5 }, + { blobId: 'new-q2', name: 'q.pdf', size: 5 }, + ], + ); + expect(pairs.get('old-1')).toBe('new-q1'); + expect(pairs.get('old-2')).toBe('new-q2'); + expect(pairs.get('old-3')).toBe('new-r'); + }); + + it('leaves a part it cannot find unpaired', () => { + expect(matchDraftParts([file('old', 'gone.pdf', 1)], [{ blobId: 'n', name: 'other.pdf', size: 1 }]).size).toBe(0); + }); +}); + +describe('rebaseDraftPartBlobs (webmail #849, second half)', () => { + it('follows the draft across successive saves so no save sends a destroyed part', () => { + // Re-opened draft D0 with one inline image and one PDF. + let state = initialDraftPartBlobState(); + const sentAt = () => [inline(liveBlobId(state, 'd0-img'), 'img@x'), file(liveBlobId(state, 'd0-pdf'), 'q.pdf', 5)]; + + // Save 1 sends D0's parts (D0 is still alive), creates D1, destroys D0. + expect(sentAt().map((a) => a.blobId)).toEqual(['d0-img', 'd0-pdf']); + state = rebaseDraftPartBlobs(state, sentAt(), [ + { blobId: 'd1-img', cid: '', name: 'img.png', size: 10 }, + { blobId: 'd1-pdf', name: 'q.pdf', size: 5 }, + ]); + // Save 2 / send must reference D1's parts, not D0's dead ones. + expect(sentAt().map((a) => a.blobId)).toEqual(['d1-img', 'd1-pdf']); + state = rebaseDraftPartBlobs(state, sentAt(), [ + { blobId: 'd2-img', cid: '', name: 'img.png', size: 10 }, + { blobId: 'd2-pdf', name: 'q.pdf', size: 5 }, + ]); + expect(sentAt().map((a) => a.blobId)).toEqual(['d2-img', 'd2-pdf']); + }); + + it('re-points uploads too: an upload expires once nothing references it', () => { + let state = initialDraftPartBlobState(); + const sent = () => [inline(liveBlobId(state, 'd0-img'), 'img@x'), file(liveBlobId(state, 'upload-1'), 'new.pdf', 3)]; + state = rebaseDraftPartBlobs(state, sent(), [ + { blobId: 'd1-img', cid: '', name: 'img.png', size: 10 }, + { blobId: 'd1-new', name: 'new.pdf', size: 3 }, + ]); + expect(sent().map((a) => a.blobId)).toEqual(['d1-img', 'd1-new']); + }); + + it('leaves the state alone when nothing was sent', () => { + const state = initialDraftPartBlobState(); + expect(rebaseDraftPartBlobs(state, [], [{ blobId: 'p', name: 'a.pdf', size: 1 }])).toBe(state); + }); + + it('keeps the old id when the read-back came back empty (next save fails loudly, nothing is lost)', () => { + const state = initialDraftPartBlobState(); + const next = rebaseDraftPartBlobs(state, [inline('d0-img', 'img@x')], []); + expect(liveBlobId(next, 'd0-img')).toBe('d0-img'); + }); +}); + +describe('withSnapshotBody', () => { + it('replaces only the body of a composer snapshot', () => { + const snap = JSON.stringify({ to: [['A', 'a@x', null]], cc: [], bcc: [], subject: 'S', body: '

raw

', att: ['b1'] }); + expect(JSON.parse(withSnapshotBody(snap, '

normalised

'))).toEqual({ + to: [['A', 'a@x', null]], cc: [], bcc: [], subject: 'S', body: '

normalised

', att: ['b1'], + }); + }); + + it('keeps the key order, so an unchanged composer still compares equal', () => { + const make = (body: string) => JSON.stringify({ to: [], cc: [], bcc: [], subject: 'S', body, att: [] }); + expect(withSnapshotBody(make('

raw

'), '

x

')).toBe(make('

x

')); + }); +}); diff --git a/src/lib/draft-part-blobs.ts b/src/lib/draft-part-blobs.ts new file mode 100644 index 00000000..44291b82 --- /dev/null +++ b/src/lib/draft-part-blobs.ts @@ -0,0 +1,108 @@ +// Keep a re-opened draft's attachments pointing at blobs that still exist. +// +// A draft's attachments and inline images are MIME parts of that draft, and a +// part's blobId lives only as long as the message that holds it: once a save +// creates the next version and destroys the previous one, the server forgets +// the old part blobs (Stalwart answers `blobNotFound`, "blobId … does not +// exist on this server"). The composer was seeded with those part blobIds, so +// without a re-point the second save of a re-opened draft - and any send +// after its first save - failed (webmail #849, second half). +// +// Upload blobs are only a reprieve: the server keeps an unreferenced upload +// for a limited time, so every attachment is re-pointed at the parts of the +// version each save creates. + +import type { Attachment } from '../api/types'; +import type { OutgoingAttachment } from '../api/email'; +import { stripMessageIdBrackets } from './email-threading'; + +const normCid = (cid?: string | null) => (cid ? stripMessageIdBrackets(cid).trim().toLowerCase() : ''); + +/** + * Pair each sent attachment with the part of the newly created version that + * carries the same content: inline parts by Content-ID, the rest by name and + * size. Each new part is claimed at most once. Returns sent blobId → new blobId. + */ +export function matchDraftParts( + sent: readonly OutgoingAttachment[], + fresh: readonly Pick[], +): Map { + const out = new Map(); + const claimed = new Set(); + const claim = (s: OutgoingAttachment, test: (p: (typeof fresh)[number]) => boolean) => { + if (out.has(s.blobId)) return; + const i = fresh.findIndex((p, idx) => !claimed.has(idx) && !!p.blobId && test(p)); + if (i === -1) return; + claimed.add(i); + out.set(s.blobId, fresh[i].blobId); + }; + for (const s of sent) { + const cid = normCid(s.cid); + if (cid) claim(s, (p) => normCid(p.cid) === cid); + } + for (const s of sent) { + claim(s, (p) => (p.name || '') === (s.name || '') && (s.size == null || p.size === s.size)); + } + return out; +} + +export interface DraftPartBlobState { + /** blobId an attachment entry holds → blobId to send for it now. */ + live: Map; +} + +export function initialDraftPartBlobState(): DraftPartBlobState { + return { live: new Map() }; +} + +/** The blobId to put on the wire for an entry that holds `blobId`. */ +export function liveBlobId(state: DraftPartBlobState, blobId: string): string { + return state.live.get(blobId) ?? blobId; +} + +/** + * After a save: point every attachment that was sent at the matching part of + * the version just created. Those parts live exactly as long as the saved + * draft, so the next save or the send always references live blobs - whether + * the entry came from the previous version (dies with it) or from an upload + * (an unreferenced upload expires on the server). Blobs that could not be + * matched keep their id: the next save then fails loudly (and, with + * create-before-destroy, never loses the draft) instead of silently dropping + * an attachment. + */ +export function rebaseDraftPartBlobs( + state: DraftPartBlobState, + sent: readonly OutgoingAttachment[], + fresh: readonly Pick[], +): DraftPartBlobState { + const pairs = matchDraftParts(sent, fresh); + if (pairs.size === 0) return state; + const live = new Map(state.live); + const current = new Set(state.live.values()); + for (const [held, sentId] of state.live) { + const next = pairs.get(sentId); + if (next) live.set(held, next); + } + for (const [sentId, next] of pairs) { + // An entry sent under its own blobId for the first time. + if (!current.has(sentId)) live.set(sentId, next); + } + return { live }; +} + +/** + * The composer's dirty snapshot with its body replaced. Opening a draft whose + * inline images get hydrated rewrites the editor body (normalised DOM HTML, + * data URLs for `cid:` images); that rewrite is not an edit, so the baseline + * has to follow it - comparing against the raw server HTML instead made every + * re-opened draft with inline images look edited, and the autosave replaced + * it (and destroyed the opened version) two seconds after opening. + */ +export function withSnapshotBody(snapshot: string, body: string): string { + try { + const parsed = JSON.parse(snapshot) as Record; + return JSON.stringify({ ...parsed, body }); + } catch { + return snapshot; + } +} diff --git a/src/screens/ComposeScreen.tsx b/src/screens/ComposeScreen.tsx index 3ec09ce2..78a3b002 100644 --- a/src/screens/ComposeScreen.tsx +++ b/src/screens/ComposeScreen.tsx @@ -41,7 +41,7 @@ import { toast } from '../stores/toast-store'; import { type EmailTemplate } from '../stores/templates-store'; import { getIdentities } from '../api/identity'; import { - sendEmail, createDraft, destroyEmails, patchKeywordsForEmails, type OutgoingAttachment, type OutgoingEmail, + sendEmail, saveDraftVersion, destroyEmails, patchKeywordsForEmails, type OutgoingAttachment, type OutgoingEmail, } from '../api/email'; import { jmapClient, RequestTimeoutError, ScheduleTooLateError } from '../api/jmap-client'; import { uploadBlob, uploadBytes } from '../api/blob'; @@ -62,6 +62,9 @@ import { findComposeIdentityId, findDraftIdentityId, resolveComposeAccountEmail, resolveReplyIdentity, } from '../lib/reply-identity'; import { shouldBlockEditorRemoteImages } from '../lib/editor-html'; +import { + initialDraftPartBlobState, liveBlobId, rebaseDraftPartBlobs, withSnapshotBody, +} from '../lib/draft-part-blobs'; import { signatureIdentityFor, buildEmbeddedSignatureHtml, containsEmbeddedSignature, spliceSignature, insertSignatureAboveQuote, getPlainTextSignature, appendPlainTextSignature, @@ -684,6 +687,13 @@ export default function ComposeScreen({ route, navigation }: Props) { // Track inline-image placeholders that haven't yet been rewritten to cid: // until send time. Maps cid → blobId/type/name/size. const inlineRegistryRef = React.useRef>(new Map()); + // A re-opened draft's attachments and inline images are parts of that + // draft: their blobIds die with each version a save replaces (#849), and an + // upload expires once nothing references it. This maps the blobId each + // entry holds to the matching part of the latest saved version, which + // `buildOutgoing` sends. Kept out of the attachment state so the dirty + // snapshot does not change on every save. + const draftPartBlobsRef = React.useRef(initialDraftPartBlobState()); const getAutocomplete = useContactsStore((s) => s.getAutocomplete); const getGroupRecipients = useContactsStore((s) => s.getGroupRecipients); @@ -843,6 +853,10 @@ export default function ComposeScreen({ route, navigation }: Props) { } } if (cancelled || map.size === 0) return; + // No change message since the editor loaded = the user has not typed + // yet, so the editor still holds the seeded body (normalised by the + // WebView DOM, which is why it can differ from the raw server HTML). + const untouched = !plainTextMode && latestRef.current.bodyHtml === initialBodyHtml; let live = latestRef.current.bodyHtml; try { live = (await editorRef.current?.getHtml()) ?? live; @@ -853,8 +867,16 @@ export default function ComposeScreen({ route, navigation }: Props) { editorRef.current?.setHtml(next); } // The hydrated images are part of the untouched baseline, not an edit. - baselineRef.current = replaceInlineImagePlaceholders(baselineRef.current, map); - if (lastSavedRef.current) lastSavedRef.current = replaceInlineImagePlaceholders(lastSavedRef.current, map); + if (untouched) { + // Rebase on the body the composer now holds: comparing it with the + // raw server HTML made every re-opened draft with inline images look + // edited, and the autosave replaced (and destroyed) it unasked. + baselineRef.current = withSnapshotBody(baselineRef.current, next); + if (lastSavedRef.current) lastSavedRef.current = withSnapshotBody(lastSavedRef.current, next); + } else { + baselineRef.current = replaceInlineImagePlaceholders(baselineRef.current, map); + if (lastSavedRef.current) lastSavedRef.current = replaceInlineImagePlaceholders(lastSavedRef.current, map); + } })(); return () => { cancelled = true; }; // eslint-disable-next-line react-hooks/exhaustive-deps @@ -1224,7 +1246,7 @@ export default function ComposeScreen({ route, navigation }: Props) { .map((cid) => inlineRegistryRef.current.get(cid)) .filter((e): e is AttachmentEntry => !!e && !!e.blobId && !e.error) .map((e) => ({ - blobId: e.blobId!, + blobId: liveBlobId(draftPartBlobsRef.current, e.blobId!), type: e.type, name: e.name, size: e.size, @@ -1235,7 +1257,7 @@ export default function ComposeScreen({ route, navigation }: Props) { const fileAttachments = attachments .filter((a) => !a.inline && a.blobId && !a.error) .map((a) => ({ - blobId: a.blobId!, + blobId: liveBlobId(draftPartBlobsRef.current, a.blobId!), type: a.type, name: a.name, size: a.size, @@ -1295,8 +1317,14 @@ export default function ComposeScreen({ route, navigation }: Props) { setDraftStatus('saving'); try { const outgoing = buildOutgoing(identity, html, { forDraft: true }); - const id = await createDraft(outgoing, draftsMailbox.id, draftIdRef.current ?? undefined); + const saved = await saveDraftVersion(outgoing, draftsMailbox.id, draftIdRef.current ?? undefined); + const id = saved.id; draftIdRef.current = id; + // The previous version and its part blobs are gone: point every + // attachment at the parts of the version just created. + draftPartBlobsRef.current = rebaseDraftPartBlobs( + draftPartBlobsRef.current, outgoing.attachments ?? [], saved.attachments, + ); lastSavedRef.current = snapshot; lastSaveAtRef.current = Date.now(); setDraftStatus('saved');