fix: run the placeholder, attachment and confirm checks before a quick reply goes out

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<boolean>((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');

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<boolean> => \{\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('));
+  });
+});

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<boolean>;
+}
+
+/**
+ * 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<boolean> {
+  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;
+}

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<boolean>((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<boolean> => {
+    if (!useSettingsStore.getState().sendConfirmation) return Promise.resolve(true);
+    const count = [finalTo, finalCc, finalBcc].reduce((n, list) => n + expandRecipients(list).length, 0);
+    return new Promise<boolean>((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.