Android - Chat - App crashes when sending 2 emojis consecutively

[kavimuru]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Action Performed:

1. Launch the App
2. Log in with any account
3. Select any user
4. Go to compose box and click on Emoji menu
5. Select any emoji and send it
6. Click on Emoji menu and pick another one emoji

Expected Result:

Emoji should be picked and sent without any issue

Actual Result:

App is crashing when selecting a second emoji

Workaround:

Unknown

Platform:

Where is this issue occurring?

Web
iOS
Android ✔️
Desktop App
Mobile Web

Version Number: 1.0.72-0
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos:

Bug5119693_Screen_Recording_20210618-220659_Expensifycash.mp4

log.txt

Expensify/Expensify Issue URL:

View all open jobs on Upwork

[OSBotify]

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open StagingDeployCash deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[MelvinBot]

Triggered auto assignment to @joelbettner (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[dklymenk]

The issue is caused by the fact that onSelectionChange of RN's TextInput in only called if the component is focused (a cursor is active). Since cursor is not active after selecting an emoji, sending a message will clear the input, but won't update the selection state. Then, in this state, if you try to add an emoji to clear input, the math for calculating the new selection will yield an invalid result (it will assume you have 2 emojis and not 1). Setting the invalid selection on android causes this error, that in prod mode causes a crash.

Now there is already some code in ReportActionCompose.js for setting focus back to the TextInput after selecting an emoji:

https://github.com/Expensify/Expensify.cash/blob/main/src/pages/home/report/ReportActionCompose.js#L326-L339

However, this.focus() will never work on native, because there is a check that never passes because this.shouldFocusInputOnScreenFocus is always false on native.

https://github.com/Expensify/Expensify.cash/blob/main/src/pages/home/report/ReportActionCompose.js#L220-L228

Even if we change this logic, this will only fix the issue in the context of the reproduction steps outlined in the OP. One will still be able to reproduce a crash by typing a message or sending an emoji, tapping elsewhere on the screen to make the TextInput lose focus, sending a message and inserting an emoji through emoji picker. In light of this, I propose two solutions:

Solution 1

Just reset the selection state manually each time we submit the form. Add this code:

this.setState({selection: {start: 0, end: 0}});

to this handler https://github.com/Expensify/Expensify.cash/blob/main/src/pages/home/report/ReportActionCompose.js#L350-L370

Solution 2

Since adding an emoji is the only way to insert something or change the selection inside the TextInput without cursor being active, another approach I see is making sure current selection is valid before doing math with it while adding the emoji. That involves adding some code to this function

https://github.com/Expensify/Expensify.cash/blob/main/src/pages/home/report/ReportActionCompose.js#L321-L339

It will look like this:

    addEmojiToTextBox(emoji) {
        this.hideEmojiPicker();
        const {selection} = this.state;
        
        // Make sure the selection is not out of bounds
        if (selection.end > this.comment.length) {
            selection.end = this.comment.length;
            selection.start = this.comment.length;
        }

        this.textInput.value = this.comment.slice(0, selection.start)
            + emoji + this.comment.slice(selection.end, this.comment.length);
        const updatedSelection = {
            start: selection.start + emoji.length,
            end: selection.start + emoji.length,
        };
        this.setState({selection: updatedSelection});
        this.setIsFocused(true);
        this.focus();
        this.updateComment(this.textInput.value);
    }

I would personally be happy with the first solution as it seems more bulletproof to me, I don't see anything going wrong here with a simple state reset there. One might be concerned with an additional re-render, but that shouldn't be noticeable as there is already a ton re-renders happening on that component and also I would I assume that the additional state update would be bundled with other updates in that callback.

[aliabbasmalik8]

PROPOSAL

I can fix this by removing selection props in text input in android and handle things using direct manipulation in android if there is some need.
after that, for android, no more produce this issue.

this approach also fix this issue as well issue

Thanks

[joelbettner]

@dklymenk I like your first solution. Feel free to assign yourself to this issue and start working on it.

Thanks!

[Jag96]

I'm able to reproduce this on 1.0.71-0 but I'm unable to reproduce this on 1.0.73-0, @kavimuru can you confirm that the issue is still occurring on the latest version? Let's hold off on exporting this until we can confirm it's still happening

[isagoico]

Issue is not reproducible in build 1.0.73.

[parasharrajat]

It seems to be fixed by the PR #3392

[joelbettner]

That's great news! I'm going to close out this issue.