Split up bodyToHtml (#12840)

* Split up bodyToHtml

This (very incrementally) splits up the bodyToHtml function to avoid
the multiple return types and hopefully make it a touch easier to
comprehend. I'd also like to see what the test coverage says about
this, so this is is somewhat experimental. This shouldn't change
any behaviour but the comments in this function indiciate just how
subtle it is.

* Remove I prefix

* Missed emoji formatting part

src/HtmlUtils.tsx

@@ -246,23 +246,6 @@ class HtmlHighlighter extends BaseHighlighter<string> {
     }
 }
 
-interface IOpts {
-    highlightLink?: string;
-    disableBigEmoji?: boolean;
-    stripReplyFallback?: boolean;
-    returnString?: boolean;
-    forComposerQuote?: boolean;
-    ref?: React.Ref<HTMLSpanElement>;
-}
-
-export interface IOptsReturnNode extends IOpts {
-    returnString?: false | undefined;
-}
-
-export interface IOptsReturnString extends IOpts {
-    returnString: true;
-}
-
 const emojiToHtmlSpan = (emoji: string): string =>
     `<span class='mx_Emoji' title='${unicodeToShortcode(emoji)}'>${emoji}</span>`;
 const emojiToJsxSpan = (emoji: string, key: number): JSX.Element => (
@@ -307,35 +290,36 @@ export function formatEmojis(message: string | undefined, isHtmlMessage?: boolea
     return result;
 }
 
-/* turn a matrix event body into html
- *
- * content: 'content' of the MatrixEvent
- *
- * highlights: optional list of words to highlight, ordered by longest word first
- *
- * opts.highlightLink: optional href to add to highlighted words
- * opts.disableBigEmoji: optional argument to disable the big emoji class.
- * opts.stripReplyFallback: optional argument specifying the event is a reply and so fallback needs removing
- * opts.returnString: return an HTML string rather than JSX elements
- * opts.forComposerQuote: optional param to lessen the url rewriting done by sanitization, for quoting into composer
- * opts.ref: React ref to attach to any React components returned (not compatible with opts.returnString)
- */
-export function bodyToHtml(content: IContent, highlights: Optional<string[]>, opts: IOptsReturnString): string;
-export function bodyToHtml(content: IContent, highlights: Optional<string[]>, opts: IOptsReturnNode): ReactNode;
-export function bodyToHtml(content: IContent, highlights: Optional<string[]>, opts: IOpts = {}): ReactNode | string {
-    const isFormattedBody = content.format === "org.matrix.custom.html" && typeof content.formatted_body === "string";
-    let bodyHasEmoji = false;
-    let isHtmlMessage = false;
+interface EventAnalysis {
+    bodyHasEmoji: boolean;
+    isHtmlMessage: boolean;
+    strippedBody: string;
+    safeBody?: string; // safe, sanitised HTML, preferred over `strippedBody` which is fully plaintext
+    isFormattedBody: boolean;
+}
 
+export interface EventRenderOpts {
+    highlightLink?: string;
+    disableBigEmoji?: boolean;
+    stripReplyFallback?: boolean;
+    forComposerQuote?: boolean;
+    ref?: React.Ref<HTMLSpanElement>;
+}
+
+function analyseEvent(content: IContent, highlights: Optional<string[]>, opts: EventRenderOpts = {}): EventAnalysis {
     let sanitizeParams = sanitizeHtmlParams;
     if (opts.forComposerQuote) {
         sanitizeParams = composerSanitizeHtmlParams;
     }
 
-    let strippedBody: string;
-    let safeBody: string | undefined; // safe, sanitised HTML, preferred over `strippedBody` which is fully plaintext
-
     try {
+        const isFormattedBody =
+            content.format === "org.matrix.custom.html" && typeof content.formatted_body === "string";
+        let bodyHasEmoji = false;
+        let isHtmlMessage = false;
+
+        let safeBody: string | undefined; // safe, sanitised HTML, preferred over `strippedBody` which is fully plaintext
+
         // sanitizeHtml can hang if an unclosed HTML tag is thrown at it
         // A search for `<foo` will make the browser crash an alternative would be to escape HTML special characters
         // but that would bring no additional benefit as the highlighter does not work with those special chars
@@ -347,7 +331,7 @@ export function bodyToHtml(content: IContent, highlights: Optional<string[]>, op
         const plainBody = typeof content.body === "string" ? content.body : "";
 
         if (opts.stripReplyFallback && formattedBody) formattedBody = stripHTMLReply(formattedBody);
-        strippedBody = opts.stripReplyFallback ? stripPlainReply(plainBody) : plainBody;
+        const strippedBody = opts.stripReplyFallback ? stripPlainReply(plainBody) : plainBody;
         bodyHasEmoji = mightContainEmoji(isFormattedBody ? formattedBody! : plainBody);
 
         const highlighter = safeHighlights?.length
@@ -384,13 +368,19 @@ export function bodyToHtml(content: IContent, highlights: Optional<string[]>, op
         } else if (highlighter) {
             safeBody = highlighter.applyHighlights(escapeHtml(plainBody), safeHighlights!).join("");
         }
+
+        return { bodyHasEmoji, isHtmlMessage, strippedBody, safeBody, isFormattedBody };
     } finally {
         delete sanitizeParams.textFilter;
     }
+}
+
+export function bodyToNode(content: IContent, highlights: Optional<string[]>, opts: EventRenderOpts = {}): ReactNode {
+    const eventInfo = analyseEvent(content, highlights, opts);
 
     let emojiBody = false;
-    if (!opts.disableBigEmoji && bodyHasEmoji) {
-        const contentBody = safeBody ?? strippedBody;
+    if (!opts.disableBigEmoji && eventInfo.bodyHasEmoji) {
+        const contentBody = eventInfo.safeBody ?? eventInfo.strippedBody;
         let contentBodyTrimmed = contentBody !== undefined ? contentBody.trim() : "";
 
         // Remove zero width joiner, zero width spaces and other spaces in body
@@ -405,48 +395,70 @@ export function bodyToHtml(content: IContent, highlights: Optional<string[]>, op
             // Prevent user pills expanding for users with only emoji in
             // their username. Permalinks (links in pills) can be any URL
             // now, so we just check for an HTTP-looking thing.
-            (strippedBody === safeBody || // replies have the html fallbacks, account for that here
+            (eventInfo.strippedBody === eventInfo.safeBody || // replies have the html fallbacks, account for that here
                 content.formatted_body === undefined ||
                 (!content.formatted_body.includes("http:") && !content.formatted_body.includes("https:")));
     }
 
-    if (isFormattedBody && bodyHasEmoji && safeBody) {
-        // This has to be done after the emojiBody check above as to not break big emoji on replies
-        safeBody = formatEmojis(safeBody, true).join("");
-    }
-
-    if (opts.returnString) {
-        return safeBody ?? strippedBody;
-    }
-
     const className = classNames({
         "mx_EventTile_body": true,
         "mx_EventTile_bigEmoji": emojiBody,
-        "markdown-body": isHtmlMessage && !emojiBody,
+        "markdown-body": eventInfo.isHtmlMessage && !emojiBody,
         // Override the global `notranslate` class set by the top-level `matrixchat` div.
         "translate": true,
     });
 
-    let emojiBodyElements: JSX.Element[] | undefined;
-    if (!safeBody && bodyHasEmoji) {
-        emojiBodyElements = formatEmojis(strippedBody, false) as JSX.Element[];
+    let formattedBody = eventInfo.safeBody;
+    if (eventInfo.isFormattedBody && eventInfo.bodyHasEmoji && eventInfo.safeBody) {
+        // This has to be done after the emojiBody check as to not break big emoji on replies
+        formattedBody = formatEmojis(eventInfo.safeBody, true).join("");
     }
 
-    return safeBody ? (
+    let emojiBodyElements: JSX.Element[] | undefined;
+    if (!eventInfo.safeBody && eventInfo.bodyHasEmoji) {
+        emojiBodyElements = formatEmojis(eventInfo.strippedBody, false) as JSX.Element[];
+    }
+
+    return formattedBody ? (
         <span
             key="body"
             ref={opts.ref}
             className={className}
-            dangerouslySetInnerHTML={{ __html: safeBody }}
+            dangerouslySetInnerHTML={{ __html: formattedBody }}
             dir="auto"
         />
     ) : (
         <span key="body" ref={opts.ref} className={className} dir="auto">
-            {emojiBodyElements || strippedBody}
+            {emojiBodyElements || eventInfo.strippedBody}
         </span>
     );
 }
 
+/**
+ * Turn a matrix event body into html
+ *
+ * content: 'content' of the MatrixEvent
+ *
+ * highlights: optional list of words to highlight, ordered by longest word first
+ *
+ * opts.highlightLink: optional href to add to highlighted words
+ * opts.disableBigEmoji: optional argument to disable the big emoji class.
+ * opts.stripReplyFallback: optional argument specifying the event is a reply and so fallback needs removing
+ * opts.forComposerQuote: optional param to lessen the url rewriting done by sanitization, for quoting into composer
+ * opts.ref: React ref to attach to any React components returned (not compatible with opts.returnString)
+ */
+export function bodyToHtml(content: IContent, highlights: Optional<string[]>, opts: EventRenderOpts = {}): string {
+    const eventInfo = analyseEvent(content, highlights, opts);
+
+    let formattedBody = eventInfo.safeBody;
+    if (eventInfo.isFormattedBody && eventInfo.bodyHasEmoji && formattedBody) {
+        // This has to be done after the emojiBody check above as to not break big emoji on replies
+        formattedBody = formatEmojis(eventInfo.safeBody, true).join("");
+    }
+
+    return formattedBody ?? eventInfo.strippedBody;
+}
+
 /**
  * Turn a room topic into html
  * @param topic plain text topic

src/components/views/messages/EditHistoryMessage.tsx

@@ -172,9 +172,8 @@ export default class EditHistoryMessage extends React.PureComponent<IProps, ISta
             if (this.props.previousEdit) {
                 contentElements = editBodyDiffToHtml(getReplacedContent(this.props.previousEdit), content);
             } else {
-                contentElements = HtmlUtils.bodyToHtml(content, null, {
+                contentElements = HtmlUtils.bodyToNode(content, null, {
                     stripReplyFallback: true,
-                    returnString: false,
                 });
             }
             if (mxEvent.getContent().msgtype === MsgType.Emote) {

src/components/views/messages/TextualBody.tsx

@@ -573,12 +573,11 @@ export default class TextualBody extends React.Component<IBodyProps, IState> {
         const stripReply = !mxEvent.replacingEvent() && !!getParentEventId(mxEvent);
         isEmote = content.msgtype === MsgType.Emote;
         isNotice = content.msgtype === MsgType.Notice;
-        let body = HtmlUtils.bodyToHtml(content, this.props.highlights, {
+        let body = HtmlUtils.bodyToNode(content, this.props.highlights, {
             disableBigEmoji: isEmote || !SettingsStore.getValue<boolean>("TextualBody.enableBigEmoji"),
             // Part of Replies fallback support
             stripReplyFallback: stripReply,
             ref: this.contentRef,
-            returnString: false,
         });
 
         if (this.props.replacingEventId) {

src/utils/MessageDiffUtils.tsx

@@ -22,7 +22,7 @@ import { IContent } from "matrix-js-sdk/src/matrix";
 import { logger } from "matrix-js-sdk/src/logger";
 import { unescape } from "lodash";
 
-import { bodyToHtml, checkBlockNode, IOptsReturnString } from "../HtmlUtils";
+import { bodyToHtml, checkBlockNode, EventRenderOpts } from "../HtmlUtils";
 
 function textToHtml(text: string): string {
     const container = document.createElement("div");
@@ -31,9 +31,8 @@ function textToHtml(text: string): string {
 }
 
 function getSanitizedHtmlBody(content: IContent): string {
-    const opts: IOptsReturnString = {
+    const opts: EventRenderOpts = {
         stripReplyFallback: true,
-        returnString: true,
     };
     if (content.format === "org.matrix.custom.html") {
         return bodyToHtml(content, null, opts);

test/HtmlUtils-test.tsx

@@ -19,7 +19,7 @@ import { mocked } from "jest-mock";
 import { render, screen } from "@testing-library/react";
 import { IContent } from "matrix-js-sdk/src/matrix";
 
-import { bodyToHtml, formatEmojis, topicToHtml } from "../src/HtmlUtils";
+import { bodyToNode, formatEmojis, topicToHtml } from "../src/HtmlUtils";
 import SettingsStore from "../src/settings/SettingsStore";
 
 jest.mock("../src/settings/SettingsStore");
@@ -66,7 +66,7 @@ describe("topicToHtml", () => {
 
 describe("bodyToHtml", () => {
     function getHtml(content: IContent, highlights?: string[]): string {
-        return (bodyToHtml(content, highlights, {}) as ReactElement).props.dangerouslySetInnerHTML.__html;
+        return (bodyToNode(content, highlights, {}) as ReactElement).props.dangerouslySetInnerHTML.__html;
     }
 
     it("should apply highlights to HTML messages", () => {
@@ -108,14 +108,14 @@ describe("bodyToHtml", () => {
     });
 
     it("generates big emoji for emoji made of multiple characters", () => {
-        const { asFragment } = render(bodyToHtml({ body: "👨‍👩‍👧‍👦 ↔️ 🇮🇸", msgtype: "m.text" }, [], {}) as ReactElement);
+        const { asFragment } = render(bodyToNode({ body: "👨‍👩‍👧‍👦 ↔️ 🇮🇸", msgtype: "m.text" }, [], {}) as ReactElement);
 
         expect(asFragment()).toMatchSnapshot();
     });
 
     it("should generate big emoji for an emoji-only reply to a message", () => {
         const { asFragment } = render(
-            bodyToHtml(
+            bodyToNode(
                 {
                     "body": "> <@sender1:server> Test\n\n🥰",
                     "format": "org.matrix.custom.html",
@@ -139,7 +139,7 @@ describe("bodyToHtml", () => {
     });
 
     it("does not mistake characters in text presentation mode for emoji", () => {
-        const { asFragment } = render(bodyToHtml({ body: "↔ ❗︎", msgtype: "m.text" }, [], {}) as ReactElement);
+        const { asFragment } = render(bodyToNode({ body: "↔ ❗︎", msgtype: "m.text" }, [], {}) as ReactElement);
 
         expect(asFragment()).toMatchSnapshot();
     });