[$250] Compose Box - Bold markdown is not working if followed by an underscore

[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: 1.4.85-6 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): nahummelaku9+cd1@gmail.com Issue reported by: Applause - Internal Team

Action Performed:

1. Log into staging.new.expensify.com
2. Navigate to a conversation
3. Write a text with a bold markdown
4. Write an underscore next to the bold markdown
5. Send the text in the compose box

Expected Result:

A text in bold followed by an underscore is shown in the conversation

Actual Result:

Asterix on both sides of the text and an underscore are sent to the conversation

Workaround:

Unknown

Platforms:

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

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

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/Expensify/App/assets/78819774/2c6199e1-a404-425d-af0f-de8a6117dcb7

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0164dc21fd43760287
• Upwork Job ID: 1805749444451342734
• Last Price Increase: 2024-07-09

Issue Owner

Current Issue Owner: @parasharrajat

[melvin-bot[bot]]

Triggered auto assignment to @dylanexpensify (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

[dylanexpensify]

cc @mallenexpensify

[mallenexpensify]

Just need to add to Live Markdown project, I bumped to Weekly since the folks are prioritizing fixes over there. I'd await an update from them.

[BartoszGrajdek]

Hi! I'm Bartosz from SWM react-native-live-markdown team.

I believe this issue could be handled by external contributors. It's probably going to require some changes in expensify-common, because it seems like that's the part responsible for the issue. WDYT @mallenexpensify ?

[melvin-bot[bot]]

Job added to Upwork: https://www.upwork.com/jobs/~0164dc21fd43760287

[melvin-bot[bot]]

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

[mallenexpensify]

Thanks @BartoszGrajdek , made external, help wanted added too.

[jp928]

Proposal

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

Asterisk on both sides of the text and an underscore are sent to the conversation without markdown style.

What is the root cause of that problem?

In the code:

https://github.com/Expensify/App/blob/64c6624d8324b5cb78c75c4d4a8bbc16e4ca0b33/src/libs/actions/Report.ts#L1512

the markdown parse.replace is invoked.

The following regular expression is used:

https://github.com/Expensify/expensify-common/blob/2879643200dcd16041c3fd60799097fe9f188a83/lib/ExpensiMark.ts#L464

But the regular expression cannot match _*test* because of the \B makes the * has to be the start of the word. test cases as below:

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

Replace the regular expression by

              regex: /(?<!<[^>]*)(\b_|\B)\*(?![^<]*(?:<\/pre>|<\/code>|<\/a>))((?![\s*])[\s\S]*?[^\s*](?<!\s))\*\B(?![^<]*>)(?![^<]*(<\/pre>|<\/code>|<\/a>))/g,
              replacement: (_extras, match, g1, g2) => {
                  if (g1.includes('_')) {
                      return `${g1}<strong>${g2}</strong>`;
                  }

                  return g2.includes('</pre>') || this.containsNonPairTag(g2) ? match : `<strong>${g2}</strong>`;
              },

Explanation: \B is used to exclude any words ahead of the asterisk *. In the proposal, use (\b_|\B) to exclude letters and special characters except underscore _.

Test of expensify-common passed.

What alternative solutions did you explore? (Optional)

NA

[parasharrajat]

@dylanexpensify I feel like we need a list of rules around parsing syntax. This is another example of an issue where we keep on changing parsing rules and one thing and another breaks over time. I believe changing the parser open-endedly is not a good tactic.

Was this issue present since the inception? Or it is something new that we found today?

What if we solve this issue now and something else pops up?

[parasharrajat]

@jp928 Does all other tests pass on ExpensiMark?

[jp928]

@parasharrajat The current ExpensiMark works with all other special characters except _, and not work with any letters.

https://github.com/Expensify/App/assets/659612/b9e5f6ac-467d-40f3-97d2-b89e7729ed86

[jp928]

@parasharrajat Also, just notice there is another \B to prevent the markdown end with a letter, so currently *b* end with a letter doesn't work either.

[parasharrajat]

@jp928 Your solution does not pass tests for ExpensiMark.

[jp928]

@parasharrajat Thanks for reviewing. The proposal updated.

[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]]

@parasharrajat @dylanexpensify 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]]

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

[parasharrajat]

I will review this in sometime.

[parasharrajat]

@jp928 So I was checking the italic parsing rule and I see that it works just fine *_test_ works.

Why can't we just pick the existing working solution? If you think your solution is better, could you please explain the reasons?

[jp928]

@parasharrajat I tried the italic parsing rule, it will fail some test cases. /(<(pre|code|a|mention-user)[^>]*>(.*?)<\/\2>)|((\b_+|\b)\*((?![\s_])[\s\S]*?[^\s_](?<!\s))\*(?![^\W_])(?![^<]*>)(?![^<]*(<\/pre>|<\/code>|<\/a>|<\/mention-user>)))/g this is what I used to test.

[parasharrajat]

@jp928 Thanks for testing this. Could you please describe the problem or cases?

[jp928]

@parasharrajat There are more some cases the italic one can't handle: *sentence,* *Here is a multi-line\ncomment that should\nbe bold*, I just updated my proposal with minimum change to just fix the bug in this thread.

[parasharrajat]

Thanks, I will review it.

[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? 💸

[parasharrajat]

I will post the next update in few hours.

[parasharrajat]

OK, Thanks. Let's go with @jp928's proposal. We need to adjust the regular expression for sure. We can test all the edge cases on PR and more unit tests for these edge cases.

:ribbon: :eyes: :ribbon: C+ reviewed

[melvin-bot[bot]]

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

[jp928]

@parasharrajat PR is linked but not sure if the package version update is needed.

[jp928]

I have read the CLA Document and I hereby sign the CLA

[parasharrajat]

We will need two PR. One n e-common and one for package version upgrade on this repo.

[melvin-bot[bot]]

📣 @jp928 You have been assigned to this job! Please apply to the Upwork job and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻 Once you apply to this job, your Upwork ID will be stored and you will be automatically hired for future jobs! Keep in mind: Code of Conduct | Contributing 📖

[parasharrajat]

Sorry @hayata-suenaga, I didn't notice that you have yet to assign @jp928 and review the PR.

[hayata-suenaga]

not problem 😄

[parasharrajat]

@jp928 Waiting for your next PR.

[jp928]

@parasharrajat My first PR has been merged. Please advise what would be included in the next PR?

[parasharrajat]

Now, you just need to create a PR to update the version of common package in this repo.

[melvin-bot[bot]]

@jp928 @parasharrajat @dylanexpensify @hayata-suenaga 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!

[jp928]

@parasharrajat Would you like a patch or update the package.json? if we update the library version in package.json, it would include other commits.