Skip to content

Displayed formatted_body is erroneously parsed as markdown: code review and potential fix #5657

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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
cloudrac3r opened this issue Mar 29, 2022 · 2 comments
Labels
A-Composer A-Markdown O-Occasional Affects or can be seen by some users regularly or most users rarely S-Minor Impairs non-critical functionality or suitable workarounds exist T-Defect Something isn't working: bugs, crashes, hangs and other reported problems

Comments

@cloudrac3r
Copy link
Contributor

A minimum example to reproduce is an event with "formatted_body": "*test*". Desktop correctly displays this as *test* (no italics). Element Android incorrectly displays this as test (italics).
The issue has already been described effectively in issue #350 and in issue #1375.
#3580 is caused by this issue. Sending a message with the content \*test\* from Element Android will create an event that exhibits the problem.

I believe the source of the issue is here: HTML messages are rendered by putting them into Markwon. That's what the htmlRenderer is. However, Markwon renders both HTML and Markdown. So any Markdown present in the formatted_body will be displayed again, which is exactly the issue we're seeing. https://github.com./vector-im/element-android/blob/ebee66cfafd6e9a0b31b5e992d22011f30271309/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/factory/MessageItemFactory.kt#L606

A fix would be to disable the default Markdown features of Markwon, leaving it to only handle the HTML using the HTML plugin. It might not be obvious how to do this, but luckily, Markwon has an example that shows exactly how. https://github.com./noties/Markwon/blob/72f6174db9530d2fab976b75a1451b63740390f7/app-sample/src/main/java/io/noties/markwon/app/samples/NoParsingSample.java
This line disables inline parsing (like bold and italics): .usePlugin(MarkwonInlineParserPlugin.create(MarkwonInlineParser.factoryBuilderNoDefaults()))
This line disables block parsing (like lists and quotes): builder.enabledBlockTypes(Collections.emptySet());
So you'd just have to add this to the Element Android code, in whatever your htmlRenderer is doing.

I'm not experienced enough with Element Android's code to be able to propose a fix that is integrated into the code, but I am able to run custom code on my cell phone, so I can test any full solutions if that would help.

Thank you so much! Here's hoping we can fix this so that people can send messages that contain asterisks. ☺️

@bmarty bmarty added T-Defect Something isn't working: bugs, crashes, hangs and other reported problems A-Markdown A-Composer S-Minor Impairs non-critical functionality or suitable workarounds exist O-Occasional Affects or can be seen by some users regularly or most users rarely labels Apr 1, 2022
@tusooa
Copy link

tusooa commented May 24, 2022

Can confirm.

@ouchadam
Copy link
Contributor

fixed by #6357 and will be available in 1.4.26

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Composer A-Markdown O-Occasional Affects or can be seen by some users regularly or most users rarely S-Minor Impairs non-critical functionality or suitable workarounds exist T-Defect Something isn't working: bugs, crashes, hangs and other reported problems
Projects
None yet
Development

No branches or pull requests

4 participants