[HOLD on #26490] [$500] Chat - Space below bold text increases after edit #29704

kbecciv · 2023-10-16T18:47:19Z

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

Version Number: 1.3.84.7
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: @dhanashree-sawant
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1697477034366019

Action Performed:

Open the app
Open any report
Send bold text with # and send code block with 3 backtick in one line eg: # test Hii
Edit the message and modify text inside code block and save
Observe that now space between bold text and code block increases

Expected Result:

Describe what you think should've happened

Actual Result:

Space below bold text increases on edit even when we don't add more space during edit if the text below bold text is code block

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Android: Native

Android.native.space.below.bold.text.increases.mp4

Android: mWeb Chrome

Android.chrome.space.below.bold.text.increases.mp4

iOS: Native

ios.native.extra.space.below.bold.text.mov

iOS: mWeb Safari

ios.safari.extra.space.below.bold.text.mov

MacOS: Chrome / Safari

windows.chrome.space.below.bold.text.increases.even.on.no.addition.of.text.mp4

mac.chrome.extra.space.below.bold.text.mov

Recording.5021.mp4

MacOS: Desktop

mac.desktop.extra.space.below.bold.text.mov

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~01b06817cf280a1ec0
Upwork Job ID: 1713990028027006976
Last Price Increase: 2023-11-06

Issue Owner

Current Issue Owner: @twisterdotcom

The text was updated successfully, but these errors were encountered:

melvin-bot · 2023-10-16T18:47:24Z

Triggered auto assignment to @twisterdotcom (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

melvin-bot · 2023-10-16T18:47:25Z

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

melvin-bot · 2023-10-16T18:47:26Z

Bug0 Triage Checklist (Main S/O)

This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
This bug is not a duplicate report (check E/App issues and #expensify-bugs)
- If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
This bug is reproducible using the reproduction steps in the OP. S/O
- If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
- If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
This issue is filled out as thoroughly and clearly as possible
- Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

melvin-bot · 2023-10-16T18:47:30Z

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

tjbo · 2023-10-17T00:22:54Z

Proposal

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

Unintended spaces are added after a user edits a markdown message.

What is the root cause of that problem?

There are 3 main call sites on the component side where this field gets updated:

in MessageComposer when the message is first submitted
UpdateDraft() - when editing the message on every key stroke in ReportActionItemMessageEdit.js#L264
PublishDraft() - when we finish editing the markdown message also in ReportActionItemMessageEdit.js#L317

If we disable 3 in this list everything works fine, except that 3 does a bunch of string processing for many different reasons.

This function calls Report.editReportComment() which calls handleUserDeletedLinksInHtml().

This is where the issue occurs.

The HTML is not completely cleaned, there are hidden spaces between the tags that later become line breaks when coverted to Markdown and then get passed over later into HTML on subsequent publishes.

It doesn't happen the first time you compose the message because that message is done with a different editor (1 above).

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

Modify LOC 1174 of Report.js with:.

const htmlForNewComment = parser.replace(newCommentText).replace(/>\s+</g, '><')

This will clean the spaces between the HTML tags.

An alternative fix might be to add a rule to the ExpensiMarker lib, that "cleans HTML" after parsing and conversion.

What alternative solutions did you explore? (Optional)

I explored all call sites on the components side where the text was parsed, saved or rendered.

melvin-bot · 2023-10-17T00:22:57Z

📣 @tjbo! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

Make sure you've read and understood the contributing guidelines.
Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.

Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

tjbo · 2023-10-17T00:29:44Z

Contributor details
Your Expensify account email: [email protected]
Upwork Profile Link: https://www.upwork.com/freelancers/tomoc

melvin-bot · 2023-10-17T00:29:47Z

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

eh2077 · 2023-10-17T06:42:51Z

Proposal

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

Chat - Space below bold text increases after edit.

What is the root cause of that problem?

The root cause of this issue is that when adding a comment or saving draft comment below

# heading
```
code
```

we call ExpensiMark.replace method to first apply codeFence rule, then heading rule and last newline rule. After applying codeFence and heading rule, we get intermediate result

<h1>heading</h1>
<pre>code test<br /></pre>

The line break after </h1> results in the unexpected space between heading and code block.

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

When translating markdown text into HTML, the single line break between two block tags should be removed. Currently, we only support 3 block HTML tags, (blockquote|h1|pre).

We can add a new rule before the newline rule to eliminate the unwanted line break between two block elements, like

            {
                name: 'removeNewlineBetweenTwoBlockElements',
                regex: /<\/(blockquote|h1|pre)>\r?\n<(blockquote|h1|pre)>/g,
                replacement: `</$1><$2>`,
            },

By having the new rule, we can fix the issue and pass all existing tests of Expensify-common lib. See demo

Screen.Recording.2023-10-17.at.2.25.43.PM.mov

What alternative solutions did you explore? (Optional)

N/A

0xmiros · 2023-10-17T07:02:45Z

Might have same root cause as #26490

eh2077 · 2023-10-17T09:18:20Z

Might have same root cause as #26490

@0xmiroslav They're related but their root causes are different. This issue occurs when adding a comment and the root cause is from the markdown-to-html process.

nikos-amofa · 2023-10-18T15:04:43Z

@0xmiroslav I think you're right, they are related.
The proposal #26490 (comment) will resolve the root issue.
The recording is based on the proposal.

Screen.Recording.2023-10-18.at.11.02.47.AM.mov

twisterdotcom · 2023-10-19T16:30:41Z

Nice, okay, shall we close this? And I'll pay out the bug reporter: @dhanashree-sawant?

nikos-amofa · 2023-10-19T17:04:18Z

@twisterdotcom I guess not yet. This is not on production yet.
@0xmiroslav will return with his feedback for the best approach on Monday and then we can see.

eh2077 · 2023-10-20T00:02:14Z

@twisterdotcom While I think their root cause is different and they should be fixed individually. I think we can wait C+ team to evaluate it.

cc @abdulrahuman5196

abdulrahuman5196 · 2023-10-23T12:04:32Z

~~Will review today.~~

melvin-bot · 2023-10-23T16:00:06Z

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

akamefi202 · 2023-10-24T15:27:01Z

Proposal

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

Space below bold text increases after edit.

What is the root cause of that problem?

{
    // We're removing <br /> because when </pre> and <br /> occur together, an extra line is added.
    name: 'replacepre',
    regex: /<\/pre>\s*<br\s*[/]?>/gi,
    replacement: '</pre>',
},
{
    // We're removing <br /> because when <br /> and <pre> occur together, an extra line is added.
    name: 'replacebr',
    regex: /<br\s*[/]?><pre>\s*/gi,
    replacement: ' <pre>',
},
{
    // We're removing <br /> because when <h1> and <br /> occur together, an extra line is added.
    name: 'replaceh1br',
    regex: /<\/h1><br\s*[/]?>/gi,
    replacement: '</h1>',
},

The root cause is these 3 replace regex rules in ExpensiMark class of expensify-common repo.
replacebr rule replaces the linebreak with a space if the linkbreak exists before <pre> tag.
And it renders the linebreak after the space while rendering in the report page.

This doesn't cause problem if previous line is a normal text.
But if the previous line is </h1> tag, we see an extra linebreak because </h1> tag renders a linebreak after it too.

For example, like below.

# test\n```code``` => <h1>test</h1>\n<pre>code</pre> => <h1>test</h1> <pre>code</pre> => TEST\n \nCODE

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

{
    // We're removing <br /> because when <h1> and <br /> occur together, an extra line is added.
    name: 'replaceh1br',
    regex: /<\/h1><br\s*[/]?>/gi,
    replacement: '</h1>',
},
{
    // We're removing <br /> because when </pre> and <br /> occur together, an extra line is added.
    name: 'replacepre',
    regex: /<\/pre>\s*<br\s*[/]?>/gi,
    replacement: '</pre>',
},
{
    // We're removing <br /> because when <br /> and <pre> occur together, an extra line is added.
    name: 'replacebr',
    regex: /<br\s*[/]?><pre>\s*/gi,
    replacement: ' <pre>',
},

To fix this issue, we need to prioritize replaceh1br rule before other two rules.
So replacebr rule won't add unnecessary space and we won't see an extra line too.

For example, like below.

# test\n```code``` => <h1>test</h1>\n<pre>code</pre> => <h1>test</h1><pre>code</pre> => TEST\nCODE

What alternative solutions did you explore? (Optional)

N/A

twisterdotcom · 2023-11-09T11:21:56Z

What does TAL mean?

melvin-bot · 2023-11-09T13:50:19Z

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

abdulrahuman5196 · 2023-11-13T08:49:42Z

Hi, Sorry TAL means Taking a look.
Will review again today, just back from weekend

melvin-bot · 2023-11-13T14:20:56Z

@twisterdotcom @abdulrahuman5196 this issue is now 4 weeks old and preventing us from maintaining WAQ, can you:

Decide whether any proposals currently meet our guidelines and can be approved as-is today
If no proposals meet that standard, please take this issue internal and treat it as one of your highest priorities
If you have any questions, don't hesitate to start a discussion in #expensify-open-source

Thanks!

melvin-bot · 2023-11-13T14:21:04Z

Current assignee @abdulrahuman5196 is eligible for the Internal assigner, not assigning anyone new.

melvin-bot · 2023-11-13T14:50:46Z

@twisterdotcom @abdulrahuman5196 this issue is now 4 weeks old and preventing us from maintaining WAQ. This should now be your highest priority. Please post below what your plan is to get a PR in review ASAP. Thanks!

melvin-bot · 2023-11-13T15:20:36Z

@twisterdotcom @abdulrahuman5196 this issue is now 4 weeks old and preventing us from maintaining WAQ. This should now be your highest priority. Please post below what your plan is to get a PR in review ASAP. Thanks!

twisterdotcom · 2023-11-16T22:01:45Z

@abdulrahuman5196 are you able to get somebody assigned here or can I reassign to a new C+? Will reassign on Monday if you're not around by then.

twisterdotcom · 2023-11-16T22:02:10Z

Not overdue, just waiting on @abdulrahuman5196

abdulrahuman5196 · 2023-11-17T11:12:31Z

Sure @twisterdotcom . I will cover this over the weekend. If not do go ahead and re-assign on monday.

abdulrahuman5196 · 2023-11-17T11:54:08Z

@abdulrahuman5196 Can you also review #26490 (comment) ? I think they are related and the proposal can resolve 29704 too

With this change, I am unable to reproduce the issue. The proposal is already approved as part of #26490.

Can we put this issue on hold for #26490? @twisterdotcom

Screen.Recording.2023-11-17.at.5.21.31.PM.mov

melvin-bot · 2023-11-20T12:58:11Z

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

twisterdotcom · 2023-11-20T22:06:19Z

Done

twisterdotcom · 2023-11-30T13:14:35Z

Looks like we'll pay out #26490 today. Is this resolved @abdulrahuman5196? Can we close?

abdulrahuman5196 · 2023-12-04T19:18:10Z

Let me check in my morning

abdulrahuman5196 · 2023-12-06T18:14:45Z

@twisterdotcom I don't see this issue on latest main. We can close this issue.

kbecciv added External Added to denote the issue can be worked on by a contributor Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Oct 16, 2023

melvin-bot bot assigned twisterdotcom Oct 16, 2023

melvin-bot bot changed the title ~~Chat - Space below bold text increases after edit~~ [$500] Chat - Space below bold text increases after edit Oct 16, 2023

melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Oct 16, 2023

melvin-bot bot assigned abdulrahuman5196 Oct 16, 2023

eh2077 mentioned this issue Oct 17, 2023

[HOLD for payment 2023-11-30] [$500] Web - Different markdown when copied after sending. #26490

Closed

6 tasks

melvin-bot bot added the Overdue label Oct 19, 2023

melvin-bot bot removed the Overdue label Oct 19, 2023

melvin-bot bot added the Overdue label Oct 23, 2023

melvin-bot bot removed the Overdue label Oct 23, 2023

melvin-bot bot removed the Overdue label Nov 13, 2023

melvin-bot bot added Internal Requires API changes or must be handled by Expensify staff and removed External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors labels Nov 13, 2023

melvin-bot bot added the Overdue label Nov 16, 2023

melvin-bot bot removed the Overdue label Nov 16, 2023

melvin-bot bot added the Overdue label Nov 20, 2023

twisterdotcom changed the title ~~[$500] Chat - Space below bold text increases after edit~~ [HOLD on #26490] [$500] Chat - Space below bold text increases after edit Nov 20, 2023

twisterdotcom added Weekly KSv2 and removed Daily KSv2 labels Nov 20, 2023

melvin-bot bot removed the Overdue label Nov 20, 2023

melvin-bot bot added the Overdue label Nov 29, 2023

melvin-bot bot removed the Overdue label Nov 30, 2023

twisterdotcom closed this as completed Dec 6, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[HOLD on #26490] [$500] Chat - Space below bold text increases after edit #29704

[HOLD on #26490] [$500] Chat - Space below bold text increases after edit #29704

kbecciv commented Oct 16, 2023 •

edited by twisterdotcom

Loading

melvin-bot bot commented Oct 16, 2023

melvin-bot bot commented Oct 16, 2023

melvin-bot bot commented Oct 16, 2023

melvin-bot bot commented Oct 16, 2023

tjbo commented Oct 17, 2023 •

edited

Loading

melvin-bot bot commented Oct 17, 2023

tjbo commented Oct 17, 2023

melvin-bot bot commented Oct 17, 2023

eh2077 commented Oct 17, 2023 •

edited

Loading

0xmiros commented Oct 17, 2023

eh2077 commented Oct 17, 2023 •

edited

Loading

nikos-amofa commented Oct 18, 2023

twisterdotcom commented Oct 19, 2023

nikos-amofa commented Oct 19, 2023

eh2077 commented Oct 20, 2023

abdulrahuman5196 commented Oct 23, 2023 •

edited

Loading

melvin-bot bot commented Oct 23, 2023

akamefi202 commented Oct 24, 2023

twisterdotcom commented Nov 9, 2023

melvin-bot bot commented Nov 9, 2023

abdulrahuman5196 commented Nov 13, 2023 •

edited

Loading

melvin-bot bot commented Nov 13, 2023

melvin-bot bot commented Nov 13, 2023

melvin-bot bot commented Nov 13, 2023

melvin-bot bot commented Nov 13, 2023

twisterdotcom commented Nov 16, 2023

twisterdotcom commented Nov 16, 2023

abdulrahuman5196 commented Nov 17, 2023

abdulrahuman5196 commented Nov 17, 2023

melvin-bot bot commented Nov 20, 2023

twisterdotcom commented Nov 20, 2023

twisterdotcom commented Nov 30, 2023

abdulrahuman5196 commented Dec 4, 2023

abdulrahuman5196 commented Dec 6, 2023

[HOLD on #26490] [$500] Chat - Space below bold text increases after edit #29704

[HOLD on #26490] [$500] Chat - Space below bold text increases after edit #29704

Comments

kbecciv commented Oct 16, 2023 • edited by twisterdotcom Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

melvin-bot bot commented Oct 16, 2023

melvin-bot bot commented Oct 16, 2023

melvin-bot bot commented Oct 16, 2023

Bug0 Triage Checklist (Main S/O)

melvin-bot bot commented Oct 16, 2023

tjbo commented Oct 17, 2023 • edited Loading

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

melvin-bot bot commented Oct 17, 2023

tjbo commented Oct 17, 2023

melvin-bot bot commented Oct 17, 2023

eh2077 commented Oct 17, 2023 • edited Loading

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

0xmiros commented Oct 17, 2023

eh2077 commented Oct 17, 2023 • edited Loading

nikos-amofa commented Oct 18, 2023

twisterdotcom commented Oct 19, 2023

nikos-amofa commented Oct 19, 2023

eh2077 commented Oct 20, 2023

abdulrahuman5196 commented Oct 23, 2023 • edited Loading

melvin-bot bot commented Oct 23, 2023

akamefi202 commented Oct 24, 2023

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

twisterdotcom commented Nov 9, 2023

melvin-bot bot commented Nov 9, 2023

abdulrahuman5196 commented Nov 13, 2023 • edited Loading

melvin-bot bot commented Nov 13, 2023

melvin-bot bot commented Nov 13, 2023

melvin-bot bot commented Nov 13, 2023

melvin-bot bot commented Nov 13, 2023

twisterdotcom commented Nov 16, 2023

twisterdotcom commented Nov 16, 2023

abdulrahuman5196 commented Nov 17, 2023

abdulrahuman5196 commented Nov 17, 2023

melvin-bot bot commented Nov 20, 2023

twisterdotcom commented Nov 20, 2023

twisterdotcom commented Nov 30, 2023

abdulrahuman5196 commented Dec 4, 2023

abdulrahuman5196 commented Dec 6, 2023

kbecciv commented Oct 16, 2023 •

edited by twisterdotcom

Loading

tjbo commented Oct 17, 2023 •

edited

Loading

eh2077 commented Oct 17, 2023 •

edited

Loading

eh2077 commented Oct 17, 2023 •

edited

Loading

abdulrahuman5196 commented Oct 23, 2023 •

edited

Loading

abdulrahuman5196 commented Nov 13, 2023 •

edited

Loading