Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Hold for Payment 10 August] Web - Chat - When Pasting the copied text in compose field there are 2 line breaks before and after the text #4066

Closed
kavimuru opened this issue Jul 15, 2021 · 29 comments · Fixed by Expensify/expensify-common#399
Assignees
Labels
Engineering External Added to denote the issue can be worked on by a contributor Improvement Item broken or needs improvement. Weekly KSv2

Comments

@kavimuru
Copy link

kavimuru commented Jul 15, 2021

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel! https://www.upwork.com/jobs/~0112eb53bb4c90676c


Action Performed:

  1. Open e.cash in Web
  2. Navigate to a conversation that has some messages
  3. In a message that only contains text, use the copy function. For web, hover over the message and a menu should appear
  4. Paste the message in a text box

Expected Result:

The message must be shown in the text box without issues

Actual Result:

The size of the compose field there are 2 line breaks before and after the copied text

Workaround:

Unknown

Platform:

Where is this issue occurring?

Web ✔️
iOS
Android
Desktop App
Mobile Web

Version Number:
1.0.78-0
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation

Bug5152245_Issuecopy_Paste.mp4

Bug5152245_image__3_

Expensify/Expensify Issue URL:

View all open jobs on Upwork

@kavimuru kavimuru added the DeployBlockerCash This issue or pull request should block deployment label Jul 15, 2021
@OSBotify
Copy link
Contributor

👋 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:

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

@MelvinBot
Copy link

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

@parasharrajat
Copy link
Member

@kavimuru Not able to reproduce, Could you please try pasting the same copied content here http://madebyevan.com/clipboard-test/

And share the output with us. It will help us debug.

@Julesssss
Copy link
Contributor

Hi @kavimuru, I'm also not able to reproduce on Chrome, E.cash version v1.0.78-1. I tried a couple of different languages.

For now I'm removing the labels.

@Julesssss Julesssss added Monthly KSv2 Improvement Item broken or needs improvement. and removed DeployBlockerCash This issue or pull request should block deployment Hourly KSv2 labels Jul 15, 2021
@Julesssss Julesssss removed their assignment Jul 15, 2021
@kavimuru
Copy link
Author

Hi @parasharrajat , I'm still able to reproduce with version v1.0.78-1. Attaching video which included reproducible steps and output from http://madebyevan.com/clipboard-test/

Reproducible.mp4

Screenshot (370)
Screenshot (369)

@Julesssss
Copy link
Contributor

@kavimuru here's what I'm seeing with the same message:

Screenshot 2021-07-15 at 12 57 59

Clearly this is still occurring for you so lets keep the issue open, but there's no need to block release for this.

@isagoico
Copy link

isagoico commented Jul 15, 2021

mmm I'm also able to reproduce this on my side Web version 1.0.78-1 (Windows / Chrome)
image

@Julesssss
Copy link
Contributor

Hmm, I wonder what the differences between our environments is? 🤔

To rule everything out, here is what I'm using:

MacOS Big Sur: `11.4`
Chrome version:  `91.0.4472.114` ✅ 
Safari version: `14.1.1 (16611.2.7.1.4)` ✅ 
Expensify.cash version: `v1.0.78-1`

I wonder if OS system language could be to blame? Or perhaps specific user accounts?

@isagoico
Copy link

I'm more inclined to the OS system part since it's the only difference in our environments and I was able to reproduce in different testing accounts. 🤔

@isagoico isagoico changed the title Web - Chat - When Pasting the copied text in compose field the size of the box expands Web - Chat - When Pasting the copied text in compose field there are 2 line breaks before and after the text Jul 16, 2021
@isagoico
Copy link

Issue reproducible during KI retests

@isagoico isagoico mentioned this issue Jul 23, 2021
5 tasks
@isagoico
Copy link

Issue reproducible during KI retests

@Julesssss
Copy link
Contributor

Just retested and I'm still not seeing the error 😕

Sharing this in Slack to get additional testers.

@laurenreidexpensify
Copy link
Contributor

I've tried this on prod v1.0.79-4 in Chrome 91.0.4472.101 and can't replicate at all 🤔

@conorpendergrast
Copy link
Contributor

I've also just tried in v1.0.79-4 in Chrome 91.0.4472.114 and can't replicate

@twisterdotcom
Copy link
Contributor

v1.0.79-4 on Firefox, also can't replicate.

@isagoico
Copy link

Are you testing on Mac or Windows? I'm also unable to replicate on my MacOS, but in Windows it has been reproducible since this was reported.

@parasharrajat
Copy link
Member

I'll test it on Windows. I am sure that it exists on Windows. I'll try to find the cause and fix it.

@twisterdotcom
Copy link
Contributor

All Mac here yes. 🤔

@parasharrajat
Copy link
Member

parasharrajat commented Jul 26, 2021

I confirm this is reproducible on Windows Platform (tested Edge & Chrome). Finding solution now.

@parasharrajat
Copy link
Member

parasharrajat commented Jul 26, 2021

I have found the core issue and Solution as well. May be we can triage this issue and I can apply the fix. Or If we are no triaging then you can assign this to me and I will apply the fix.

Exaplanation:

  1. On window platform, copy works a little differently. Its appends and prepends html & body tags to the html fragments. And each tag following a newline. HTML tags are successfully trimmed by Str.stripHtml in HTML to MD conversion. but not the newlines between tags.
<html>
<body>
<!--StartFragment--><span style="color: rgb(0, 0, 0); font-family: &quot;Times New Roman&quot;; font-size: medium; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; white-space: pre-wrap; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration-thickness: initial; text-decoration-style: initial; text-decoration-color: initial; display: inline !important; float: none;">test message</span><!--EndFragment-->
</body>
</html>

Fix:

  1. We have to add another rule where we specify these special tags which need to be trimmed with the ending newline. For now HTML and body tag. We can not trim the converted string as it is possible that there were <br> tags for the converted newlines.

  2. I think it would be good to match the ending newline chars with each tag. Thus updating the regex of each tag to match this.

Unix tends to use \n as a line separator; Windows tends to use \r\n as a line separator and Macs (up to OS 9) used to use \r as the line separator. (Mac OS X is Unix-y, so uses \n instead; there may be some compatibility situations where \r is used instead though.)

@Julesssss
Copy link
Contributor

Would you like to take this issue @parasharrajat?

@parasharrajat
Copy link
Member

Yeah Sure. My pleasure. I am already tackling other paste-related issues.

@Julesssss Julesssss added the External Added to denote the issue can be worked on by a contributor label Jul 26, 2021
@MelvinBot
Copy link

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

@laurenreidexpensify
Copy link
Contributor

@Julesssss just a reminder we need to have a Contributor Manager and Contributor Manager Engineer assigned the issue along with the Contributor, so that we can ensure the job is posted to Upwork + the contributor is hired and paid, and that the CME is able to review the PR.

@laurenreidexpensify
Copy link
Contributor

Job posted on Upwork https://www.upwork.com/jobs/~0112eb53bb4c90676c

@laurenreidexpensify laurenreidexpensify changed the title Web - Chat - When Pasting the copied text in compose field there are 2 line breaks before and after the text [Hold for Payment 4 August] Web - Chat - When Pasting the copied text in compose field there are 2 line breaks before and after the text Jul 28, 2021
@laurenreidexpensify laurenreidexpensify added Weekly KSv2 and removed Daily KSv2 labels Jul 28, 2021
@isagoico
Copy link

isagoico commented Aug 2, 2021

Issue reproducible during KI retests

@parasharrajat
Copy link
Member

parasharrajat commented Aug 2, 2021

Oh sorry I missed to update the lib version on NewDot

@parasharrajat parasharrajat mentioned this issue Aug 2, 2021
5 tasks
@laurenreidexpensify laurenreidexpensify changed the title [Hold for Payment 4 August] Web - Chat - When Pasting the copied text in compose field there are 2 line breaks before and after the text [Hold for Payment 10 August] Web - Chat - When Pasting the copied text in compose field there are 2 line breaks before and after the text Aug 3, 2021
@isagoico
Copy link

isagoico commented Aug 9, 2021

Issue not reproducible during KI retests (Fixed 🎉)

@laurenreidexpensify
Copy link
Contributor

Paid @parasharrajat 👍🏽

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Engineering External Added to denote the issue can be worked on by a contributor Improvement Item broken or needs improvement. Weekly KSv2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants