[HOLD for payment 2021-09-16] Desktop - Big empty space in message fields if copy/paste the Text from MAC Note.app #4484

kavimuru · 2021-08-07T00:06:38Z

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

Issue was found when executing the PR #4363

Action Performed:

Navigate to a conversation that has some messages.
Paste the Text from MAC note.app to the composer.

Expected Result:

Only entered message should be visible in compose field if copy/paste the Text from MAC Note.app

Actual Result:

Big empty space in message compose field if copy/past the Text from MAC Note.app

Workaround:

Unknown

Platform:

Where is this issue occurring?

Desktop App

Version Number:
v1.0.83-0
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos:

Bug5182815_empty.mp4

Expensify/Expensify Issue URL:

View all open jobs on Upwork

OSBotify · 2021-08-07T00:07:20Z

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open StagingDeployCash deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

Identify the pull request that introduced this issue and revert it.
Find someone who can quickly fix the issue.
Fix the issue yourself.

MelvinBot · 2021-08-07T00:07:21Z

Triggered auto assignment to @bondydaa (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

bondydaa · 2021-08-07T22:34:19Z

I am OOO

francoisl · 2021-08-10T20:46:08Z

@parasharrajat - just checking, it looks like you were aware of this issue, correct? Was there a separate GH issue that was opened to track the bug where newlines are prepended to messages pasted into the message box?

parasharrajat · 2021-08-10T20:52:41Z

No, a new issue was not opened at that time. I was waiting for #4363 to merge as only after that, this behaviour is revealed.
So with an inspection, I come to know that the Mac Note app. usages   chars to show spaces which are also needed to be escaped and current _.unescape does not do that. I think we need a custom implementation as is not made to parse HTMLEntities and its usages do not seem sufficient any more.

About the main issue, let me quickly take a good peek at it. Only then I can tell what is causing this much space.

roryabraham · 2021-08-11T00:18:41Z

I think we should demote this from deploy blocker to regular-old 🐛

marcaaron · 2021-08-21T01:27:26Z

This hasn't been looked at in 9 days and has no assignee. Bumping to daily and making it External so we do something.

MelvinBot · 2021-08-21T01:27:34Z

Triggered auto assignment to @kadiealexander (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

kadiealexander · 2021-08-22T22:15:44Z

@parasharrajat is this related at all to #4759?

parasharrajat · 2021-08-22T22:56:49Z

@kadiealexander #4759 is needed to improve the parsing. But I was not able to reproduce the same behavior of this issue. There is space between pasted text but not before or after.

parasharrajat · 2021-08-22T23:00:50Z

After fixing #4759,
I get this where I see no space.

note-fix.mp4

kadiealexander · 2021-08-22T23:22:27Z

What version are you working with @parasharrajat? I just tested on version 1.0.85-9 (1.0.85-9) and I still see the white space above the message:

2021-08-23_11-21-10.mp4

parasharrajat · 2021-08-22T23:25:02Z

Ok. I think it's specific to chrome. Let me retest.

@kadiealexander ~~I am on v1.0.86-4 and for me, chrome is also fine.~~

EDIT: it may be note.app version. My Note.app version is 4.7

Sorry I got it now. It is reproducible now.

kadiealexander · 2021-08-22T23:44:35Z

Mine was on desktop app too, just FYI for the thread.

Thanks for helping me troubleshoot it @parasharrajat! Will post to UW now :)

parasharrajat · 2021-08-22T23:45:05Z

I have found the fix as well. Preparing the proposal now.

parasharrajat · 2021-08-23T00:02:52Z

Explanation

We faced an issue before where there were extra spaces on the Windows platform as windows surround the pasted HTML between html & body tags. Similarly, I found, in this case, the Whole valid HTML5 document is copied into the clipboard. It contains whole meta tags, head, doctype, link, and others. But Each tag contains a trailing newline for example

<html>\n
<body>\n
asdasdsadsadsad
</body>\n
</html>\n

While we trim all these extra tags but all those newlines are being prepended which creates a big empty space.

Proposal

We should only parse the body tag, if present as only the body tag contains a representational text.
we only use content present between body tag if body tag is present. Then apply all of our parsing rules.
We don't need this rule

  {
                name: 'Strip Special Tags',
                regex: /(\n|\r\n)?<\/?(html|body)(?:"[^"]*"|'[^']*'|[^'"><])*>(?![^<]*(<\/pre>|<\/code>))(\n|\r\n)?/gim,
                replacement: ''
            },

Instead in htmlToMarkdown method we first try to find the body tag. if found we replace the original text being converted with its content.

 let generatedMarkdown = htmlString;
        const body = /<(body)(?:"[^"]*"|'[^']*'|[^'"><])*>(?:\n|\r\n)?([\s\S]*?)(?:\n|\r\n)?<\/\1>(?![^<]*(<\/pre>|<\/code>))/im;
        const parseBodyTag = generatedMarkdown.match(body);
        if(parseBodyTag){
            generatedMarkdown = parseBodyTag[2];
        }

This will trim the Extra newlines. At last, I suggest we should use a mature html-to-md parser. Otherwise, we will have many bugs like this.

kadiealexander · 2021-08-23T00:12:50Z

Upwork job: https://www.upwork.com/jobs/~01078e92ead30d76a6

MelvinBot · 2021-08-23T00:13:03Z

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

kadiealexander · 2021-08-25T01:07:43Z

@johnmlee101 when you get a moment, would you mind reviewing the existing proposals here?

johnmlee101 · 2021-08-25T15:00:46Z

Love the proposal @parasharrajat .

@kadiealexander feel free to assign out the job!

kadiealexander · 2021-08-26T00:59:40Z

@parasharrajat, I've hired you in Upwork! Please go ahead with your PR 😊

johnmlee101 · 2021-08-30T19:28:12Z

Bumping back to weekly while we wait for this to close out

parasharrajat · 2021-09-07T13:21:35Z

I am on vacation. back on 8.

botify · 2021-09-08T16:26:26Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

botify · 2021-09-08T22:05:11Z

🚀 Deployed to staging by @johnmlee101 in version: 1.0.94-2 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

botify · 2021-09-09T21:00:33Z

🚀 Deployed to production by @yuwenmemon in version: 1.0.95-1 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

parasharrajat · 2021-09-17T17:43:56Z

@kadiealexander Any update for Upwork?

kadiealexander · 2021-09-19T22:38:47Z

Sorry @parasharrajat, timezones meant this fell on my weekend! I've paid this now. 💸

kavimuru added the DeployBlockerCash This issue or pull request should block deployment label Aug 7, 2021

OSBotify mentioned this issue Aug 7, 2021

Deploy Checklist: Expensify.cash 2021-08-06 #4447

Closed

89 tasks

github-actions bot added Engineering Hourly KSv2 labels Aug 7, 2021

MelvinBot assigned bondydaa Aug 7, 2021

bondydaa removed their assignment Aug 7, 2021

roryabraham removed the DeployBlockerCash This issue or pull request should block deployment label Aug 11, 2021

flodnv added Weekly KSv2 and removed Hourly KSv2 labels Aug 12, 2021

marcaaron added Daily KSv2 and removed Weekly KSv2 labels Aug 21, 2021

marcaaron added the External Added to denote the issue can be worked on by a contributor label Aug 21, 2021

MelvinBot assigned kadiealexander Aug 21, 2021

kadiealexander added the Exported label Aug 23, 2021

MelvinBot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Aug 23, 2021

MelvinBot assigned johnmlee101 Aug 23, 2021

kadiealexander assigned parasharrajat Aug 26, 2021

MelvinBot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Aug 26, 2021

parasharrajat mentioned this issue Aug 26, 2021

fix extra spaces in HTML to markdown Expensify/expensify-common#413

Merged

MelvinBot added the Overdue label Aug 30, 2021

MelvinBot removed the Overdue label Aug 30, 2021

johnmlee101 added Weekly KSv2 and removed Daily KSv2 labels Aug 30, 2021

parasharrajat mentioned this issue Sep 1, 2021

Update e-common & fix copy/paste the Text from MAC Note.app #4990

Merged

5 tasks

botify added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Sep 9, 2021

botify changed the title ~~Desktop - Big empty space in message fields if copy/paste the Text from MAC Note.app~~ [HOLD for payment 2021-09-16] Desktop - Big empty space in message fields if copy/paste the Text from MAC Note.app Sep 9, 2021

kadiealexander closed this as completed Sep 19, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[HOLD for payment 2021-09-16] Desktop - Big empty space in message fields if copy/paste the Text from MAC Note.app #4484

[HOLD for payment 2021-09-16] Desktop - Big empty space in message fields if copy/paste the Text from MAC Note.app #4484

kavimuru commented Aug 7, 2021

OSBotify commented Aug 7, 2021

MelvinBot commented Aug 7, 2021

bondydaa commented Aug 7, 2021

francoisl commented Aug 10, 2021

parasharrajat commented Aug 10, 2021 •

edited

Loading

roryabraham commented Aug 11, 2021

marcaaron commented Aug 21, 2021

MelvinBot commented Aug 21, 2021

kadiealexander commented Aug 22, 2021

parasharrajat commented Aug 22, 2021

parasharrajat commented Aug 22, 2021

kadiealexander commented Aug 22, 2021

parasharrajat commented Aug 22, 2021 •

edited

Loading

kadiealexander commented Aug 22, 2021 •

edited

Loading

parasharrajat commented Aug 22, 2021

parasharrajat commented Aug 23, 2021 •

edited

Loading

kadiealexander commented Aug 23, 2021

MelvinBot commented Aug 23, 2021

kadiealexander commented Aug 25, 2021

johnmlee101 commented Aug 25, 2021

kadiealexander commented Aug 26, 2021

johnmlee101 commented Aug 30, 2021

parasharrajat commented Sep 7, 2021

botify commented Sep 8, 2021

botify commented Sep 8, 2021

botify commented Sep 9, 2021

parasharrajat commented Sep 17, 2021

kadiealexander commented Sep 19, 2021

[HOLD for payment 2021-09-16] Desktop - Big empty space in message fields if copy/paste the Text from MAC Note.app #4484

[HOLD for payment 2021-09-16] Desktop - Big empty space in message fields if copy/paste the Text from MAC Note.app #4484

Comments

kavimuru commented Aug 7, 2021

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platform:

OSBotify commented Aug 7, 2021

MelvinBot commented Aug 7, 2021

bondydaa commented Aug 7, 2021

francoisl commented Aug 10, 2021

parasharrajat commented Aug 10, 2021 • edited Loading

roryabraham commented Aug 11, 2021

marcaaron commented Aug 21, 2021

MelvinBot commented Aug 21, 2021

kadiealexander commented Aug 22, 2021

parasharrajat commented Aug 22, 2021

parasharrajat commented Aug 22, 2021

kadiealexander commented Aug 22, 2021

parasharrajat commented Aug 22, 2021 • edited Loading

kadiealexander commented Aug 22, 2021 • edited Loading

parasharrajat commented Aug 22, 2021

parasharrajat commented Aug 23, 2021 • edited Loading

Explanation

Proposal

kadiealexander commented Aug 23, 2021

MelvinBot commented Aug 23, 2021

kadiealexander commented Aug 25, 2021

johnmlee101 commented Aug 25, 2021

kadiealexander commented Aug 26, 2021

johnmlee101 commented Aug 30, 2021

parasharrajat commented Sep 7, 2021

botify commented Sep 8, 2021

botify commented Sep 8, 2021

botify commented Sep 9, 2021

parasharrajat commented Sep 17, 2021

kadiealexander commented Sep 19, 2021

parasharrajat commented Aug 10, 2021 •

edited

Loading

parasharrajat commented Aug 22, 2021 •

edited

Loading

kadiealexander commented Aug 22, 2021 •

edited

Loading

parasharrajat commented Aug 23, 2021 •

edited

Loading