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.

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

Fixes improper escaping in edit message composer #8600

Closed
wants to merge 1 commit into from

Conversation

yaya-usman
Copy link
Contributor

@yaya-usman yaya-usman commented May 14, 2022

Fixes: vector-im/element-web#22127

Signed-off-by: Yaya Usman yaya-usman@users.noreply.github.com

Original Text
original
Before:

onEditbefore

After:
onEditAfter

Type: defect


Here's what your changelog entry will look like:

馃悰 Bug Fixes

  • Fixes improper escaping in edit message composer (#8600). Contributed by @yaya-usman.

@yaya-usman yaya-usman requested a review from a team as a code owner May 14, 2022 21:12
@github-actions github-actions bot added the T-Defect Bugs, crashes, hangs, vulnerabilities, or other reported problems label May 14, 2022
@@ -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

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

@MadLittleMods MadLittleMods added the Z-Community-PR Issue is solved by a community member's PR label Jun 1, 2022
@t3chguy
Copy link
Member

t3chguy commented Jul 7, 2023

Abandoned - reopen if you are working on it.

@t3chguy t3chguy closed this Jul 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-Defect Bugs, crashes, hangs, vulnerabilities, or other reported problems Z-Community-PR Issue is solved by a community member's PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Square bracket escaped in edit composer
3 participants