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

Emojis in emotes causing crashes #4744

Merged
merged 3 commits into from
Dec 17, 2021
Merged

Conversation

ouchadam
Copy link
Contributor

Fixes #4743

A follow up to the original emoji fix #4698, we had a missing case (the emote section eg /me :tada:)

BEFORE AFTER
before-emoji-crash after-emoji-crash

- fixes android 12+ crash when certain emojis are used within the emote content
@github-actions
Copy link

github-actions bot commented Dec 16, 2021

Unit Test Results

  66 files  ±0    66 suites  ±0   52s ⏱️ -7s
135 tests ±0  135 ✔️ ±0  0 💤 ±0  0 ±0 
418 runs  ±0  418 ✔️ ±0  0 💤 ±0  0 ±0 

Results for commit 9e2fb13. ± Comparison against base commit 3b35be5.

♻️ This comment has been updated with latest results.

Copy link
Member

@bmarty bmarty left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@@ -625,6 +625,7 @@ class MessageItemFactory @Inject constructor(
message(message)
}
}
.bindingOptions(bindingOptions)
Copy link
Member

Choose a reason for hiding this comment

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

If only I had checked all the instance of MessageTextItem_() in this class...

changelog.d/4743.bugfix Outdated Show resolved Hide resolved
Copy link
Member

@ganfra ganfra 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 catch, are we sure its ok on all items now?

@ganfra ganfra merged commit ac65942 into develop Dec 17, 2021
@ganfra ganfra deleted the feature/adm/emoji-notice-fix branch December 17, 2021 10:47
@ouchadam
Copy link
Contributor Author

ouchadam commented Dec 17, 2021

Thanks for the catch, are we sure its ok on all items now?

I'm not 100% as any expoy attribute which can contain an emoji is at risk on android 12+

@ouchadam ouchadam changed the title Emojis in emotes causing crashes on Android 12+ Emojis in emotes causing crashes ~on Android 12+~ Dec 21, 2021
@ouchadam ouchadam changed the title Emojis in emotes causing crashes ~on Android 12+~ Emojis in emotes causing crashes Dec 21, 2021
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.

Certain emojis crash on Android 12+ when used in notices
4 participants