From 272a66baa5fccfb4140cf83753dbc120824d1cfa Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 30 Jul 2024 14:35:16 +0100 Subject: [PATCH] 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 | 130 ++++++++++-------- .../views/messages/EditHistoryMessage.tsx | 3 +- src/components/views/messages/TextualBody.tsx | 3 +- src/utils/MessageDiffUtils.tsx | 5 +- test/HtmlUtils-test.tsx | 10 +- 5 files changed, 80 insertions(+), 71 deletions(-) diff --git a/src/HtmlUtils.tsx b/src/HtmlUtils.tsx index 33f242b4fc..15b45171fe 100644 --- a/src/HtmlUtils.tsx +++ b/src/HtmlUtils.tsx @@ -246,23 +246,6 @@ class HtmlHighlighter extends BaseHighlighter { } } -interface IOpts { - highlightLink?: string; - disableBigEmoji?: boolean; - stripReplyFallback?: boolean; - returnString?: boolean; - forComposerQuote?: boolean; - ref?: React.Ref; -} - -export interface IOptsReturnNode extends IOpts { - returnString?: false | undefined; -} - -export interface IOptsReturnString extends IOpts { - returnString: true; -} - const emojiToHtmlSpan = (emoji: string): string => `${emoji}`; 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, opts: IOptsReturnString): string; -export function bodyToHtml(content: IContent, highlights: Optional, opts: IOptsReturnNode): ReactNode; -export function bodyToHtml(content: IContent, highlights: Optional, 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; +} + +function analyseEvent(content: IContent, highlights: Optional, 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 `, 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, 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, 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, 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 ? ( ) : ( - {emojiBodyElements || strippedBody} + {emojiBodyElements || eventInfo.strippedBody} ); } +/** + * 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, 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 diff --git a/src/components/views/messages/EditHistoryMessage.tsx b/src/components/views/messages/EditHistoryMessage.tsx index 49e0f1f7de..d688650353 100644 --- a/src/components/views/messages/EditHistoryMessage.tsx +++ b/src/components/views/messages/EditHistoryMessage.tsx @@ -172,9 +172,8 @@ export default class EditHistoryMessage extends React.PureComponent { 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("TextualBody.enableBigEmoji"), // Part of Replies fallback support stripReplyFallback: stripReply, ref: this.contentRef, - returnString: false, }); if (this.props.replacingEventId) { diff --git a/src/utils/MessageDiffUtils.tsx b/src/utils/MessageDiffUtils.tsx index ff0c42e1a1..6cc6a59e77 100644 --- a/src/utils/MessageDiffUtils.tsx +++ b/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); diff --git a/test/HtmlUtils-test.tsx b/test/HtmlUtils-test.tsx index ae12a71780..b3bad1813c 100644 --- a/test/HtmlUtils-test.tsx +++ b/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(); });