Skip to content

Commit 72fcd1a

Browse files
authored
fix: preserve linkification in formatted body text (#632)
### Description Fixes the regression caused by 18a7ca092 that makes urls not get dedicated in `formatted_body` - preserve auto-linking for bare URLs in `formatted_body` text nodes when abbreviation replacement runs - keep URL tokens intact when an abbreviation term appears inside the URL itself - add regression coverage for adjacent-to-URL and inside-URL abbreviation cases #### Type of change - [x] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) - [ ] This change requires a documentation update ### Checklist: - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings ### AI disclosure: - [ ] Partially AI assisted (clarify which code was AI assisted and briefly explain what it does). - [x] Fully AI generated (explain what all the generated code does in moderate detail).
2 parents 4ff8b3c + 00c4544 commit 72fcd1a

6 files changed

Lines changed: 153 additions & 23 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
default: patch
3+
---
4+
5+
Fixes links not being clickable in formatted messages, including messages that use abbreviations.

src/app/components/message/RenderBody.tsx

Lines changed: 50 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
import { MouseEventHandler, useEffect, useState } from 'react';
22
import parse, { HTMLReactParserOptions } from 'html-react-parser';
33
import Linkify from 'linkify-react';
4-
import { Opts } from 'linkifyjs';
4+
import { find, Opts } from 'linkifyjs';
55
import { PopOut, RectCords, Text, Tooltip, TooltipProvider, toRem } from 'folds';
66
import { sanitizeCustomHtml } from '$utils/sanitize';
77
import { highlightText, scaleSystemEmoji } from '$plugins/react-custom-html-parser';
88
import { useRoomAbbreviationsContext } from '$hooks/useRoomAbbreviations';
9-
import { splitByAbbreviations } from '$utils/abbreviations';
9+
import { splitByAbbreviations, TextSegment } from '$utils/abbreviations';
1010
import { MessageEmptyContent } from './content';
1111

1212
function getRenderedBodyText(text: string, highlightRegex?: RegExp): (string | JSX.Element)[] {
@@ -28,6 +28,47 @@ function renderLinkifiedBodyText(
2828
);
2929
}
3030

31+
type RenderTextFn = (text: string, key?: string) => JSX.Element;
32+
33+
function splitBodyTextByAbbreviations(
34+
text: string,
35+
abbrMap: Map<string, string>,
36+
linkifyOpts?: Opts
37+
): TextSegment[] {
38+
if (abbrMap.size === 0) return [{ id: 'txt-0', text }];
39+
40+
const linkMatches = find(text, linkifyOpts).filter((match) => match.isLink);
41+
if (linkMatches.length === 0) return splitByAbbreviations(text, abbrMap);
42+
43+
const segments: Array<Omit<TextSegment, 'id'>> = [];
44+
let lastIndex = 0;
45+
46+
linkMatches.forEach(({ start, end }) => {
47+
if (start > lastIndex) {
48+
splitByAbbreviations(text.slice(lastIndex, start), abbrMap).forEach(
49+
({ text: segmentText, termKey }) => {
50+
segments.push({ text: segmentText, termKey });
51+
}
52+
);
53+
}
54+
55+
segments.push({ text: text.slice(start, end) });
56+
lastIndex = end;
57+
});
58+
59+
if (lastIndex < text.length) {
60+
splitByAbbreviations(text.slice(lastIndex), abbrMap).forEach(
61+
({ text: segmentText, termKey }) => {
62+
segments.push({ text: segmentText, termKey });
63+
}
64+
);
65+
}
66+
67+
return segments.length > 0
68+
? segments.map((segment, index) => ({ ...segment, id: `txt-${index}` }))
69+
: [{ id: 'txt-0', text }];
70+
}
71+
3172
type AbbreviationTermProps = {
3273
text: string;
3374
definition: string;
@@ -84,11 +125,12 @@ function AbbreviationTerm({ text, definition }: AbbreviationTermProps) {
84125
* extra closures in the common case).
85126
*/
86127
export function buildAbbrReplaceTextNode(
87-
abbrMap: Map<string, string>
88-
): ((text: string) => JSX.Element | undefined) | undefined {
128+
abbrMap: Map<string, string>,
129+
linkifyOpts?: Opts
130+
): ((text: string, renderText: RenderTextFn) => JSX.Element | undefined) | undefined {
89131
if (abbrMap.size === 0) return undefined;
90-
return function replaceTextNode(text: string) {
91-
const segments = splitByAbbreviations(text, abbrMap);
132+
return function replaceTextNode(text: string, renderText: RenderTextFn) {
133+
const segments = splitBodyTextByAbbreviations(text, abbrMap, linkifyOpts);
92134
if (!segments.some((s) => s.termKey !== undefined)) return undefined;
93135
return (
94136
<>
@@ -100,7 +142,7 @@ export function buildAbbrReplaceTextNode(
100142
definition={abbrMap.get(seg.termKey) ?? ''}
101143
/>
102144
) : (
103-
seg.text
145+
renderText(seg.text, seg.id)
104146
)
105147
)}
106148
</>
@@ -130,7 +172,7 @@ export function RenderBody({
130172
if (body === '') return <MessageEmptyContent />;
131173

132174
if (abbrMap.size > 0) {
133-
const segments = splitByAbbreviations(body, abbrMap);
175+
const segments = splitBodyTextByAbbreviations(body, abbrMap, linkifyOpts);
134176
if (segments.some((s) => s.termKey !== undefined)) {
135177
return (
136178
<>

src/app/features/room/RoomTimeline.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -520,7 +520,7 @@ export function RoomTimeline({
520520
handleMentionClick: mentionClickHandler,
521521
nicknames,
522522
autoplayEmojis,
523-
replaceTextNode: buildAbbrReplaceTextNode(abbrMap),
523+
replaceTextNode: buildAbbrReplaceTextNode(abbrMap, linkifyOpts),
524524
}),
525525
[
526526
mx,

src/app/features/room/ThreadDrawer.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -412,7 +412,7 @@ export function ThreadDrawer({ room, threadRootId, onClose, overlay }: ThreadDra
412412
handleSpoilerClick: spoilerClickHandler,
413413
handleMentionClick: mentionClickHandler,
414414
nicknames,
415-
replaceTextNode: buildAbbrReplaceTextNode(abbrMap),
415+
replaceTextNode: buildAbbrReplaceTextNode(abbrMap, linkifyOpts),
416416
}),
417417
[
418418
mx,

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

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { render, screen } from '@testing-library/react';
22
import parse from 'html-react-parser';
33
import { afterEach, describe, expect, it, vi } from 'vitest';
44
import * as customHtmlCss from '$styles/CustomHtml.css';
5+
import { buildAbbrReplaceTextNode } from '$components/message/RenderBody';
56
import { sanitizeCustomHtml } from '$utils/sanitize';
67
import {
78
LINKIFY_OPTS,
@@ -257,4 +258,53 @@ describe('react custom html parser', () => {
257258

258259
expect(logSpy).not.toHaveBeenCalled();
259260
});
261+
262+
it('linkifies bare urls in formatted html text nodes even when abbreviation replacement runs', () => {
263+
const parserOptions = getReactCustomHtmlParser(createMatrixClient(), '!room:example.com', {
264+
settingsLinkBaseUrl,
265+
linkifyOpts: LINKIFY_OPTS,
266+
handleMentionClick: undefined,
267+
replaceTextNode: buildAbbrReplaceTextNode(new Map([['PR', 'Pull request']]), LINKIFY_OPTS),
268+
});
269+
270+
render(
271+
<div>
272+
{parse(
273+
'<p>figured out the section could be removed; set up a PR for it: https://github.com/SableClient/Sable/pull/626</p>',
274+
parserOptions
275+
)}
276+
</div>
277+
);
278+
279+
expect(screen.getByText('PR')).toBeInTheDocument();
280+
expect(
281+
screen.getByRole('link', {
282+
name: 'https://github.com/SableClient/Sable/pull/626',
283+
})
284+
).toHaveAttribute('href', 'https://github.com/SableClient/Sable/pull/626');
285+
});
286+
287+
it('keeps the full link intact when an abbreviation term appears inside the url token', () => {
288+
const parserOptions = getReactCustomHtmlParser(createMatrixClient(), '!room:example.com', {
289+
settingsLinkBaseUrl,
290+
linkifyOpts: LINKIFY_OPTS,
291+
handleMentionClick: undefined,
292+
replaceTextNode: buildAbbrReplaceTextNode(new Map([['PR', 'Pull request']]), LINKIFY_OPTS),
293+
});
294+
295+
render(
296+
<div>
297+
{parse(
298+
'<p>see https://github.com/SableClient/Sable/pull/PR/626 for context</p>',
299+
parserOptions
300+
)}
301+
</div>
302+
);
303+
304+
expect(
305+
screen.getByRole('link', {
306+
name: 'https://github.com/SableClient/Sable/pull/PR/626',
307+
})
308+
).toHaveAttribute('href', 'https://github.com/SableClient/Sable/pull/PR/626');
309+
});
260310
});

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

Lines changed: 46 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
import {
33
CSSProperties,
44
ComponentPropsWithoutRef,
5+
Fragment,
56
ReactEventHandler,
67
ReactNode,
78
useMemo,
@@ -500,14 +501,52 @@ export const getReactCustomHtmlParser = (
500501
useAuthentication?: boolean;
501502
nicknames?: Nicknames;
502503
autoplayEmojis?: boolean;
503-
replaceTextNode?: (text: string) => JSX.Element | undefined;
504+
replaceTextNode?: (
505+
text: string,
506+
renderText: (text: string, key?: string) => JSX.Element
507+
) => JSX.Element | undefined;
504508
}
505509
): HTMLReactParserOptions => {
506510
const { replaceTextNode } = params;
511+
512+
const shouldLinkifyDomText = (domNode: DOMText): boolean =>
513+
!(domNode.parent && 'name' in domNode.parent && domNode.parent.name === 'code') &&
514+
!(domNode.parent && 'name' in domNode.parent && domNode.parent.name === 'a');
515+
516+
const decorateText = (text: string) => {
517+
let jsx = scaleSystemEmoji(text);
518+
519+
if (params.highlightRegex) {
520+
jsx = highlightText(params.highlightRegex, jsx);
521+
}
522+
523+
return jsx;
524+
};
525+
526+
const renderReplacementText = (text: string, linkify: boolean, key?: string): JSX.Element => {
527+
const decoratedText = decorateText(text);
528+
529+
if (linkify) {
530+
return (
531+
<Linkify key={key} options={params.linkifyOpts}>
532+
{decoratedText}
533+
</Linkify>
534+
);
535+
}
536+
537+
return <Fragment key={key}>{decoratedText}</Fragment>;
538+
};
539+
507540
const opts: HTMLReactParserOptions = {
508541
replace: (domNode) => {
509542
if (replaceTextNode && domNode instanceof DOMText) {
510-
return replaceTextNode(domNode.data) ?? undefined;
543+
const replacement = replaceTextNode(domNode.data, (text, key) =>
544+
renderReplacementText(text, shouldLinkifyDomText(domNode), key)
545+
);
546+
547+
if (replacement !== undefined) {
548+
return replacement;
549+
}
511550
}
512551
if (domNode instanceof Element && 'name' in domNode) {
513552
const { name, attribs, children, parent } = domNode;
@@ -839,20 +878,14 @@ export const getReactCustomHtmlParser = (
839878
}
840879

841880
if (domNode instanceof DOMText) {
842-
const linkify =
843-
!(domNode.parent && 'name' in domNode.parent && domNode.parent.name === 'code') &&
844-
!(domNode.parent && 'name' in domNode.parent && domNode.parent.name === 'a');
845-
846-
let jsx = scaleSystemEmoji(domNode.data);
847-
848-
if (params.highlightRegex) {
849-
jsx = highlightText(params.highlightRegex, jsx);
850-
}
881+
const linkify = shouldLinkifyDomText(domNode);
882+
const decoratedText = decorateText(domNode.data);
851883

852884
if (linkify) {
853-
return <Linkify options={params.linkifyOpts}>{jsx}</Linkify>;
885+
return <Linkify options={params.linkifyOpts}>{decoratedText}</Linkify>;
854886
}
855-
return jsx;
887+
888+
return decoratedText;
856889
}
857890
return undefined;
858891
},

0 commit comments

Comments
 (0)