From e864dbfacf01a5714722017bf61248b861faac72 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 5 Aug 2022 08:08:56 +0100 Subject: [PATCH] Fix highlights not being applied to plaintext messages (#9126) * Fix highlights not being applied to plaintext messages * Add percy test for search result highlighting * Fix percy target * Update timeline.spec.ts --- cypress/e2e/timeline/timeline.spec.ts | 16 +++++++- src/HtmlUtils.tsx | 54 +++++++++++++-------------- 2 files changed, 41 insertions(+), 29 deletions(-) diff --git a/cypress/e2e/timeline/timeline.spec.ts b/cypress/e2e/timeline/timeline.spec.ts index 2778ab56dd..6eacacfed2 100644 --- a/cypress/e2e/timeline/timeline.spec.ts +++ b/cypress/e2e/timeline/timeline.spec.ts @@ -57,12 +57,12 @@ const expectAvatar = (e: JQuery, avatarUrl: string): void => { }); }; -const sendEvent = (roomId: string): Chainable => { +const sendEvent = (roomId: string, html = false): Chainable => { return cy.sendEvent( roomId, null, "m.room.message" as EventType, - MessageEvent.from("Message").serialize().content, + MessageEvent.from("Message", html ? "Message" : undefined).serialize().content, ); }; @@ -261,6 +261,18 @@ describe("Timeline", () => { // Make sure "collapse" link button worked cy.get(".mx_GenericEventListSummary_toggle[aria-expanded=false]"); }); + + it("should highlight search result words regardless of formatting", () => { + sendEvent(roomId); + sendEvent(roomId, true); + cy.visit("/#/room/" + roomId); + + cy.get(".mx_RoomHeader_searchButton").click(); + cy.get(".mx_SearchBar_input input").type("Message{enter}"); + + cy.get(".mx_EventTile:not(.mx_EventTile_contextual)").find(".mx_EventTile_searchHighlight").should("exist"); + cy.get(".mx_RoomView_searchResultsPanel").percySnapshotElement("Highlighted search results"); + }); }); describe("message sending", () => { diff --git a/src/HtmlUtils.tsx b/src/HtmlUtils.tsx index dac79a37a1..4a4ac9e56c 100644 --- a/src/HtmlUtils.tsx +++ b/src/HtmlUtils.tsx @@ -470,40 +470,38 @@ export function bodyToHtml(content: IContent, highlights: Optional, op } let strippedBody: string; - let safeBody: string; - let isDisplayedWithHtml: boolean; - // XXX: We sanitize the HTML whilst also highlighting its text nodes, to avoid accidentally trying - // to highlight HTML tags themselves. However, this does mean that we don't highlight textnodes which - // are interrupted by HTML tags (not that we did before) - e.g. foobar won't get highlighted - // by an attempt to search for 'foobar'. Then again, the search query probably wouldn't work either + let safeBody: string; // safe, sanitised HTML, preferred over `strippedBody` which is fully plaintext + try { - if (highlights && highlights.length > 0) { - const highlighter = new HtmlHighlighter("mx_EventTile_searchHighlight", opts.highlightLink); - const safeHighlights = highlights - // sanitizeHtml can hang if an unclosed HTML tag is thrown at it - // A search for ` !highlight.includes("<")) - .map((highlight: string): string => sanitizeHtml(highlight, sanitizeParams)); - // XXX: hacky bodge to temporarily apply a textFilter to the sanitizeParams structure. - sanitizeParams.textFilter = function(safeText) { - return highlighter.applyHighlights(safeText, safeHighlights).join(''); - }; - } + // sanitizeHtml can hang if an unclosed HTML tag is thrown at it + // A search for ` !highlight.includes("<")) + .map((highlight: string): string => sanitizeHtml(highlight, sanitizeParams)); let formattedBody = typeof content.formatted_body === 'string' ? content.formatted_body : null; const plainBody = typeof content.body === 'string' ? content.body : ""; if (opts.stripReplyFallback && formattedBody) formattedBody = stripHTMLReply(formattedBody); strippedBody = opts.stripReplyFallback ? stripPlainReply(plainBody) : plainBody; - bodyHasEmoji = mightContainEmoji(isFormattedBody ? formattedBody : plainBody); - // Only generate safeBody if the message was sent as org.matrix.custom.html + const highlighter = safeHighlights?.length + ? new HtmlHighlighter("mx_EventTile_searchHighlight", opts.highlightLink) + : null; + if (isFormattedBody) { - isDisplayedWithHtml = true; + if (highlighter) { + // XXX: We sanitize the HTML whilst also highlighting its text nodes, to avoid accidentally trying + // to highlight HTML tags themselves. However, this does mean that we don't highlight textnodes which + // are interrupted by HTML tags (not that we did before) - e.g. foobar won't get highlighted + // by an attempt to search for 'foobar'. Then again, the search query probably wouldn't work either + // XXX: hacky bodge to temporarily apply a textFilter to the sanitizeParams structure. + sanitizeParams.textFilter = function(safeText) { + return highlighter.applyHighlights(safeText, safeHighlights).join(''); + }; + } safeBody = sanitizeHtml(formattedBody, sanitizeParams); const phtml = cheerio.load(safeBody, { @@ -533,12 +531,14 @@ export function bodyToHtml(content: IContent, highlights: Optional, op if (bodyHasEmoji) { safeBody = formatEmojis(safeBody, true).join(''); } + } else if (highlighter) { + safeBody = highlighter.applyHighlights(plainBody, safeHighlights).join(''); } } finally { delete sanitizeParams.textFilter; } - const contentBody = isDisplayedWithHtml ? safeBody : strippedBody; + const contentBody = safeBody ?? strippedBody; if (opts.returnString) { return contentBody; } @@ -576,11 +576,11 @@ export function bodyToHtml(content: IContent, highlights: Optional, op }); let emojiBodyElements: JSX.Element[]; - if (!isDisplayedWithHtml && bodyHasEmoji) { + if (!safeBody && bodyHasEmoji) { emojiBodyElements = formatEmojis(strippedBody, false) as JSX.Element[]; } - return isDisplayedWithHtml ? + return safeBody ?