Switching format keeps the original quote, not a flattened copy (#409) (#409)

Switching a reply between plain text and rich text converted whatever
body the draft was showing. Going from plain text to rich, that meant
the quoted message came back as the "> " text quote run through a
converter -- the sender's formatting, images and links gone, even though
the original markup was sitting on the draft untouched.

Both forms of the quote are prepared when the reply opens, so keep them
on the draft and re-attach the right one when the format changes. Only
what the author typed above the quote is converted. Where the quote
can't be found any more -- edited by hand, or a draft that quotes
nothing -- the whole body is converted as before, which is what every
non-reply draft does.

No new strings.

web/src/store/__tests__/compose-email.test.ts

@@ -18,7 +18,7 @@ function draft(over: Partial<Draft> = {}): Draft {
     requestReceipt: false, priority: "normal",
     showCc: false, showBcc: false, showReplyTo: false,
     minimized: false, maximized: false, dirty: false, savedAt: null,
-    saving: false, sending: false, error: null, signatureHtml: "", replyMode: null, formatOffer: null, sendAt: null,
+    saving: false, sending: false, error: null, signatureHtml: "", replyMode: null, quoteHtml: "", quoteText: "", formatOffer: null, sendAt: null,
     ...over,
   };
 }

web/src/store/__tests__/reply-format-offer.test.ts

@@ -98,6 +98,24 @@ describe("answering a message written in the other format", () => {
     expect(d.html).toContain("hi");
   });
 
+  it("keeps the quoted message in both formats, so a switch can restore it", async () => {
+    composeIn("text");
+    const d = await draftFor(RICH, "reply");
+    // The HTML quote is the original's markup, not the text one converted.
+    expect(d.quoteHtml).toContain("<p>hi</p>");
+    expect(d.quoteHtml).toContain("ihm-quote");
+    expect(d.quoteText).toContain("Ann");
+    expect(d.text.endsWith(d.quoteText)).toBe(true);
+  });
+
+  it("quotes nothing on a message started from scratch", () => {
+    composeIn("text");
+    const key = useCompose.getState().open();
+    const d = useCompose.getState().drafts.find((x) => x.key === key)!;
+    expect(d.quoteHtml).toBe("");
+    expect(d.quoteText).toBe("");
+  });
+
   it("makes no offer on a message started from scratch", () => {
     composeIn("text");
     const key = useCompose.getState().open();

web/src/store/compose.ts

@@ -75,6 +75,14 @@ export interface Draft {
   /** Original identity signature HTML currently embedded, to replace on identity switch. */
   signatureHtml: string;
   replyMode: "reply" | "replyAll" | "forward" | null;
+  /**
+   * The quoted message as it was prepared in each format, kept so that
+   * switching format re-attaches the original rather than a conversion of
+   * whatever the other format flattened it into. Empty on a draft that quotes
+   * nothing.
+   */
+  quoteHtml: string;
+  quoteText: string;
   /**
    * The format the message being answered was written in, when it is not the
    * one this draft opened in (#407). The composer offers the switch; answering
@@ -151,6 +159,8 @@ function blankDraft(init: Partial<Draft> = {}): Draft {
     error: null,
     signatureHtml: "",
     replyMode: null,
+    quoteHtml: "",
+    quoteText: "",
     formatOffer: null,
     sendAt: null,
     ...init,
@@ -448,6 +458,8 @@ export const useCompose = create<ComposeState>((set, get) => ({
       relatedKeyword: mode === "forward" ? "$forwarded" : "$answered",
       signatureHtml: sigHtml,
       replyMode: mode,
+      quoteHtml,
+      quoteText: quoteTxt,
       formatOffer: origFormat === s.composeFormat ? null : origFormat,
     });
     set((st) => ({ drafts: [...st.drafts, d], activeKey: d.key }));

web/src/views/compose/Composer.tsx

@@ -147,12 +147,26 @@ export function Composer({ draft }: { draft: Draft }) {
     patch({ sendAt: at.getTime() });
   };
 
+  /*
+   * Switching format converts what has been written, but the quoted message
+   * is not something this draft wrote: it was prepared in both formats when
+   * the reply opened. Converting the plain-text quote into HTML would hand
+   * back a flattened copy of a message that still exists in its original
+   * markup, so re-attach that instead, and keep only what the author typed
+   * above it. Where the quote can no longer be found -- edited, or a draft
+   * that quotes nothing -- convert the whole body as before.
+   */
   const toggleFormat = () => {
     // Whichever way the format is changed, the offer has been answered.
     if (d.format === "html") {
-      patch({ format: "text", text: htmlToText(d.html), formatOffer: null });
+      const at = d.quoteHtml ? d.html.indexOf('<div class="ihm-quote">') : -1;
+      const written = at >= 0 ? htmlToText(d.html.slice(0, at)) : htmlToText(d.html);
+      patch({ format: "text", text: at >= 0 ? written.replace(/\s+$/, "") + d.quoteText : written, formatOffer: null });
     } else {
-      patch({ format: "html", html: textToHtml(d.text, { linkify: false, quoteColors: false }).replace(/\n/g, "<br>"), formatOffer: null });
+      const keeps = Boolean(d.quoteText) && d.text.endsWith(d.quoteText);
+      const written = keeps ? d.text.slice(0, d.text.length - d.quoteText.length) : d.text;
+      const asHtml = textToHtml(written, { linkify: false, quoteColors: false }).replace(/\n/g, "<br>");
+      patch({ format: "html", html: keeps ? asHtml + d.quoteHtml : asHtml, formatOffer: null });
     }
   };
 

web/src/views/compose/__tests__/format-offer-bar.test.tsx

@@ -15,9 +15,13 @@ import { useMail } from "@/store/mail";
 
 window.matchMedia = ((q: string) => ({ matches: false, media: q, addEventListener() {}, removeEventListener() {} })) as unknown as typeof window.matchMedia;
 
+const QUOTE_HTML = '<div class="ihm-quote"><br><div>On Friday, Ann wrote:</div><blockquote><p>Look at <b>this</b></p></blockquote></div>';
+const QUOTE_TEXT = "\n\nOn Friday, Ann wrote:\n> Look at this";
+
 const REPLY: Partial<Draft> = {
   key: "d1", replyMode: "reply", subject: "Re: Numbers",
-  format: "text", text: "\n\nOn Friday, Ann wrote:\n> hi", html: "<div><br></div><div class=\"ihm-quote\">hi</div>",
+  format: "text", text: QUOTE_TEXT, html: `<div><br></div>${QUOTE_HTML}`,
+  quoteHtml: QUOTE_HTML, quoteText: QUOTE_TEXT,
   formatOffer: "html",
 };
 
@@ -54,6 +58,46 @@ describe("the format offer in the composer", () => {
     expect(bar()).toBeNull();
   });
 
+  /*
+   * The message being quoted was prepared in both formats when the reply
+   * opened. Switching used to convert the plain-text body it had, handing
+   * back a flattened copy -- "> Look at this" -- of markup that still
+   * existed untouched on the draft.
+   */
+  it("restores the original message, rather than converting the flattened quote", () => {
+    act(() => button("Switch to rich text").click());
+    act(() => root.render(<Composer draft={draft()} />));
+    expect(draft().html).toContain("<b>this</b>");
+    expect(draft().html).toContain("<blockquote>");
+    expect(draft().html).not.toContain("&gt; Look at this");
+  });
+
+  it("keeps what the author typed above the quote", () => {
+    useCompose.getState().update("d1", { text: `Thanks, that helps.${QUOTE_TEXT}` });
+    act(() => root.render(<Composer draft={draft()} />));
+    act(() => button("Switch to rich text").click());
+    act(() => root.render(<Composer draft={draft()} />));
+    expect(draft().html).toContain("Thanks, that helps.");
+    expect(draft().html).toContain("<b>this</b>");
+    // Once only: the typed reply must not arrive with the quote doubled.
+    expect(draft().html.match(/On Friday, Ann wrote:/g)).toHaveLength(1);
+  });
+
+  it("goes back to plain text with the prepared quote, not a re-flattened one", () => {
+    // A rich draft answering a plain-text message: the offer runs the other way.
+    act(() => {
+      useCompose.getState().update("d1", { format: "html", html: `<div>Thanks.</div>${QUOTE_HTML}`, formatOffer: "text" });
+    });
+    act(() => root.render(<Composer draft={draft()} />));
+    act(() => button("Switch to plain text").click());
+    act(() => root.render(<Composer draft={draft()} />));
+    expect(draft().format).toBe("text");
+    expect(draft().text).toContain("Thanks.");
+    // The prepared plain-text quote, not HTML run through a converter.
+    expect(draft().text.endsWith(QUOTE_TEXT)).toBe(true);
+    expect(draft().text).not.toContain("<blockquote>");
+  });
+
   it("dismisses without changing the format", () => {
     act(() => button("Dismiss").click());
     act(() => root.render(<Composer draft={draft()} />));