From c7d551f185ab30c9c7a4e0df2e37260c204fc8ba Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Tue, 7 Jul 2026 23:54:50 +0200 Subject: [PATCH] fix: attachment reminder ignores quoted text on reply/forward #570 --- components/email/email-composer.tsx | 9 ++- lib/__tests__/email-composer-utils.test.ts | 73 ++++++++++++++++++++++ lib/email-composer-utils.ts | 55 ++++++++++++++++ 3 files changed, 136 insertions(+), 1 deletion(-) diff --git a/components/email/email-composer.tsx b/components/email/email-composer.tsx index ef5bb310..c2b231f1 100644 --- a/components/email/email-composer.tsx +++ b/components/email/email-composer.tsx @@ -46,6 +46,7 @@ import { formatRecipientList, splitPastedRecipients, waitForPendingUploads, + extractUserAuthoredText, type Recipient, } from "@/lib/email-composer-utils"; import { isValidEmail } from "@/lib/validation"; @@ -1599,7 +1600,13 @@ export function EmailComposer({ if (!skipAttachmentCheck && attachmentReminderEnabled) { const hasAttachments = attachmentsRef.current.some(att => att.blobId && !att.uploading && !att.error); if (!hasAttachments) { - const bodyText = htmlToPlainText(body); + // Scan only the user-authored text: the quoted original of a + // reply/forward often mentions an attachment itself, which used to fire + // the reminder even when the user typed no keyword and added nothing (#570). + const bodyText = extractUserAuthoredText(body, { + plainTextMode, + forwardedSeparator: tQuote('forwarded_separator'), + }); const searchText = `${subject} ${bodyText}`.toLowerCase(); const matched = attachmentReminderKeywords.find(kw => searchText.includes(kw.toLowerCase())); if (matched) { diff --git a/lib/__tests__/email-composer-utils.test.ts b/lib/__tests__/email-composer-utils.test.ts index f2d742a8..36956d94 100644 --- a/lib/__tests__/email-composer-utils.test.ts +++ b/lib/__tests__/email-composer-utils.test.ts @@ -11,8 +11,81 @@ import { formatRecipientList, splitPastedRecipients, waitForPendingUploads, + extractUserAuthoredText, } from "../email-composer-utils"; +const FORWARDED_SEPARATOR = "---------- Forwarded message ----------"; + +describe("extractUserAuthoredText", () => { + const scan = (body: string, plainTextMode: boolean) => + extractUserAuthoredText(body, { + plainTextMode, + forwardedSeparator: FORWARDED_SEPARATOR, + }).toLowerCase(); + + it("keeps user text and drops the quoted island on an HTML reply (#570)", () => { + const body = + "

Here is my reply.

" + + '
On Mon, Someone wrote:
' + + '

Please find attached the invoice (anexo).

'; + const result = scan(body, false); + expect(result).toContain("here is my reply"); + expect(result).not.toContain("anexo"); + expect(result).not.toContain("attached"); + }); + + it("drops a
quote when the original had no HTML part", () => { + const body = + "

Thanks!

" + + '
segue em anexo o documento
'; + const result = scan(body, false); + expect(result).toContain("thanks"); + expect(result).not.toContain("anexo"); + }); + + it("drops the forwarded header and original on an HTML forward", () => { + const body = + "

FYI



" + + FORWARDED_SEPARATOR + + "
From: a@b.com
Subject: Invoice attached

" + + '

em anexo

'; + const result = scan(body, false); + expect(result).toContain("fyi"); + expect(result).not.toContain("anexo"); + expect(result).not.toContain("attached"); + expect(result).not.toContain("forwarded message"); + }); + + it("drops '>' quoted lines on a plain-text reply", () => { + const body = "My reply here.\n\nOn Mon, X wrote:\n> please find attached\n> anexo"; + const result = scan(body, true); + expect(result).toContain("my reply here"); + expect(result).not.toContain("attached"); + expect(result).not.toContain("anexo"); + }); + + it("drops the bare forwarded original on a plain-text forward", () => { + const body = + "See below.\n\n" + + FORWARDED_SEPARATOR + + "\nFrom: a@b.com\nSubject: hi\n\nem anexo o contrato"; + const result = scan(body, true); + expect(result).toContain("see below"); + expect(result).not.toContain("anexo"); + }); + + it("still surfaces a keyword the user actually typed", () => { + const body = + "

See the attached file.

" + + '

nothing here

'; + expect(scan(body, false)).toContain("attached"); + }); + + it("tolerates a missing forwarded separator", () => { + expect(scan("

plain reply

", false)).toContain("plain reply"); + }); +}); + describe("plainTextToComposerBody", () => { it("returns an empty string for empty input", () => { expect(plainTextToComposerBody("")).toBe(""); diff --git a/lib/email-composer-utils.ts b/lib/email-composer-utils.ts index f927ddd2..d3f0555c 100644 --- a/lib/email-composer-utils.ts +++ b/lib/email-composer-utils.ts @@ -1,4 +1,5 @@ import { isValidEmail } from "@/lib/validation"; +import { htmlToPlainText } from "@/lib/html-to-text"; const HTML_ESCAPE_MAP = { "&": "&", @@ -54,6 +55,60 @@ export function rewriteCidImagesForEditor(html: string): string { return touched ? doc.body.innerHTML : html; } +/** + * Reduce a composer body to just the user-authored text for the attachment + * reminder's keyword scan, dropping the quoted original of a reply/forward. + * + * Scanning the whole body triggered false positives whenever the quoted message + * mentioned an attachment - common, since the original often did carry one, and + * the default keyword list is broad and multilingual (#570). We strip: + * - HTML mode: the QuotedHtml island ([data-quoted-html]) and any
+ * (the wrapper used when the original had no HTML part), then convert to text. + * - Plain-text mode: lines prefixed with ">" (the reply quote). + * - Both modes: everything from the "Forwarded message" separator onward, which + * also removes the forwarded From/Date/Subject header lines and the bare + * forwarded original (which carries no blockquote/island wrapper). + * + * `forwardedSeparator` is the localized quote_header.forwarded_separator string; + * pass it so the forward cut works in the active locale. + */ +export function extractUserAuthoredText( + body: string, + options: { plainTextMode: boolean; forwardedSeparator?: string } +): string { + const { plainTextMode, forwardedSeparator } = options; + + let text: string; + if (plainTextMode) { + text = body + .split("\n") + .filter((line) => !/^\s*>/.test(line)) + .join("\n"); + } else { + const doc = new DOMParser().parseFromString(`${body}`, "text/html"); + doc + .querySelectorAll("[data-quoted-html], blockquote") + .forEach((el) => el.remove()); + text = htmlToPlainText(doc.body.innerHTML, { paragraphSpacing: true }); + } + + // Cut everything from the forwarded-message separator onward. htmlToPlainText + // collapses the separator's internal whitespace, so match with a + // whitespace-flexible, regex-escaped pattern rather than an exact string. + const trimmedSeparator = forwardedSeparator?.trim(); + if (trimmedSeparator) { + const pattern = trimmedSeparator + .replace(/[.*+?^${}()|[\]\\]/g, "\\$&") + .replace(/\s+/g, "\\s+"); + const match = text.match(new RegExp(pattern)); + if (match && match.index !== undefined) { + text = text.slice(0, match.index); + } + } + + return text; +} + /** A composer recipient. Display name is optional; email is required. */ export type Recipient = { name?: string; email: string };