Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fixes improper escaping in edit message composer #8600

Closed
wants to merge 1 commit into from
Closed

Fixes improper escaping in edit message composer #8600

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion src/editor/deserialize.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ const LIST_TYPES = ["UL", "OL", "LI"];

// Escapes all markup in the given text
function escape(text: string): string {
return text.replace(/[\\*_[\]`<]|^>/g, match => `\\${match}`);
return text.replace(/[\\*_[\]`<]|^>/g, match => `${match}`);
Copy link
Member

Choose a reason for hiding this comment

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

This replace is now doing absolutely nothing? You're replacing match with itself

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, you're right. I could have done better just wanted to push my changes to get assigned. So i can continue working on it later.

Copy link
Contributor Author

@yaya-usman yaya-usman May 18, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Nonetheless it seems to work without any problem even though it seems illogical, I'll probably need your help. I guess I need to figure out if any other part of the project is using this escape function if not I should probably remove it what do you think or figure out a better way to handle it

Copy link
Member

Choose a reason for hiding this comment

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

Well no, it regresses the reason this code was added in the first place.
See element-hq/element-web#10722 + #8004

}

// Finds the length of the longest backtick sequence in the given text, used for
Expand Down