[$250] Android - Mark down - App crashes when deleting text with mark down in a sentence with heading

[lanitochka17]

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

---

Version Number: 9.0.17-0 Reproducible in staging?: Y Reproducible in production?: Y If this was caught during regression testing, add the test name, ID and link from TestRail: N/A Email or phone of affected tester (no customers): applausetester+kh050806@applause.expensifail.com Issue reported by: Applause - Internal Team

Action Performed:

Precondition:

• Device keyboard must be Samsung keyboard
• Issue is not reproducible with Gboard
  1. Launch New Expensify app
  2. Go to chat
  3. Type # hey hey
  4. Double tap to select and highlight the second word with mark down hey
  5. Tap delete on Samsung keyboard

Expected Result:

App will not crash

Actual Result:

App crashes This issue is only reproducible with Samsung keyboard and not Gboard

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• [x] Android: Native
• [ ] Android: mWeb Chrome
• [ ] iOS: Native
• [ ] iOS: mWeb Safari
• [ ] MacOS: Chrome / Safari
• [ ] MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/user-attachments/assets/b0bc1b8b-a385-4021-a81a-38a074fb0cb6

logs (2).txt

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01bc8d3436a587add7
• Upwork Job ID: 1821616561702134137
• Last Price Increase: 2024-08-29
• Automatic offers:
• alitoshmatov | Reviewer | 103818157
• wildan-m | Contributor | 103818158

Issue Owner

Current Issue Owner: @isabelastisser

[melvin-bot[bot]]

Triggered auto assignment to @isabelastisser (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[lanitochka17]

We think that this bug might be related to #Live Markdown

[lanitochka17]

@isabelastisser FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

[melvin-bot[bot]]

Job added to Upwork: https://www.upwork.com/jobs/~01bc8d3436a587add7

[melvin-bot[bot]]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @alitoshmatov (External)

[BartoszGrajdek]

cc @tomekzaw 👀

[isabelastisser]

No proposals yet.

[melvin-bot[bot]]

@isabelastisser, @alitoshmatov Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[melvin-bot[bot]]

@isabelastisser, @alitoshmatov Huh... This is 4 days overdue. Who can take care of this?

[isabelastisser]

Still waiting for proposals.

[melvin-bot[bot]]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[melvin-bot[bot]]

@isabelastisser, @alitoshmatov 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

[alitoshmatov]

Waiting for proposals

[melvin-bot[bot]]

@isabelastisser @alitoshmatov this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[melvin-bot[bot]]

@isabelastisser, @alitoshmatov Whoops! This issue is 2 days overdue. Let's get this updated quick!

[isabelastisser]

No proposals yet.

[melvin-bot[bot]]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[melvin-bot[bot]]

@isabelastisser, @alitoshmatov Eep! 4 days overdue now. Issues have feelings too...

[alitoshmatov]

No proposals yet

[melvin-bot[bot]]

@isabelastisser, @alitoshmatov Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[isabelastisser]

Still waiting for proposals.

[melvin-bot[bot]]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[melvin-bot[bot]]

@isabelastisser, @alitoshmatov Huh... This is 4 days overdue. Who can take care of this?

[isabelastisser]

no proposals yet.

[melvin-bot[bot]]

@isabelastisser, @alitoshmatov 8 days overdue is a lot. Should this be a Weekly issue? If so, feel free to change it!

[shubham1206agra]

@tomekzaw might be able to help here.

[wildan-m]

Edited by proposal-police: This proposal was edited at 2024-09-03 07:48:57 UTC.

Proposal

Please re-state the problem that we are trying to solve in this issue.

App crashes when deleting text with markdown in a sentence containing a heading on Android.

What is the root cause of that problem?

This is a react-native-live-markdown issue where markdown formatting is applied in the onTextChanged event.

https://github.com/Expensify/react-native-live-markdown/blob/73488ad3b7e0ece4ca5ffe76c10b0ba124a9b27d/android/src/main/java/com/expensify/livemarkdown/MarkdownTextWatcher.java#L24-L32

When applying formatting, there may be a race condition if we are trying to remove a span that has already been removed.

The issue is not keyboard-specific; certain keyboards may experience delays that result in accidental errors. The most straightforward way to replicate the issue is:

1. type: #a
2. double tap a, ensure only a letter selected
3. when tap action shown, choose cut

https://github.com/user-attachments/assets/21bac680-bbd7-43d4-87d3-78baa454a769

What changes do you think we should make in order to solve the problem?

To avoid the race condition, move applyMarkdownFormatting from onTextChanged to afterTextChanged after the text has changed.

Change:

https://github.com/Expensify/react-native-live-markdown/blob/73488ad3b7e0ece4ca5ffe76c10b0ba124a9b27d/android/src/main/java/com/expensify/livemarkdown/MarkdownTextWatcher.java#L24-L37

To

  @Override
  public void onTextChanged(CharSequence s, int start, int before, int count) {
    if (mShouldSkip) {
      return;
    }
    // Set the flag to indicate text is being changed
    mShouldSkip = true;
  }

  @Override
  public void afterTextChanged(Editable editable) {
      if (!mShouldSkip) {
          return;
      }

      if (editable instanceof SpannableStringBuilder) {
          mMarkdownUtils.applyMarkdownFormatting((SpannableStringBuilder) editable);
      }

      // Reset the flag after formatting is applied
      mShouldSkip = false;
  }

Result

https://github.com/user-attachments/assets/27248373-b430-4c30-bbf0-63b9ac623004

RN Markdown Branch for this solution

What alternative solutions did you explore? (Optional)

N/A

[wildan-m]

Proposal Updated

• Add result video

[alitoshmatov]

@wildan-m Thank you for your proposal, I think your RCA is correct and we can go with you solution

C+ reviewed 🎀 👀 🎀

[melvin-bot[bot]]

Triggered auto assignment to @neil-marcellini, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[melvin-bot[bot]]

@neil-marcellini @isabelastisser @alitoshmatov this issue is now 4 weeks old, please consider:

• Finding a contributor to fix the bug
• Closing the issue if BZ has been unable to add the issue to a VIP or Wave project
• If you have any questions, don't hesitate to start a discussion in #expensify-open-source

Thanks!

[isabelastisser]

We have chosen a proposal and are waiting for @neil-marcellini's review.

[neil-marcellini]

@wildan-m Thank you for your proposal, I think your RCA is correct and we can go with you solution

C+ reviewed 🎀 👀 🎀

Wow great job! Hiring

[melvin-bot[bot]]

📣 @alitoshmatov 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link Upwork job

[melvin-bot[bot]]

📣 @wildan-m 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link Upwork job Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻 Keep in mind: Code of Conduct | Contributing 📖

[wildan-m]

@alitoshmatov The upstream PR is ready. https://github.com/Expensify/react-native-live-markdown/pull/469

[melvin-bot[bot]]

@wildan-m, @neil-marcellini, @isabelastisser, @alitoshmatov Eep! 4 days overdue now. Issues have feelings too...

[isabelastisser]

@alitoshmatov @wildan-m, can you please provide an update? Thanks!

[wildan-m]

@isabelastisser discussing optimal approach to avoid duplicate calls in upstream. https://github.com/Expensify/react-native-live-markdown/pull/469#issuecomment-2335002872

[tomekzaw]

https://github.com/Expensify/react-native-live-markdown/pull/469 has just been merged

[wildan-m]

Thank you, @tomekzaw!

@alitoshmatov @neil-marcellini Should we simply update the version in the E/App to apply it? I see that the live markdown version in E/App is not the latest. Are there specific steps to follow?

[tomekzaw]

There's already a PR that bumps react-native-live-markdown here: https://github.com/Expensify/App/pull/48626

[wildan-m]

@tomekzaw can we bump the version to 0.1.140 there or we should temporarily create patch until that PR closed?

[tomekzaw]

Let's avoid patches and bump directly to latest if possible

[melvin-bot[bot]]

@wildan-m, @neil-marcellini, @isabelastisser, @alitoshmatov 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

[tomekzaw]

Should be fixed with https://github.com/Expensify/App/pull/48626

[alitoshmatov]

Waiting for react-native-live-markdown bump on https://github.com/Expensify/App/pull/48626

[mvtglobally]

Issue not reproducible during KI retests. (First week)

[melvin-bot[bot]]

@wildan-m, @neil-marcellini, @isabelastisser, @alitoshmatov Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[wildan-m]

Not overdue, The fix PR passed the test and is now live in production https://github.com/Expensify/App/pull/48626#issuecomment-2353604788.