fix: preserve linkification in formatted body text

.changeset/fix-formatted-body-linkify.md

@@ -0,0 +1,5 @@
+---
+default: patch
+---
+
+Fixes links not being clickable in formatted messages, including messages that use abbreviations.

src/app/components/message/RenderBody.tsx

@@ -1,12 +1,12 @@
 import { MouseEventHandler, useEffect, useState } from 'react';
 import parse, { HTMLReactParserOptions } from 'html-react-parser';
 import Linkify from 'linkify-react';
-import { Opts } from 'linkifyjs';
+import { find, Opts } from 'linkifyjs';
 import { PopOut, RectCords, Text, Tooltip, TooltipProvider, toRem } from 'folds';
 import { sanitizeCustomHtml } from '$utils/sanitize';
 import { highlightText, scaleSystemEmoji } from '$plugins/react-custom-html-parser';
 import { useRoomAbbreviationsContext } from '$hooks/useRoomAbbreviations';
-import { splitByAbbreviations } from '$utils/abbreviations';
+import { splitByAbbreviations, TextSegment } from '$utils/abbreviations';
 import { MessageEmptyContent } from './content';
 
 function getRenderedBodyText(text: string, highlightRegex?: RegExp): (string | JSX.Element)[] {
@@ -28,6 +28,47 @@ function renderLinkifiedBodyText(
   );
 }
 
+type RenderTextFn = (text: string, key?: string) => JSX.Element;
+
+function splitBodyTextByAbbreviations(
+  text: string,
+  abbrMap: Map<string, string>,
+  linkifyOpts?: Opts
+): TextSegment[] {
+  if (abbrMap.size === 0) return [{ id: 'txt-0', text }];
+
+  const linkMatches = find(text, linkifyOpts).filter((match) => match.isLink);
+  if (linkMatches.length === 0) return splitByAbbreviations(text, abbrMap);
+
+  const segments: Array<Omit<TextSegment, 'id'>> = [];
+  let lastIndex = 0;
+
+  linkMatches.forEach(({ start, end }) => {
+    if (start > lastIndex) {
+      splitByAbbreviations(text.slice(lastIndex, start), abbrMap).forEach(
+        ({ text: segmentText, termKey }) => {
+          segments.push({ text: segmentText, termKey });
+        }
+      );
+    }
+
+    segments.push({ text: text.slice(start, end) });
+    lastIndex = end;
+  });
+
+  if (lastIndex < text.length) {
+    splitByAbbreviations(text.slice(lastIndex), abbrMap).forEach(
+      ({ text: segmentText, termKey }) => {
+        segments.push({ text: segmentText, termKey });
+      }
+    );
+  }
+
+  return segments.length > 0
+    ? segments.map((segment, index) => ({ ...segment, id: `txt-${index}` }))
+    : [{ id: 'txt-0', text }];
+}
+
 type AbbreviationTermProps = {
   text: string;
   definition: string;
@@ -84,11 +125,12 @@ function AbbreviationTerm({ text, definition }: AbbreviationTermProps) {
  * extra closures in the common case).
  */
 export function buildAbbrReplaceTextNode(
-  abbrMap: Map<string, string>
-): ((text: string) => JSX.Element | undefined) | undefined {
+  abbrMap: Map<string, string>,
+  linkifyOpts?: Opts
+): ((text: string, renderText: RenderTextFn) => JSX.Element | undefined) | undefined {
   if (abbrMap.size === 0) return undefined;
-  return function replaceTextNode(text: string) {
-    const segments = splitByAbbreviations(text, abbrMap);
+  return function replaceTextNode(text: string, renderText: RenderTextFn) {
+    const segments = splitBodyTextByAbbreviations(text, abbrMap, linkifyOpts);
     if (!segments.some((s) => s.termKey !== undefined)) return undefined;
     return (
       <>
@@ -100,7 +142,7 @@ export function buildAbbrReplaceTextNode(
               definition={abbrMap.get(seg.termKey) ?? ''}
             />
           ) : (
-            seg.text
+            renderText(seg.text, seg.id)
           )
         )}
       </>
@@ -130,7 +172,7 @@ export function RenderBody({
   if (body === '') return <MessageEmptyContent />;
 
   if (abbrMap.size > 0) {
-    const segments = splitByAbbreviations(body, abbrMap);
+    const segments = splitBodyTextByAbbreviations(body, abbrMap, linkifyOpts);
     if (segments.some((s) => s.termKey !== undefined)) {
       return (
         <>

src/app/features/room/RoomTimeline.tsx

@@ -520,7 +520,7 @@ export function RoomTimeline({
         handleMentionClick: mentionClickHandler,
         nicknames,
         autoplayEmojis,
-        replaceTextNode: buildAbbrReplaceTextNode(abbrMap),
+        replaceTextNode: buildAbbrReplaceTextNode(abbrMap, linkifyOpts),
       }),
     [
       mx,

src/app/features/room/ThreadDrawer.tsx

@@ -412,7 +412,7 @@ export function ThreadDrawer({ room, threadRootId, onClose, overlay }: ThreadDra
         handleSpoilerClick: spoilerClickHandler,
         handleMentionClick: mentionClickHandler,
         nicknames,
-        replaceTextNode: buildAbbrReplaceTextNode(abbrMap),
+        replaceTextNode: buildAbbrReplaceTextNode(abbrMap, linkifyOpts),
       }),
     [
       mx,

src/app/plugins/react-custom-html-parser.test.tsx

@@ -2,6 +2,7 @@ import { render, screen } from '@testing-library/react';
 import parse from 'html-react-parser';
 import { afterEach, describe, expect, it, vi } from 'vitest';
 import * as customHtmlCss from '$styles/CustomHtml.css';
+import { buildAbbrReplaceTextNode } from '$components/message/RenderBody';
 import { sanitizeCustomHtml } from '$utils/sanitize';
 import {
   LINKIFY_OPTS,
@@ -257,4 +258,53 @@ describe('react custom html parser', () => {
 
     expect(logSpy).not.toHaveBeenCalled();
   });
+
+  it('linkifies bare urls in formatted html text nodes even when abbreviation replacement runs', () => {
+    const parserOptions = getReactCustomHtmlParser(createMatrixClient(), '!room:example.com', {
+      settingsLinkBaseUrl,
+      linkifyOpts: LINKIFY_OPTS,
+      handleMentionClick: undefined,
+      replaceTextNode: buildAbbrReplaceTextNode(new Map([['PR', 'Pull request']]), LINKIFY_OPTS),
+    });
+
+    render(
+      <div>
+        {parse(
+          '<p>figured out the section could be removed; set up a PR for it: https://github.com/SableClient/Sable/pull/626</p>',
+          parserOptions
+        )}
+      </div>
+    );
+
+    expect(screen.getByText('PR')).toBeInTheDocument();
+    expect(
+      screen.getByRole('link', {
+        name: 'https://github.com/SableClient/Sable/pull/626',
+      })
+    ).toHaveAttribute('href', 'https://github.com/SableClient/Sable/pull/626');
+  });
+
+  it('keeps the full link intact when an abbreviation term appears inside the url token', () => {
+    const parserOptions = getReactCustomHtmlParser(createMatrixClient(), '!room:example.com', {
+      settingsLinkBaseUrl,
+      linkifyOpts: LINKIFY_OPTS,
+      handleMentionClick: undefined,
+      replaceTextNode: buildAbbrReplaceTextNode(new Map([['PR', 'Pull request']]), LINKIFY_OPTS),
+    });
+
+    render(
+      <div>
+        {parse(
+          '<p>see https://github.com/SableClient/Sable/pull/PR/626 for context</p>',
+          parserOptions
+        )}
+      </div>
+    );
+
+    expect(
+      screen.getByRole('link', {
+        name: 'https://github.com/SableClient/Sable/pull/PR/626',
+      })
+    ).toHaveAttribute('href', 'https://github.com/SableClient/Sable/pull/PR/626');
+  });
 });

src/app/plugins/react-custom-html-parser.tsx

@@ -2,6 +2,7 @@
 import {
   CSSProperties,
   ComponentPropsWithoutRef,
+  Fragment,
   ReactEventHandler,
   ReactNode,
   useMemo,
@@ -500,14 +501,52 @@ export const getReactCustomHtmlParser = (
     useAuthentication?: boolean;
     nicknames?: Nicknames;
     autoplayEmojis?: boolean;
-    replaceTextNode?: (text: string) => JSX.Element | undefined;
+    replaceTextNode?: (
+      text: string,
+      renderText: (text: string, key?: string) => JSX.Element
+    ) => JSX.Element | undefined;
   }
 ): HTMLReactParserOptions => {
   const { replaceTextNode } = params;
+
+  const shouldLinkifyDomText = (domNode: DOMText): boolean =>
+    !(domNode.parent && 'name' in domNode.parent && domNode.parent.name === 'code') &&
+    !(domNode.parent && 'name' in domNode.parent && domNode.parent.name === 'a');
+
+  const decorateText = (text: string) => {
+    let jsx = scaleSystemEmoji(text);
+
+    if (params.highlightRegex) {
+      jsx = highlightText(params.highlightRegex, jsx);
+    }
+
+    return jsx;
+  };
+
+  const renderReplacementText = (text: string, linkify: boolean, key?: string): JSX.Element => {
+    const decoratedText = decorateText(text);
+
+    if (linkify) {
+      return (
+        <Linkify key={key} options={params.linkifyOpts}>
+          {decoratedText}
+        </Linkify>
+      );
+    }
+
+    return <Fragment key={key}>{decoratedText}</Fragment>;
+  };
+
   const opts: HTMLReactParserOptions = {
     replace: (domNode) => {
       if (replaceTextNode && domNode instanceof DOMText) {
-        return replaceTextNode(domNode.data) ?? undefined;
+        const replacement = replaceTextNode(domNode.data, (text, key) =>
+          renderReplacementText(text, shouldLinkifyDomText(domNode), key)
+        );
+
+        if (replacement !== undefined) {
+          return replacement;
+        }
       }
       if (domNode instanceof Element && 'name' in domNode) {
         const { name, attribs, children, parent } = domNode;
@@ -839,20 +878,14 @@ export const getReactCustomHtmlParser = (
       }
 
       if (domNode instanceof DOMText) {
-        const linkify =
-          !(domNode.parent && 'name' in domNode.parent && domNode.parent.name === 'code') &&
-          !(domNode.parent && 'name' in domNode.parent && domNode.parent.name === 'a');
-
-        let jsx = scaleSystemEmoji(domNode.data);
-
-        if (params.highlightRegex) {
-          jsx = highlightText(params.highlightRegex, jsx);
-        }
+        const linkify = shouldLinkifyDomText(domNode);
+        const decoratedText = decorateText(domNode.data);
 
         if (linkify) {
-          return <Linkify options={params.linkifyOpts}>{jsx}</Linkify>;
+          return <Linkify options={params.linkifyOpts}>{decoratedText}</Linkify>;
         }
-        return jsx;
+
+        return decoratedText;
       }
       return undefined;
     },