From db8eb731f141ed709b7e2e42054b9b285b1d131e Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Sat, 31 Oct 2026 03:33:16 +0100 Subject: [PATCH] fix: run the placeholder, attachment and confirm checks before a quick reply goes out --- src/components/email/QuickReplyBox.tsx | 80 +++++++++++++++- src/lib/__tests__/compose-send-checks.test.ts | 93 +++++++++++++++++++ src/lib/compose-send-checks.ts | 66 +++++++++++++ src/screens/ComposeScreen.tsx | 29 ++++-- 4 files changed, 256 insertions(+), 12 deletions(-) create mode 100644 src/lib/__tests__/compose-send-checks.test.ts create mode 100644 src/lib/compose-send-checks.ts diff --git a/src/components/email/QuickReplyBox.tsx b/src/components/email/QuickReplyBox.tsx index 420c8f77..662ba5e1 100644 --- a/src/components/email/QuickReplyBox.tsx +++ b/src/components/email/QuickReplyBox.tsx @@ -25,6 +25,8 @@ import { htmlToPlainText } from '../../lib/compose-html'; import { sendTargetFor } from '../../lib/compose-send-account'; import { emailDisplayDate } from '../../lib/email-date'; import { buildQuoteHeader, quoteHeaderLabels } from '../../lib/quote-header'; +import { passesQuickReplyChecks, type SendQuestion } from '../../lib/compose-send-checks'; +import { placeholderWordsFor } from '../../lib/placeholder-check'; interface Props { email: Email; @@ -63,6 +65,9 @@ export function QuickReplyBox({ email, jmapAccountId, onMoreOptions, onSent, onF const ownerLoginRef = React.useRef(useAccountStore.getState().activeAccountId); React.useEffect(() => { ownerLoginRef.current = useAccountStore.getState().activeAccountId; }, [email.id]); + // Taps while a send question is open do nothing. + const checkingRef = React.useRef(false); + const from = email.from?.[0]; if (!from?.email || email.keywords?.$draft) return null; @@ -75,7 +80,58 @@ export function QuickReplyBox({ email, jmapAccountId, onMoreOptions, onSent, onF return true; }; + // Asked as the composer asks, with "Back to editing" to stop the send. + const askBeforeSend = (question: SendQuestion) => new Promise((resolve) => { + const back = { text: t('email_composer.placeholder_warning.back', 'Back to editing'), style: 'cancel' as const, onPress: () => resolve(false) }; + const opts = { cancelable: true, onDismiss: () => resolve(false) }; + switch (question.kind) { + case 'placeholders': + Alert.alert( + t('email_composer.placeholder_warning.title', 'Check placeholders?'), + `${t( + 'email_composer.placeholder_warning.message', + 'Your message has placeholder text that recipients will see. Do you want to edit it before sending?', + )}\n\n${t('email_composer.placeholder_warning.found', 'Placeholders found')}: ${question.found.join(', ')}`, + [back, { text: t('email_composer.placeholder_warning.send_anyway', 'Send anyway'), onPress: () => resolve(true) }], + opts, + ); + return; + case 'attachment': + Alert.alert( + t('email_composer.attachment_reminder_title', 'Forgot an attachment?'), + t( + 'email_composer.attachment_reminder_body', + 'Your message mentions "{keyword}" but no file is attached. Send anyway?', + { keyword: question.keyword }, + ), + [back, { text: t('email_composer.send', 'Send'), onPress: () => resolve(true) }], + opts, + ); + return; + case 'send': + Alert.alert( + t('email_composer.send_confirm.title', 'Send this message?'), + t('email_composer.send_confirm.message', 'It goes to {count, plural, one {# recipient} other {# recipients}}.', { count: question.count }), + [ + { text: t('email_composer.cancel', 'Cancel'), style: 'cancel', onPress: () => resolve(false) }, + { text: t('email_composer.send', 'Send'), onPress: () => resolve(true) }, + ], + opts, + ); + } + }); + const send = async () => { + if (checkingRef.current) return; + checkingRef.current = true; + try { + await sendInner(); + } finally { + checkingRef.current = false; + } + }; + + const sendInner = async () => { const body = text.trim(); if (!body || sending) return; if (refuseAfterSwitch()) return; @@ -91,6 +147,25 @@ export function QuickReplyBox({ email, jmapAccountId, onMoreOptions, onSent, onF Alert.alert(t('common.error', 'Error'), t('email_viewer.unsubscribe_banner.no_identity', 'No sending identity available')); return; } + const recipients = buildReplyRecipients( + { from: email.from, replyToAddresses: email.replyTo, to: email.to, cc: email.cc }, + 'reply', + ownEmails, + ); + // The composer's placeholder warning, attachment reminder and "Confirm + // before sending" (webmail 534049952): a quick reply is no way around them. + const settings = useSettingsStore.getState(); + const passed = await passesQuickReplyChecks({ + body, + placeholderWords: settings.placeholderWarningEnabled ? placeholderWordsFor(locale) : null, + attachmentKeywords: settings.attachmentReminderEnabled ? settings.attachmentReminderKeywords : null, + confirmSend: settings.sendConfirmation, + recipientCount: [...recipients.to, ...recipients.cc].filter((r) => !!r.email).length, + ask: askBeforeSend, + }); + if (!passed) return; + // The questions wait for the user, long enough for a switch of logins. + if (refuseAfterSwitch()) return; setSending(true); try { // The account, identity and folders the reply goes out through, as the @@ -105,11 +180,6 @@ export function QuickReplyBox({ email, jmapAccountId, onMoreOptions, onSent, onF } // The lookup may have outlasted a switch of logins. if (refuseAfterSwitch()) return; - const recipients = buildReplyRecipients( - { from: email.from, replyToAddresses: email.replyTo, to: email.to, cc: email.cc }, - 'reply', - ownEmails, - ); const picked = pickEmailBody(email); const original = picked.text ?? (picked.html ? htmlToPlainText(picked.html) : plainTextBody(email)); const quoted = original.split('\n').map((l) => `> ${l}`).join('\n'); diff --git a/src/lib/__tests__/compose-send-checks.test.ts b/src/lib/__tests__/compose-send-checks.test.ts new file mode 100644 index 00000000..42daa0cd --- /dev/null +++ b/src/lib/__tests__/compose-send-checks.test.ts @@ -0,0 +1,93 @@ +import { describe, expect, it, vi } from 'vitest'; +import fs from 'node:fs'; +import path from 'node:path'; +import { findAttachmentKeyword, passesQuickReplyChecks, type SendQuestion } from '../compose-send-checks'; +import { placeholderWordsFor } from '../placeholder-check'; + +const read = (rel: string) => fs.readFileSync(path.resolve(__dirname, rel), 'utf8'); + +const base = { + placeholderWords: placeholderWordsFor('en'), + attachmentKeywords: ['attached', 'attachment'], + confirmSend: false, + recipientCount: 1, +}; + +describe('findAttachmentKeyword', () => { + it('matches whole words only, ignoring case and blanks', () => { + expect(findAttachmentKeyword('See the ATTACHED file', ['', 'attached'])).toBe('attached'); + expect(findAttachmentKeyword('The detached house', ['attach', 'tached'])).toBeUndefined(); + expect(findAttachmentKeyword('price (c++)', ['c++'])).toBeUndefined(); + }); +}); + +describe('passesQuickReplyChecks (webmail lib/outgoing/quick-reply.ts)', () => { + it('asks about unfilled placeholders and stops on Back', async () => { + const ask = vi.fn(async () => false); + const ok = await passesQuickReplyChecks({ ...base, body: 'See you at [TIME].', ask }); + expect(ok).toBe(false); + expect(ask).toHaveBeenCalledWith({ kind: 'placeholders', found: ['[TIME]'] }); + }); + + it('asks about a forgotten attachment', async () => { + const asked: SendQuestion[] = []; + const ok = await passesQuickReplyChecks({ + ...base, + body: 'Here is the attachment.', + ask: async (q) => { asked.push(q); return true; }, + }); + expect(ok).toBe(true); + expect(asked).toEqual([{ kind: 'attachment', keyword: 'attachment' }]); + }); + + it('asks "Send this message?" last, with the recipient count', async () => { + const asked: SendQuestion[] = []; + const ok = await passesQuickReplyChecks({ + ...base, + body: 'TODO attached', + confirmSend: true, + recipientCount: 3, + ask: async (q) => { asked.push(q); return true; }, + }); + expect(ok).toBe(true); + expect(asked.map((q) => q.kind)).toEqual(['placeholders', 'attachment', 'send']); + expect(asked[2]).toEqual({ kind: 'send', count: 3 }); + }); + + it('sends without asking when the warnings are off and nothing matches', async () => { + const ask = vi.fn(async () => false); + expect(await passesQuickReplyChecks({ + body: 'TODO [TIME] attached', placeholderWords: null, attachmentKeywords: null, confirmSend: false, recipientCount: 1, ask, + })).toBe(true); + expect(await passesQuickReplyChecks({ ...base, body: 'Thanks, see you then.', ask })).toBe(true); + expect(ask).not.toHaveBeenCalled(); + }); + + it('stops when the confirm is declined', async () => { + expect(await passesQuickReplyChecks({ ...base, body: 'Thanks', confirmSend: true, ask: async () => false })).toBe(false); + }); +}); + +describe('send checks wiring', () => { + it('runs the checks in quick reply before anything is sent', () => { + const source = read('../../components/email/QuickReplyBox.tsx'); + const checks = source.indexOf('await passesQuickReplyChecks('); + expect(checks).toBeGreaterThan(-1); + expect(checks).toBeLessThan(source.indexOf('await sendEmail(')); + expect(checks).toBeLessThan(source.indexOf('setSending(true)')); + const call = source.slice(checks, source.indexOf('if (!passed) return;', checks)); + expect(call).toMatch(/placeholderWarningEnabled \? placeholderWordsFor\(locale\) : null/); + expect(call).toMatch(/attachmentReminderEnabled \? settings\.attachmentReminderKeywords : null/); + expect(call).toMatch(/confirmSend: settings\.sendConfirmation/); + }); + + it('asks "Send this message?" in the composer after the other checks when the setting is on', () => { + const source = read('../../screens/ComposeScreen.tsx'); + const fn = source.slice(source.indexOf('const passesSendConfirmation')); + expect(fn).toMatch(/^const passesSendConfirmation = \(\): Promise => \{\s+if \(!useSettingsStore\.getState\(\)\.sendConfirmation\) return Promise\.resolve\(true\);/); + const inner = source.slice(source.indexOf('const performSendInner')); + const confirm = inner.indexOf('await passesSendConfirmation()'); + expect(confirm).toBeGreaterThan(inner.indexOf('await passesAttachmentReminder(')); + expect(confirm).toBeLessThan(inner.indexOf('await sendEmail(')); + }); +}); diff --git a/src/lib/compose-send-checks.ts b/src/lib/compose-send-checks.ts new file mode 100644 index 00000000..d085980d --- /dev/null +++ b/src/lib/compose-send-checks.ts @@ -0,0 +1,66 @@ +// The questions the composer asks before a message goes out, shared with +// quick reply so it is no way around them (webmail lib/outgoing/quick-reply.ts, +// 534049952). + +import { unfilledPlaceholdersInMessage, type PlaceholderWords } from './placeholder-check'; + +/** A question asked before sending. */ +export type SendQuestion = + | { kind: 'placeholders'; found: string[] } + | { kind: 'attachment'; keyword: string } + | { kind: 'send'; count: number }; + +/** + * The first attachment keyword in `text`, matched as a whole word so + * "attach" does not fire on "detached". Case-insensitive; blank keywords + * are skipped. + */ +export function findAttachmentKeyword(text: string, keywords: readonly string[]): string | undefined { + const haystack = text.toLowerCase(); + return keywords.find((kw) => { + const k = kw.trim().toLowerCase(); + if (!k) return false; + return new RegExp(`\\b${k.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')}\\b`).test(haystack); + }); +} + +export interface QuickReplyChecks { + /** The typed reply, without the quote and the signature. */ + body: string; + /** The fill-in words of the placeholder warning; null when it is off. */ + placeholderWords: PlaceholderWords[] | null; + /** The keywords of the attachment reminder; null when it is off. */ + attachmentKeywords: readonly string[] | null; + /** "Confirm before sending" is on. */ + confirmSend: boolean; + /** How many addresses the reply goes to, for the confirm. */ + recipientCount: number; + /** Resolves true to send anyway. */ + ask: (question: SendQuestion) => Promise; +} + +/** + * The composer's warnings for a quick reply, in the composer's order: + * placeholders, the forgotten attachment, then "Send this message?" last so + * an answered warning doesn't ask twice. Only the typed text counts: the + * subject and the signature are not the user's words here, and a quick reply + * carries no attachments, so any keyword is a reminder. Resolves true when + * the reply may go out. + */ +export async function passesQuickReplyChecks(input: QuickReplyChecks): Promise { + if (input.placeholderWords) { + const found = unfilledPlaceholdersInMessage({ + subject: '', + body: input.body, + plainTextMode: true, + words: input.placeholderWords, + }); + if (found.length > 0 && !(await input.ask({ kind: 'placeholders', found }))) return false; + } + if (input.attachmentKeywords) { + const keyword = findAttachmentKeyword(input.body, input.attachmentKeywords); + if (keyword && !(await input.ask({ kind: 'attachment', keyword }))) return false; + } + if (input.confirmSend && !(await input.ask({ kind: 'send', count: input.recipientCount }))) return false; + return true; +} diff --git a/src/screens/ComposeScreen.tsx b/src/screens/ComposeScreen.tsx index ceb25ef1..5604df87 100644 --- a/src/screens/ComposeScreen.tsx +++ b/src/screens/ComposeScreen.tsx @@ -68,6 +68,7 @@ import { import { shouldBlockEditorRemoteImages } from '../lib/editor-html'; import { placeholderWordsFor, unfilledPlaceholdersInMessage } from '../lib/placeholder-check'; import { sendTargetFor } from '../lib/compose-send-account'; +import { findAttachmentKeyword } from '../lib/compose-send-checks'; import { inlineImageType } from '../lib/compose-inline-type'; import { attachmentFromFileNode } from '../lib/compose-files'; import { supportsFiles } from '../api/files'; @@ -2084,13 +2085,7 @@ export default function ComposeScreen({ route, navigation }: Props) { plainTextMode, forwardedSeparator: quoteLabels.forwardedSeparator, }); - const haystack = `${subject}\n${authored}`.toLowerCase(); - const matchedKeyword = attachmentReminderKeywords.find((kw) => { - const k = kw.trim().toLowerCase(); - if (!k) return false; - // Word-boundary match so 'attach' doesn't fire on 'detached'. - return new RegExp(`\\b${k.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')}\\b`).test(haystack); - }); + const matchedKeyword = findAttachmentKeyword(`${subject}\n${authored}`, attachmentReminderKeywords); if (!matchedKeyword) return true; return new Promise((resolve) => { Alert.alert( @@ -2175,6 +2170,25 @@ export default function ComposeScreen({ route, navigation }: Props) { }); }; + // "Send this message?" when "Confirm before sending" is on (webmail + // 4d91d9b29). Asked after every other check, so a warning answered first + // doesn't ask it twice. + const passesSendConfirmation = (): Promise => { + if (!useSettingsStore.getState().sendConfirmation) return Promise.resolve(true); + const count = [finalTo, finalCc, finalBcc].reduce((n, list) => n + expandRecipients(list).length, 0); + return new Promise((resolve) => { + Alert.alert( + t('email_composer.send_confirm.title', 'Send this message?'), + t('email_composer.send_confirm.message', 'It goes to {count, plural, one {# recipient} other {# recipients}}.', { count }), + [ + { text: t('email_composer.cancel', 'Cancel'), style: 'cancel', onPress: () => resolve(false) }, + { text: t('email_composer.send', 'Send'), onPress: () => resolve(true) }, + ], + { cancelable: true, onDismiss: () => resolve(false) }, + ); + }); + }; + // Translate an absolute "send at" time into the HOLDFOR seconds the server // expects, clamping to what it actually supports. Returns null (and alerts) // when scheduling isn't possible so the caller can abort. @@ -2326,6 +2340,7 @@ export default function ComposeScreen({ route, navigation }: Props) { if (!(await passesEmptySubjectCheck())) return; if (!(await passesPlaceholderCheck(plainTextMode ? plainBody : liveBodyHtml))) return; if (!(await passesAttachmentReminder(plainTextMode ? plainBody : liveBodyHtml))) return; + if (!(await passesSendConfirmation())) return; // Autosave must not race the send: cancel a scheduled save and let an // in-flight one finish so its version can be destroyed after the send.