fix: attachment reminder ignores quoted text on reply/forward #570
This commit is contained in:
@@ -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) {
|
||||
|
||||
@@ -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 =
|
||||
"<p>Here is my reply.</p>" +
|
||||
'<div>On Mon, Someone wrote:</div>' +
|
||||
'<div data-quoted-html><p>Please find attached the invoice (anexo).</p></div>';
|
||||
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 <blockquote> quote when the original had no HTML part", () => {
|
||||
const body =
|
||||
"<p>Thanks!</p>" +
|
||||
'<blockquote>segue em anexo o documento</blockquote>';
|
||||
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 =
|
||||
"<p>FYI</p><br><br>" +
|
||||
FORWARDED_SEPARATOR +
|
||||
"<br>From: a@b.com<br>Subject: Invoice attached<br><br>" +
|
||||
'<div data-quoted-html><p>em anexo</p></div>';
|
||||
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 =
|
||||
"<p>See the attached file.</p>" +
|
||||
'<div data-quoted-html><p>nothing here</p></div>';
|
||||
expect(scan(body, false)).toContain("attached");
|
||||
});
|
||||
|
||||
it("tolerates a missing forwarded separator", () => {
|
||||
expect(scan("<p>plain reply</p>", false)).toContain("plain reply");
|
||||
});
|
||||
});
|
||||
|
||||
describe("plainTextToComposerBody", () => {
|
||||
it("returns an empty string for empty input", () => {
|
||||
expect(plainTextToComposerBody("")).toBe("");
|
||||
|
||||
@@ -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 <blockquote>
|
||||
* (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>${body}</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 };
|
||||
|
||||
|
||||
Reference in New Issue
Block a user