-
Notifications
You must be signed in to change notification settings - Fork 29
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: Crash on inserting mention #WPB-15717 #3856
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #3856 +/- ##
===========================================
+ Coverage 45.23% 45.46% +0.22%
===========================================
Files 491 491
Lines 17006 17008 +2
Branches 2845 2846 +1
===========================================
+ Hits 7693 7732 +39
+ Misses 8529 8489 -40
- Partials 784 787 +3
... and 2 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nice to add tests for this case, apart from that good catch 🦅 :D
@@ -200,6 +201,10 @@ class MessageCompositionHolder( | |||
userId = UserId(contact.id, contact.domain), | |||
handler = String.MENTION_SYMBOL + contact.name | |||
) | |||
if (mention.start < 0 || mention.length < 2) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: Perhaps we can add tests for this case, I see we have a few on MessageCompositionHolderTest
:D
|
Built wire-android-staging-compat-pr-3856.apk is available for download |
Built wire-android-dev-debug-pr-3856.apk is available for download |
What's new in this PR?
Issues
Crash in PlayStore while inserting mention.
Causes (Optional)
Hard to understand exact STR, but the crash happens when mentions place in text can't be found. In that case mention's position is
-1
and crash come up inString.subSequence(0, -1)
Solutions
Added checking if mention position and length are good, do not add mention otherwise and added logs.