Skip to content

Fix URL corruption caused by HTML escaping - #1124

Closed
SyntaxNyah wants to merge 2 commits into
AttorneyOnline:masterfrom
SyntaxNyah:master
Closed

Fix URL corruption caused by HTML escaping#1124
SyntaxNyah wants to merge 2 commits into
AttorneyOnline:masterfrom
SyntaxNyah:master

Conversation

@SyntaxNyah

@SyntaxNyah SyntaxNyah commented May 26, 2026

Copy link
Copy Markdown

URLs containing & for example discord and stuff were being corrupted because toHtmlEscaped() was called before
URL detection, turning & into amp. inside link hrefs.

This shit has been pissing me off for far too long. This fix detects urls on the raw message then html escapes only the non url text. Applied to server descriptions and OOC chat. Heres a screenshot of me building the client and testing it and yes the discord link downloaded instead of being an invalid &amp link at least. It works and functions.
image

If merged this would close this #1116 issue. Feel free to review if people want to improve this or just merge as is do what you like. It does build though.

URLs containing & for example discord and stuff were being corrupted because   toHtmlEscaped() was called before
 URL detection, turning & into amp. inside link hrefs.

This shit has been pissing me off for far too long.  This fix detects urls on the raw message then html escapes only the non url text. Applied to server descriptions and OOC chat. Heres a  screenshot of me building the client and testing it and yes the discord link downloaded.
@SyntaxNyah

Copy link
Copy Markdown
Author

I hate QT.

@Ganty1999

Copy link
Copy Markdown

(the "while" loop is acceptable and confirmed..
if devs merge this..)

@WisoAltred

Copy link
Copy Markdown
Contributor

Thank you for your contribution in dealing with #1116. This looks to work fine. Apologies that it took some time to check up on this,

The loop is probably fine, though there does exist an iteration beyond this with a simpler implementation for this particular issue now in 1129. For that reason, we'll go with that one. I'll note down that this was originally presented here.

For future reference, it helps to create a branch on master with the fix or change provided so that people can switch and test/add onto it easier, instead of merging from a fork. It's fine though. Thank you for your time on it.

@WisoAltred WisoAltred closed this Jun 17, 2026
@SyntaxNyah

Copy link
Copy Markdown
Author

Thank you for your contribution in dealing with #1116. This looks to work fine. Apologies that it took some time to check up on this,

The loop is probably fine, though there does exist an iteration beyond this with a simpler implementation for this particular issue now in 1129. For that reason, we'll go with that one. I'll note down that this was originally presented here.

For future reference, it helps to create a branch on master with the fix or change provided so that people can switch and test/add onto it easier, instead of merging from a fork. It's fine though. Thank you for your time on it.

Yeah Tricky's Implementation seems more structured and cleaner, than this loop. It was just a band aid fix to a frustrating problem. Was a quick fix in the heat of the moment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants