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

[$500] BUG: Copy / Pasting a last message in the compose box, returning the copied text and some random text reported by @mollfpr #11921

Closed
kavimuru opened this issue Oct 17, 2022 · 39 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering 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

Comments

@kavimuru
Copy link

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


Action Performed:

  1. Open a chat
  2. Go to the last chat of a thread
  3. Double/triple tap until all the text is highlighted and copy with CMD+C / CTRL+C
  4. Paste the text to the composer input or notepad

Expected Result:

The pasted text is the text that you just copy

Actual Result:

The pasted text is the text that you just copy and some random text and error message appears

Workaround:

unknown

Platform:

Where is this issue occurring?

  • Web
  • Desktop App

Version Number: 1.2.16-2
Reproducible in staging?: y
Reproducible in production?: y
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos:

https://user-images.githubusercontent.com/43996225/196245398-44c8e0e9-d06d-4dda-ac78-1c2a51465e83.mp4
https://user-images.githubusercontent.com/43996225/196245405-3f3d68f8-2a5f-452f-8dfe-ac9057ccec10.mov

Expensify/Expensify Issue URL:
Issue reported by: @mollfpr
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1666000158578689

View all open jobs on GitHub

@kavimuru kavimuru added AutoAssignerTriage Auto assign issues for triage to an available triage team member Daily KSv2 labels Oct 17, 2022
@melvin-bot
Copy link

melvin-bot bot commented Oct 17, 2022

Triggered auto assignment to @alexpensify (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

@melvin-bot melvin-bot bot removed the AutoAssignerTriage Auto assign issues for triage to an available triage team member label Oct 17, 2022
@alexpensify
Copy link
Contributor

@kavimuru - I'm unable to replicate this issue. Can you please try again? Thanks!

@mollfpr
Copy link
Contributor

mollfpr commented Oct 18, 2022

@alexpensify Could you try to send a message and double/triple tap on empty space chat to highlight the text then copy with cmd+c or ctrl+c?

Screen.Recording.2022-10-18.at.09.52.52.mov

@alexpensify alexpensify removed their assignment Oct 18, 2022
@alexpensify
Copy link
Contributor

I'm still unable to replicate it. I followed the new steps to select the white space and input cmd+c and was successful to copy. Assigning it to Engineering for another review.

@melvin-bot
Copy link

melvin-bot bot commented Oct 18, 2022

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

@alexpensify
Copy link
Contributor

Here are screenshots from my test via the web app:

image

image

@jasperhuangg
Copy link
Contributor

jasperhuangg commented Oct 18, 2022

Can repro on both staging and production; you need to triple-click to highlight the entire message. Going to investigate

@jasperhuangg jasperhuangg added the External Added to denote the issue can be worked on by a contributor label Oct 18, 2022
@melvin-bot
Copy link

melvin-bot bot commented Oct 18, 2022

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

@melvin-bot
Copy link

melvin-bot bot commented Oct 18, 2022

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Oct 18, 2022
@melvin-bot
Copy link

melvin-bot bot commented Oct 18, 2022

Current assignee @jasperhuangg is eligible for the External assigner, not assigning anyone new.

@melvin-bot melvin-bot bot changed the title BUG: Copy / Pasting a last message in the compose box, returning the copied text and some random text reported by @mollfpr [$250] BUG: Copy / Pasting a last message in the compose box, returning the copied text and some random text reported by @mollfpr Oct 18, 2022
@abekkala
Copy link
Contributor

@NikkiWines Can I get your opinion if your GH here would also fix this one?

@abekkala
Copy link
Contributor

confirmed this one is unique and won't fix with that one

@puneetlath puneetlath added the Bug Something is broken. Auto assigns a BugZero manager. label Oct 19, 2022
@abekkala abekkala added Weekly KSv2 and removed Daily KSv2 labels Oct 20, 2022
@osama256
Copy link
Contributor

PROPOSAL

Replacing copySelectionToClipboard method in src/components/CopySelectionHelper.js with this method :

  import React from 'react';
  import ExpensiMark from 'expensify-common/lib/ExpensiMark';
+ import Str from 'expensify-common/lib/str';

....


    copySelectionToClipboard() {
        const selection = SelectionScraper.getCurrentSelection();
        if (!selection) {
            return;
        }
        const parser = new ExpensiMark();
        if (!Clipboard.canSetHtml()) {
            Clipboard.setString(parser.htmlToMarkdown(selection));
            return;
        }
        if (selection.match(/It's.*Chats/)) {
            const replacement = parser.htmlToMarkdown(selection.split(/It's.*Chats/)[0]);
            Clipboard.setHtml(Str.htmlEncode(replacement), parser.htmlToText(selection).split(/It's.*Chats/)[0]);
            return;
        }
        if (selection.includes('Chats') && selection.includes('Concierge')) {
            const replacement = parser.htmlToMarkdown(selection.split(/Chats/)[0]);
            Clipboard.setHtml(Str.htmlEncode(replacement), parser.htmlToText(selection).split(/Chats/)[0]);
            return;
        }
        Clipboard.setHtml(selection, parser.htmlToText(selection));
    }
webb-2022-10-24_09.16.52.mp4

@melvin-bot melvin-bot bot removed the Overdue label Nov 3, 2022
@trjExpensify
Copy link
Contributor

It can be reliably reproduced on my side on v1.2.23-7. Got a vid Jasper? Are you tapping three times?

copy junk

@trjExpensify trjExpensify reopened this Nov 4, 2022
@melvin-bot melvin-bot bot added the Overdue label Nov 7, 2022
@melvin-bot
Copy link

melvin-bot bot commented Nov 7, 2022

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

@abekkala
Copy link
Contributor

abekkala commented Nov 7, 2022

@jasperhuangg can you try to replicate again? it seems that @trjExpensify was able to replicate

@melvin-bot melvin-bot bot removed the Overdue label Nov 7, 2022
@jasperhuangg
Copy link
Contributor

jasperhuangg commented Nov 7, 2022

@abekkala @trjExpensify sorry missed these notifications, ah I realized I was triple tapping a message with links in it, it only selects everything in the message up to (and not including) the link. The bug won't happen if you do this:

Untitled.mov

To reproduce this you have to use with messages without links (i.e. where triple tapping highlights the entire message). Thanks for reopening and keeping it going!

@jasperhuangg
Copy link
Contributor

^TLDR: we're still looking for proposals for this one!

@trjExpensify
Copy link
Contributor

Yeah, the URL bug is here: #11735

@abekkala abekkala changed the title [$250] BUG: Copy / Pasting a last message in the compose box, returning the copied text and some random text reported by @mollfpr [$500] BUG: Copy / Pasting a last message in the compose box, returning the copied text and some random text reported by @mollfpr Nov 8, 2022
@abekkala
Copy link
Contributor

abekkala commented Nov 8, 2022

@wildan-m
Copy link
Contributor

Proposal

Seems this issue has the same fix with #12028
Please review it:
#12028 (comment)

@abekkala
Copy link
Contributor

@jasperhuangg @sobitneupane
If you approve of the above proposal just give me a quick thumbs ups and I'll start the hiring steps in Upwork

@trjExpensify
Copy link
Contributor

If the fix for this is going to be coming in the same PR as the one for #12028, then we should combine C+ reviewers and issues.

@sobitneupane
Copy link
Contributor

@trjExpensify It is likely that the same PR will solve both the issues.

@melvin-bot
Copy link

melvin-bot bot commented Nov 14, 2022

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

@melvin-bot melvin-bot bot added the Overdue label Nov 14, 2022
@sobitneupane
Copy link
Contributor

@trjExpensify Should I also review the proposal #12028 (comment) for this issue?

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Nov 14, 2022
@trjExpensify
Copy link
Contributor

@sobitneupane it looks like @thesahindia has reviewed that proposal and given it the ✅. Though, @thesahindia can you confirm it fixes this issue as well ahead of us closing it?

@thesahindia
Copy link
Member

Though, @thesahindia can you confirm it fixes this issue as well ahead of us closing it?

Correct it will be fixed at #12028

@trjExpensify
Copy link
Contributor

Cool! So @mollfpr, for reporting this issue, I've sent you a combined offer on the Upwork job attached to #12028.

@abekkala you can go ahead and close the Upwork job and then this issue. Thanks everyone! 🎉

@abekkala
Copy link
Contributor

Closed Upwork job.

@melvin-bot
Copy link

melvin-bot bot commented Nov 16, 2022

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

  • @abekkala A regression test has been added or updated so that the same bug will not reach production again. Link to the GH issue for creating the test here:
  • @sobitneupane @jasperhuangg The PR that introduced the bug has been identified. Link to the PR:
  • @abekkala The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
  • @abekkala A discussion in #contributor-plus has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
  • @abekkala Payment has been made to the issue reporter (if applicable)
  • @abekkala Payment has been made to the contributor that fixed the issue (if applicable)
  • @abekkala Payment has been made to the contributor+ that helped on the issue (if applicable)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering 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
Projects
None yet
Development

No branches or pull requests