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

Already on GitHub? Sign in to your account

Fix HTML entities being displayed in messages #6447

Merged
merged 2 commits into from
Jul 4, 2022

Conversation

cloudrac3r
Copy link
Contributor

@cloudrac3r cloudrac3r commented Jul 3, 2022

Type of change

  • Bugfix

Content

HTML entities are now displayed as the characters they represent.

Motivation and context

Fixes #6445. This was a regression from #6357.

Tests

My test case was an event with "formatted_body": "​ ♫ We're on easy street ♫". If successful, when displayed in Element Android, it should look like "♫ We're on easy street ♫".

I also looked at the tests I used in #6357.

Screenshots

{
  "body": "raw",
  "formatted_body": "​ ♫ We're on easy street ♫",
   "msgtype": "m.text",
   "format": "org.matrix.custom.html"
}
BEFORE AFTER
Screenshot_20220704_142338 Screenshot_20220704_141944

Tested devices

  • Physical, LineageOS 18.1 (Android 11)

Checklist

  • Changes has been tested on an Android device or Android emulator with API 21
  • N/A - UI change has been tested on both light and dark themes
  • N/A - Accessibility has been taken into account
  • Pull request is based on the develop branch
  • Pull request includes a new file under ./changelog.d. See https://github.com/vector-im/element-android/blob/develop/CONTRIBUTING.md#changelog
  • N/A - Pull request includes screenshots or videos if containing UI changes
  • Pull request includes a sign off
  • You've made a self review of your PR
  • N/A - If you have modified the screen flow, or added new screens to the application

Signed-off-by: Cadence Ember [email protected]

EDIT: @ouchadam - Added before/after screenshots

Initially reported in element-hq#6445. Fixes element-hq#6445.
This was a regression from element-hq#6357.

The fix is to enable Markwon's HTML entities processor.
@ouchadam ouchadam self-assigned this Jul 4, 2022
Copy link
Contributor

@ouchadam ouchadam left a comment

Choose a reason for hiding this comment

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

💯 thanks for the quick fix and adding some comments

I'll look into creating a test to capture regressions like this

@ouchadam ouchadam added the Z-Community-PR Issue is solved by a community member's PR label Jul 4, 2022
@ouchadam ouchadam merged commit 2c44452 into element-hq:develop Jul 4, 2022
@MyIgel
Copy link

MyIgel commented Jul 17, 2022

Shouldn't there be some regression tests that cover this change?

@cloudrac3r
Copy link
Contributor Author

Could be good. I didn't realise my original change would have as wide impacts as it did.

Fortunately, all issues around this seem to be fixed now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

Characters rendered as html tags in 1.4.26
3 participants