[$250] Markdown- Bold markdown is not rendered when copied from another app

[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
Issue reported by: Applause - Internal Team

Action Performed:

An account signed in to newdot

1. Navigate to a chat app that supports Markdown (Example: Slack)
2. Send a message containing the bold markdown
3. Copy the text (CTRL + C) with markdown and navigate back to NewDot
4. Open any conversation and paste the copied text with CTRL + V into the compose box

Expected Result:

The text is displayed with the bold markdown icon in the compose box and is rendered as bold when the text is sent

Actual Result:

The text is not displayed with the bold markdown icon in the compose box and is not rendered as bold when the text is sent

Workaround:

Unknown

Platforms:

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

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6563272_1722942138525.2024-08-06_13_59_36.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01e87ca27ee2372740
• Upwork Job ID: 1821228929551530763
• Last Price Increase: 2024-08-14

[melvin-bot]

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

@sakluger 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

[bernhardoj]

Proposal

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

Copying bold text from another app (such as Slack) isn't pasted as a bold markdown which is inconsistent with other markdowns.

What is the root cause of that problem?

The regex for bold and italic for example is pretty similar.
https://github.com/Expensify/expensify-common/blob/7351ac7e19f5cc8e694f8eb40768b9c6d6302852/lib/ExpensiMark.ts#L558-L564

The difference in bold markdown replacement is that it checks the <strong> or <b> HTML style attribute and doesn't render it as bold if the style doesn't have a font-weight of bold or 700.

https://github.com/Expensify/expensify-common/blob/7351ac7e19f5cc8e694f8eb40768b9c6d6302852/lib/ExpensiMark.ts#L565-L592

It's to fix #41109 where copying text from gdocs is rendered as bold because gdocs text HTML is b but with a style of font-weight: 400.

The problem with the current bold replacement logic is that if a style attribute is found on the HTML tag, then the decision to render it as bold or not relies on the style attribute even though there is no font-weight style found. This causes any bold HTML tag that has a style but no font-weight to be rendered as not bold, for example:

<b>example</b> rendered as bold
<b style="font-size:10px">example</b> not rendered as bold
<b style="font-size:10px; font-weight:bold">example</b> rendered as bold

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

We must decide whether to render it as bold or not based on the style attribute only if the style has font-weight value, otherwise, we will render it as bold. To do that, update the regex [1] [2] to:

/style="([^"]*?\bfont-weight:\s*(\d+|bold|normal)[^"]*?)"/

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

@sakluger, @rushatgabhane Eep! 4 days overdue now. Issues have feelings too...

[sakluger]

@rushatgabhane thoughts on the proposal above?

[rushatgabhane]

@bernhardoj font weight can be a number too right?

[bernhardoj]

@rushatgabhane Yes it can. The regex in the proposal can handle that (\d+)

[melvin-bot]

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

[sakluger]

@rushatgabhane friendly bump.

[rushatgabhane]

@bernhardoj's proposal LGTM #46891 (comment)

🎀 👀 🎀

[melvin-bot]

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

[bernhardoj]

The expensify-common PR is ready

cc: @rushatgabhane

[sakluger]

@rushatgabhane is OOO today - commenting to clear the Overdue label.

[melvin-bot]

@sakluger @rushatgabhane @bernhardoj @aldo-expensify 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!

[bernhardoj]

Opened the App PR, but I hold it.

[sakluger]

Commenting for Melvin since I will be OOO until Aug 30. @bernhardoj let us know if you make any progress in the meantime.

[bernhardoj]

PR is merged

[sakluger]

The automation didn't work. The PR was deployed on Aug 30, so payment was due on Sept 6.

[sakluger]

Summarizing payment on this issue:

Contributor: @bernhardoj $250, please request on Newdot
Contributor+: @rushatgabhane $250, please request on Newdot

[sakluger]

@rushatgabhane can you please propose regression test steps if you think they are needed?

[bernhardoj]

Requested in ND.

[JmillsExpensify]

$250 approved for @bernhardoj

[rushatgabhane]

no regression steps needed here. it's covered by Expensify markdown tests

[garrettmknight]

$250 approved @rushatgabhane

[melvin-bot]

@sakluger, @rushatgabhane, @bernhardoj, @aldo-expensify Eep! 4 days overdue now. Issues have feelings too...

[sakluger]

No regression steps needed and all payments approved. We're good to close 👍