Skip to content

Commit 0edf45c

Browse files
7w17w1
authored
Fix link editing behaviors (#847)
<!-- Please read https://github.com/SableClient/Sable/blob/dev/CONTRIBUTING.md before submitting your pull request --> ### Description <!-- Please include a summary of the change. Please also include relevant motivation and context. List any dependencies that are required for this change. --> Fixes the random \<> and links being too aggressive with parsing leading to the entire markdown structure being seen as part of it and fixes editing messages with suppressed links not remaining suppressed. #### 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: - [ ] My code follows the style guidelines of this project - [ ] 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 - [ ] My changes generate no new warnings ### AI disclosure: - [x] Partially AI assisted (clarify which code was AI assisted and briefly explain what it does). - [ ] Fully AI generated (explain what all the generated code does in moderate detail). <!-- Write any explanation required here, but do not generate the explanation using AI!! You must prove you understand what the code in this PR does. --> Tests were AI generated.
2 parents 42e9d61 + e19e0df commit 0edf45c

8 files changed

Lines changed: 99 additions & 9 deletions

File tree

‎.changeset/fix-link-editing-issues.md‎

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+
Fix issues related to editing messages with links losing previews or gaining \<>

‎src/app/components/editor/getLinks.test.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,15 @@ describe('getLinks', () => {
3232
expect(links).toContain('https://example.com');
3333
});
3434

35+
it('does not merge link text URL with destination when both are https (edited bare link)', () => {
36+
const node: ParagraphElement = {
37+
type: BlockType.Paragraph,
38+
children: [{ text: '[https://example.com/](https://example.com/)' }],
39+
};
40+
const links = getLinks([node]);
41+
expect(links).toEqual(['https://example.com/']);
42+
});
43+
3544
it('excludes URLs inside markdown inline code spans', () => {
3645
const node: ParagraphElement = {
3746
type: BlockType.Paragraph,

‎src/app/components/editor/output.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ const elementToPlainText = (node: CustomElement, children: string): string => {
156156
};
157157

158158
const SPOILERINPUTREGEX = /\|\|.+?\|\|/g;
159-
const LINK_URL = `(https?:\\/\\/.[A-Za-z0-9-._~:/?#[\\]()@!$&'*+,;%=]+)`;
159+
const LINK_URL = `(https?:\\/\\/.[A-Za-z0-9-._~:/?#[\\()@!$&'*+,;%=]+)`;
160160
export const LINKINPUTREGEX = new RegExp(`\\(?(${LINK_URL})\\)?`, 'g');
161161
const SPOILEREDLINKINPUTREGEX = new RegExp(`<(${LINK_URL})>`, 'g');
162162
const SPOILEREDLINKDIRECTREGEX = new RegExp(`\\|\\|(${LINK_URL})\\|\\|`, 'g');

‎src/app/features/room/message/MessageEditor.tsx‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -121,9 +121,9 @@ export const MessageEditor = as<'div', MessageEditorProps>(
121121
);
122122
}
123123

124-
const bundleContent = content['com.beeper.linkpreviews'] as BundleContent[];
124+
const bundleContent =
125+
(content['com.beeper.linkpreviews'] as BundleContent[] | undefined) ?? [];
125126
const markHiddenLinks = (original: string, isHTML?: boolean) => {
126-
if (!bundleContent) return original;
127127
if (!isHTML) {
128128
return readdAngleBracketsForHiddenPreviews(original, bundleContent);
129129
}
@@ -155,7 +155,14 @@ export const MessageEditor = as<'div', MessageEditorProps>(
155155
(bundleContent?.length === 0 ||
156156
bundleContent.filter((b) => s.includes(b.matched_url)).length === 0) &&
157157
strippedS.match(LINKINPUTREGEX) !== null;
158-
newBody += `${isHidden ? (isHTML && ((s.startsWith('<a') && `&lt;${s[0]}`) || `${s[0]}&lt;`)) || `${s[0]}<` : s[0]}${strippedS}${isHidden ? (isHTML && '&gt;') || '>' : ''}`;
158+
159+
// Wrap whole <a>…</a> as &lt;…&gt; once; duplicating the leading "<" breaks htmlToMarkdown's [<][a][>] detection.
160+
if (isHidden && isHTML && s.toLowerCase().startsWith('<a')) {
161+
newBody += `&lt;${s}&gt;`;
162+
return;
163+
}
164+
165+
newBody += `${isHidden ? (isHTML && `${s[0]}&lt;`) || `${s[0]}<` : s[0]}${strippedS}${isHidden ? (isHTML && '&gt;') || '>' : ''}`;
159166
});
160167
return newBody;
161168
};

‎src/app/features/room/message/hiddenLinkPreviews.test.ts‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,22 @@ describe('stripMarkdownEscapesForHiddenPreviews', () => {
2525
String.raw`keep \*this\* and \<not-a-url\>`
2626
);
2727
});
28+
29+
it('unwraps outer \\< \\> around a preview-suppressed markdown link from htmlToMarkdown', () => {
30+
expect(
31+
stripMarkdownEscapesForHiddenPreviews(
32+
String.raw`\<[https://example.org/](<https://example.org/>)\>`
33+
)
34+
).toBe('[https://example.org/](<https://example.org/>)');
35+
});
36+
37+
it('fixes escaped outer brackets when destination lost angle brackets (bad HTML wrap)', () => {
38+
expect(
39+
stripMarkdownEscapesForHiddenPreviews(
40+
String.raw`\<[https://example.com/](https://example.com/)>`
41+
)
42+
).toBe('[https://example.com/](<https://example.com/>)');
43+
});
2844
});
2945

3046
describe('readdAngleBracketsForHiddenPreviews', () => {
@@ -47,4 +63,13 @@ describe('readdAngleBracketsForHiddenPreviews', () => {
4763
'see <https://example.org/>'
4864
);
4965
});
66+
67+
it('does not corrupt markdown suppressed links [url](<url>)', () => {
68+
expect(
69+
readdAngleBracketsForHiddenPreviews(
70+
'see [https://example.org/](<https://example.org/>) thanks',
71+
[]
72+
)
73+
).toBe('see [https://example.org/](<https://example.org/>) thanks');
74+
});
5075
});

‎src/app/features/room/message/hiddenLinkPreviews.ts‎

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import type { BundleContent } from '$components/message';
22

3-
const LINK_URL = `(https?:\\/\\/.[A-Za-z0-9-._~:/?#[\\]()@!$&'*+,;%=]+)`;
3+
const LINK_URL = `(https?:\\/\\/.[A-Za-z0-9-._~:/?#[\\()@!$&'*+,;%=]+)`;
44
const LINKINPUTREGEX = new RegExp(`\\(?(${LINK_URL})\\)?`, 'g');
55

66
/**
@@ -18,7 +18,20 @@ export function stripMarkdownEscapesForHiddenPreviews(markdown: string): string
1818
const OPEN_ONLY = new RegExp(String.raw`\\<(${LINK_URL})`, 'g');
1919
const CLOSE_ONLY = new RegExp(String.raw`(${LINK_URL})\\>`, 'g');
2020

21-
return markdown.replace(WRAPPED, '<$1>').replace(OPEN_ONLY, '<$1').replace(CLOSE_ONLY, '$1>');
21+
let s = markdown.replace(WRAPPED, '<$1>').replace(OPEN_ONLY, '<$1').replace(CLOSE_ONLY, '$1>');
22+
23+
// Restore [label](<url>) after htmlToMarkdown escaped a surrounding "\<...\>".
24+
const ESCAPED_SUPPRESSED_MD_LINK = new RegExp(
25+
String.raw`\\<\[([^\]]*)\]\((<https?:\/\/[^>\s]+>)\)\\>`,
26+
'g'
27+
);
28+
s = s.replace(ESCAPED_SUPPRESSED_MD_LINK, '[$1]($2)');
29+
30+
// Same for "\<[label](bare-url)>" when angle brackets were lost on the destination.
31+
const WRONG_OUTER_ESCAPED_AUTOLINK = /\\<\[([^\]]+)\]\((https?:\/\/[^)\s]+)\)(?:>|\\>)/g;
32+
s = s.replace(WRONG_OUTER_ESCAPED_AUTOLINK, '[$1](<$2>)');
33+
34+
return s;
2235
}
2336

2437
export function readdAngleBracketsForHiddenPreviews(
@@ -30,9 +43,23 @@ export function readdAngleBracketsForHiddenPreviews(
3043
const previewed = new Set(linkPreviews.map((b) => b.matched_url));
3144

3245
LINKINPUTREGEX.lastIndex = 0;
33-
return body.replace(LINKINPUTREGEX, (full, url: string, offset: number) => {
46+
return body.replace(LINKINPUTREGEX, (...args: unknown[]) => {
47+
const full = args[0] as string;
48+
const url = args[args.length - 3] as string;
49+
const offset = args[args.length - 2] as number;
3450
if (!url || previewed.has(url)) return full;
3551

52+
// URL is the label of a markdown link [url](...) — do not insert "<" into the label.
53+
const after = body.slice(offset + full.length, offset + full.length + 2);
54+
if (after === '](') {
55+
return full;
56+
}
57+
58+
// Already a preview-suppressed destination ...](<https://...>)
59+
if (offset >= 3 && body.slice(offset - 3, offset) === '](<') {
60+
return full;
61+
}
62+
3663
// If the URL is already wrapped as <url>, leave it alone.
3764
const urlIndex = body.indexOf(url, offset);
3865
if (urlIndex !== -1 && body.slice(urlIndex - 1, urlIndex + url.length + 1) === `<${url}>`) {

‎src/app/plugins/markdown/htmlToMarkdown.test.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,11 @@ describe('htmlToMarkdown', () => {
6767
expect(htmlToMarkdown(html)).toBe('[https://example.org/](<https://example.org/>)');
6868
});
6969

70+
it('converts hidden-preview wrapped links when angle brackets are decimal entities', () => {
71+
const html = '<p>&#60;<a href="https://example.org/">https://example.org/</a>&#62;</p>';
72+
expect(htmlToMarkdown(html)).toBe('[https://example.org/](<https://example.org/>)');
73+
});
74+
7075
it('converts spoiler spans', () => {
7176
expect(htmlToMarkdown('<span data-mx-spoiler>hidden</span>')).toContain('||hidden||');
7277
});

‎src/app/plugins/markdown/htmlToMarkdown.ts‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,17 @@ function processInlineElements(
196196
return processChildren(node.children, listDepth, insideCode);
197197
}
198198

199+
/** Text node is a literal or entity-encoded angle bracket (preview-suppressed autolink wrapper). */
200+
function isOpeningAngleBracketText(data: string): boolean {
201+
const t = data.trim();
202+
return t === '<' || t === '&lt;' || t === '&#60;' || t === '&#x3c;';
203+
}
204+
205+
function isClosingAngleBracketText(data: string): boolean {
206+
const t = data.trim();
207+
return t === '>' || t === '&gt;' || t === '&#62;' || t === '&#x3e;';
208+
}
209+
199210
function processChildren(
200211
children: ChildNode[],
201212
listDepth: number = 0,
@@ -213,14 +224,15 @@ function processChildren(
213224
next &&
214225
next2 &&
215226
isText(cur) &&
216-
cur.data === '<' &&
227+
isOpeningAngleBracketText(cur.data) &&
217228
isTag(next) &&
218229
next.name.toLowerCase() === 'a' &&
219230
isText(next2) &&
220-
next2.data === '>'
231+
isClosingAngleBracketText(next2.data)
221232
) {
222233
const href = next.attribs.href ?? '';
223234
const content = next.children.map((c) => processNode(c, listDepth, insideCode)).join('');
235+
// Suppressed autolink: [label](<href>) so bracket text is not run through escapeMarkdown as "\<".
224236
out.push(`[${content}](<${href}>)`);
225237
i += 2;
226238
continue;

0 commit comments

Comments
 (0)