[$125] Markdown - Markdown is not rendered if there is a space after a triple backtick

[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: .0.5-8 Reproducible in staging?: Y Reproducible in production?: Y If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4700427 Email or phone of affected tester (no customers): dave0123seife+stest@gmail.com Issue reported by: Applause - Internal Team

Action Performed:

1. Go to any conversation
2. Send a message with a triple back tick with the following format

   test

3. Important step: Make sure there is a space after the top backtick.

Expected Result:

Markdown is rendered

Actual Result:

Markdown is not rendered if there is a space after a triple backtick

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/e823e85f-17d6-4970-bb21-d5689220145d

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0177eb2919e6316787
• Upwork Job ID: 1811456787683573189
• Last Price Increase: 2024-08-19
• Automatic offers:
• akinwale | Reviewer | 103254066
• devguest07 | Contributor | 103254071
• tsa321 | Contributor | 103602722

Issue Owner

Current Issue Owner: @akinwale

Issue Owner

Current Issue Owner: @devguest07

[melvin-bot[bot]]

Triggered auto assignment to @greg-schroeder (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]

@greg-schroeder 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

[lanitochka17]

We think that this bug might be related to #vip-vsp

[jacobkim9881]

Small change can fix the issue at https://github.com/Expensify/react-native-live-markdown

[tsa321]

Edited by proposal-police: This proposal was edited at 2024-08-20 12:46:23 UTC.

Proposal

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

Markdown is not rendered if there is a space after a triple backtick

What is the root cause of that problem?

In here:

https://github.com/Expensify/expensify-common/blob/5373eff291c28766607e3bc9491822aa1381cdf4/lib/ExpensiMark.ts#L132

We don't consider whitespace after the three backticks.

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

We could modify the regex as follows:

regex: /(```.*?(?:\r\n|\n))((?:\s*?(?!(?:\r\n|\n)?```(?!`))[\S])+\s*?(?:\r\n|\n))\s*(```)/g,

Adding .*? after the three backticks.

The unit test for this issue could be:

    codeFenceExample = '```   \r\n# test\r\n```';
    expect(parser.replace(codeFenceExample)).toBe('<pre>#&#32;test<br /></pre>');

    codeFenceExample = '```test\r\n# test\r\n```';
    expect(parser.replace(codeFenceExample)).toBe('<pre>#&#32;test<br /></pre>');

and put it in this clause:

https://github.com/Expensify/expensify-common/blob/e14dff64438b19fc9b6aa6937502b64870e0be86/__tests__/ExpensiMark-HTML-test.js#L444-L449

[devguest07]

Proposal

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

Markdown is not rendered if there is a space after a triple backtick

What is the root cause of that problem?

The issue in the code is with the regex pattern used to match the code fence. The current pattern is too strict and doesn't allow for any characters or spaces after the opening backticks. Let's break down the problem and provide a solution:

https://github.com/Expensify/expensify-common/blob/5373eff291c28766607e3bc9491822aa1381cdf4/lib/ExpensiMark.ts#L131-L132

/(```(?:\r\n|\n))((?:\s*?(?!(?:\r\n|\n)?```(?!`))[\S])+\s*?(?:\r\n|\n))(```)/g

The pattern (```(?:\r\n|\n)) at the beginning expects the three backticks to be immediately followed by a newline character, without allowing any space or other characters in between.

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

We need to modify the regex to allow for optional characters after the opening backticks, before the newline. Here's an updated version of the regex that should solve your issue:

/(```.*?(?:\r\n|\n))((?:\s*?(?!(?:\r\n|\n)?```(?!`))[\S])+\s*?(?:\r\n|\n))(```)/g

The key change is replacing (?:\r\n|\n) with .*?(?:\r\n|\n) after the opening backticks. This allows for any characters (including spaces) between the backticks and the newline.

Here's the updated code block with the new regex:

{
    name: 'codeFence',
    regex: /(```.*?(?:\r\n|\n))((?:\s*?(?!(?:\r\n|\n)?```(?!`))[\S])+\s*?(?:\r\n|\n))(```)/g,
    replacement: (_extras, _match, _g1, textWithinFences) => {
        const group = textWithinFences.replace(/(?:(?![\n\r])\s)/g, ' ');
        return `<pre>${group}</pre>`;
    },
    rawInputReplacement: (_extras, _match, _g1, textWithinFences) => {
        const group = textWithinFences.replace(/(?:(?![\n\r])\s)/g, '&#32;').replace(/<emoji>|<\/emoji>/g, '');
        return `<pre>${group}</pre>`;
    },
}

This modification should allow the code to detect and process code fences correctly, even when there are spaces or other characters after the opening backticks.

What alternative solutions did you explore?

[melvin-bot[bot]]

Job added to Upwork: https://www.upwork.com/jobs/~0177eb2919e6316787

[melvin-bot[bot]]

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

[melvin-bot[bot]]

Upwork job price has been updated to $125

[ShridharGoel]

Proposal

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

Markdown is not rendered if there is a space after a triple backtick.

What is the root cause of that problem?

Existing codeFence logic doesn't handle whitespaces after opening backticks or before closing backticks.

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

Use the below updated code:

{
    name: 'codeFence',
    // ` is a backtick symbol we are matching on three of them before then after a new line character
    // Adjusting to account for spaces before and after the triple backticks
    regex: /(```(\r?\n|\r)?)([\s\S]*?)(\r?\n|\r)?(```)/g,
    // We're using a function here to perform an additional replace on the content
    // inside the backticks because Android is not able to use <pre> tags and does
    // not respect whitespace characters at all like HTML does. We do not want to mess
    // with the new lines here since they need to be converted into <br>. And we don't
    // want to do this anywhere else since that would break HTML.
    // &nbsp; will create styling issues so use &#32;
    replacement: (_extras, _match, _g1, _g2, textWithinFences) => {
        const group = textWithinFences.trim().replace(/(?:(?![\n\r])\s)/g, '&#32;');
        return `<pre>${group}</pre>`;
    },
    rawInputReplacement: (_extras, _match, _g1, newLineCharacter, textWithinFences) => {
        const group = textWithinFences.trim().replace(/(?:(?![\n\r])\s)/g, '&#32;').replace(/<emoji>|<\/emoji>/g, '');
        return `<pre>${newLineCharacter}${group}</pre>`;
    },
},

The regex /(```(\r?\n|\r)?)([\s\S]*?)(\r?\n|\r)?(```)/g has been adjusted to account for spaces and optional newline characters before and after the triple backticks.

(\r?\n|\r)? matches optional newline characters after the opening backticks and before the closing backticks. ([\s\S]*?) matches any character, including spaces and newlines, in a non-greedy manner to avoid capturing extra content.

The textWithinFences.trim() method was added to remove leading and trailing whitespace characters, including newlines, before replacing spaces with  . This ensures that the content within the backticks does not have extra lines.

Apply same logic in live-markdown as well if we want to fix the live markdown UX as well.

[melvin-bot[bot]]

@akinwale, @greg-schroeder Eep! 4 days overdue now. Issues have feelings too...

[greg-schroeder]

@akinwale is up next on reviewing 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]]

@akinwale, @greg-schroeder 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

[akinwale]

Reviewing proposals today.

[akinwale]

The solution in @devguest07's proposal is the best approach here.

@ShridharGoel I don't see a point in trimming whitespace characters, and that could introduce a bug, if the whitespace characters were intentionally added by the user.

🎀👀🎀 C+ reviewed.

[melvin-bot[bot]]

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

[AndrewGable]

@devguest07 @akinwale - Do we have existing tests that will verify this change will not break anything? Can we update the tests with this new case?

[devguest07]

@AndrewGable I'll create new tests to cover this specific case. I'll also review the existing markdown tests to ensure we maintain current functionality while implementing these changes.

[greg-schroeder]

Not overdue, trying to decide whether we're ready to move forward with @devguest07's proposal

[melvin-bot[bot]]

@AndrewGable @akinwale @greg-schroeder 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]]

📣 @akinwale 🎉 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]]

📣 @devguest07 🎉 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 📖

[greg-schroeder]

Next up is PR from @devguest07

[melvin-bot[bot]]

@AndrewGable, @akinwale, @greg-schroeder, @devguest07 Eep! 4 days overdue now. Issues have feelings too...

[greg-schroeder]

Any update here @devguest07?

[devguest07]

Sorry was on vacation, I will create the PR ASAP.

[melvin-bot[bot]]

@AndrewGable @akinwale @greg-schroeder @devguest07 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!

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

@AndrewGable, @akinwale, @greg-schroeder, @devguest07 Whoops! This issue is 2 days overdue. Let's get this updated quick!

[AndrewGable]

What's the latest here?

[greg-schroeder]

bump @devguest07

[melvin-bot[bot]]

@AndrewGable, @akinwale, @greg-schroeder, @devguest07 Whoops! This issue is 2 days overdue. Let's get this updated quick!

[tsa321]

Can I raise a PR since my proposal is quite similar to @devguest07's proposal?

[mountiny]

Unassigning the contributor for lack of urgency on the solution.

@akinwale open to other proposals

[tsa321]

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

[akinwale]

@tsa321 Can you run the unit tests after applying your proposed solution to make sure they all pass? You would also need to add tests for the updates if your proposal is selected as previously indicated.

[tsa321]

@akinwale yes all test passed:

The unit test for this issue could be:

    codeFenceExample = '```   \r\n# test\r\n```';
    expect(parser.replace(codeFenceExample)).toBe('<pre>#&#32;test<br /></pre>');

    codeFenceExample = '```test\r\n# test\r\n```';
    expect(parser.replace(codeFenceExample)).toBe('<pre>#&#32;test<br /></pre>');

and put it in this clause:

https://github.com/Expensify/expensify-common/blob/e14dff64438b19fc9b6aa6937502b64870e0be86/__tests__/ExpensiMark-HTML-test.js#L444-L449

I have updated my proposal.

[akinwale]

@AndrewGable We can reassign this to @tsa321 based on the proposed solution.

🎀👀🎀 C+ reviewed.

[melvin-bot[bot]]

Current assignee @AndrewGable is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

[melvin-bot[bot]]

📣 @tsa321 🎉 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 📖

[tsa321]

@akinwale PR is ready.

[akinwale]

@tsa321 The OP in the PR you created does not follow the PR author checklist template. You would need to close the PR and create a new one which follows the template.

[tsa321]

@akinwale, I am following the expensify-common PR template. Should I also include the author checklist from the App repository PR template?

After the expensify-common PR is merged, I will create a follow-up PR in the App repository to bump the expensify-common version. I will include the usual author checklist in that PR.

Maybe we need to inform @AndrewGable to assign @akinwale in the expensify-common PR.

[melvin-bot[bot]]

@AndrewGable, @akinwale, @greg-schroeder, @tsa321 Eep! 4 days overdue now. Issues have feelings too...

[tsa321]

PR is under review

[greg-schroeder]

Thanks @tsa321!

[melvin-bot[bot]]

@AndrewGable, @akinwale, @greg-schroeder, @tsa321 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[tsa321]

PR is waiting review from @akinwale