Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Fix links being mangled by markdown processing #9570

Merged
merged 2 commits into from
Nov 14, 2022
Merged

Fix links being mangled by markdown processing #9570

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6f3fa85
Fix links being mangled by markdown processing
weeman1337 Nov 11, 2022
14060e6
Merge branch 'develop' into weeman1337/fix-markdown-links
weeman1337 Nov 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 21 additions & 1 deletion src/Markdown.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,26 @@ const formattingChangesByNodeType = {
'strong': '__',
};

/**
* Returns the literal of a node an all child nodes.
*/
const innerNodeLiteral = (node: commonmark.Node): string => {
let literal = "";

const walker = node.walker();
let step: commonmark.NodeWalkingStep;

while (step = walker.next()) {
const currentNode = step.node;
const currentNodeLiteral = currentNode.literal;
if (step.entering && currentNode.type === "text" && currentNodeLiteral) {
literal += currentNodeLiteral;
}
}

return literal;
};

/**
* Class that wraps commonmark, adding the ability to see whether
* a given message actually uses any markdown syntax or whether
Expand Down Expand Up @@ -185,7 +205,7 @@ export default class Markdown {
* but this solution seems to work well and is hopefully slightly easier to understand too
*/
const format = formattingChangesByNodeType[node.type];
const nonEmphasizedText = `${format}${node.firstChild.literal}${format}`;
const nonEmphasizedText = `${format}${innerNodeLiteral(node)}${format}`;
Copy link
Contributor Author

@weeman1337 weeman1337 Nov 11, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The node will be unlinked https://github.com/matrix-org/matrix-react-sdk/pull/9570/files#diff-3080504149f1c5e77feab2433cd58ec989ca431665891e39534fa4969b43eb01R220

Current implementation assumes the text being in a single node inside the emphasis. If it has more levels it will drop everything except the contents of the first child node. To fix this we need to get the entire „inner“ text (= all child nodes) of the node being dropped.

I hope my thoughts are correct here. The markdown processing is kind of complex and hard to understand.

const f = getTextUntilEndOrLinebreak(node);
const newText = value + nonEmphasizedText + f;
const newLinks = linkify.find(newText);
Expand Down
12 changes: 12 additions & 0 deletions test/Markdown-test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,10 +124,22 @@ describe("Markdown parser test", () => {
const testString = [
'http://domain.xyz/foo/bar-_stuff-like-this_-in-it.jpg' + " " + 'http://domain.xyz/foo/bar-_stuff-like-this_-in-it.jpg',
'http://domain.xyz/foo/bar-_stuff-like-this_-in-it.jpg' + " " + 'http://domain.xyz/foo/bar-_stuff-like-this_-in-it.jpg',
"https://example.com/_test_test2_-test3",
"https://example.com/_test_test2_test3_",
"https://example.com/_test__test2_test3_",
"https://example.com/_test__test2__test3_",
"https://example.com/_test__test2_test3__",
"https://example.com/_test__test2",
].join('\n');
const expectedResult = [
"http://domain.xyz/foo/bar-_stuff-like-this_-in-it.jpg http://domain.xyz/foo/bar-_stuff-like-this_-in-it.jpg",
"http://domain.xyz/foo/bar-_stuff-like-this_-in-it.jpg http://domain.xyz/foo/bar-_stuff-like-this_-in-it.jpg",
"https://example.com/_test_test2_-test3",
"https://example.com/_test_test2_test3_",
"https://example.com/_test__test2_test3_",
"https://example.com/_test__test2__test3_",
"https://example.com/_test__test2_test3__",
"https://example.com/_test__test2",
].join('<br />');
/* eslint-enable max-len */
const md = new Markdown(testString);
Expand Down