[HOLD] [$1000] When you triple tap on a message and copy it then it also copies emojis from context menu

[kavimuru]

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 the app
2. Go to any chat
3. Triple tap a message & copy it
4. You'll notice that extra emojis have been copied along with the message

Expected Result:

message is copied

Actual Result:

Extra emojis along with the message gets copied

Workaround:

unknown

Platforms:

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

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: 1.2.81-1
Reproducible in staging?: y
Reproducible in production?: new feature
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
Notes/Photos/Videos:

Recording.1668.mp4

emoji-1.1.mov

Expensify/Expensify Issue URL:
Issue reported by: @daraksha-dk
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1678358066038339

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0136ad60bd0af53825
• Upwork Job ID: 1634242845095542784
• Last Price Increase: 2023-03-10

[MelvinBot]

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

[MelvinBot]

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

[maddylewis]

i can reproduce this:

2023-03-10_11-35-27.mp4

[maddylewis]

not a dupe - https://expensify.slack.com/archives/C049HHMV9SM/p1678397732479089

[MelvinBot]

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

[maddylewis]

looking for a buddy check to confirm if this should be an internal or external issue. lmk!

[stitesExpensify]

External is fine :)

[MelvinBot]

Job added to Upwork: https://www.upwork.com/jobs/~0136ad60bd0af53825

[MelvinBot]

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

[MelvinBot]

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

[MelvinBot]

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

[Alpha-T30]

Just checked your website, and it looks working fine. Is the issue still open or not?

[MelvinBot]

📣 @Alpha-T30! 📣

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:

1. 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.
2. 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.
3. 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>

[s77rt]

Proposal

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

Triple-click selection selects text that should be unselectable.

What is the root cause of that problem?

The SelectionScraper selects any text without taking into consideration if it's supposed to be selectable. This is not necessary a bug in the SelectionScraper as that's the default behaviour of the Selection API.

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

Explicitly check the if captured text is supposed to be unselectable and if so ignore it. This can be achieved by extending the condition here and as pseudo code that would be:

if dom type is a text that should be unselectable
    set data to empty string
else
    set data as we normally do
endif

data is what we end up coping
text is considered unselectable if it's parent's style has user-select: none;

What alternative solutions did you explore? (Optional)

• Avoid the issue by moving the mini context menu to be rendered outside the hierarchy of chat messages e.g. createPortal

[allroundexperts]

Proposal

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

Triple-click selection selects text that should be unselectable.

What is the root cause of that problem?

The SelectionScraper selects any text without taking into consideration if it's supposed to be selectable. This is not necessary a bug in the SelectionScraper as that's the default behaviour of the Selection API.

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

Explicitly check the if captured text is supposed to be unselectable and if so ignore it. This can be achieved by extending the condition here and as pseudo code that would be:

if dom type is a text that should be unselectable
    set data to empty string
else
    set data as we normally do
endif

data is what we end up coping text is considered unselectable if it's parent's style has user-select: none;

What alternative solutions did you explore? (Optional)

• Avoid the issue by moving the mini context menu to be rendered outside the hierarchy of chat messages e.g. createPortal

@s77rt Is the approach you mentioned above following the same idea as listed here?

Also, do you agree that we're trying to solve the same problem as mentioned in this issue?

[s77rt]

@allroundexperts Ah yeah nice catch! Not exactly the same idea (not the same place where to apply the fix) but in general yes about the same. We can fix both issues with the same fix!

[allroundexperts]

I did some research on this when proposing for this issue. We can't / shouldn't use portals because the same behaviour exists for the reaction icons that appear in the next row (once the user has reacted on something). Of course, that is not something that should be placed outside of the component hierarchy.

Also, if dom type is a text that should be unselectable would be sort of inefficient because we'll have to rely on getComputedStyles method on each child in order to check if user-select is false. I think a better approach might be to use some sort of data-id which if present will be ignored from selection.

Lastly, we can also convert our emoji's to be images instead of text. This will solve the problem as well (Slack uses images instead of text).

[s77rt]

Thanks for the info @allroundexperts.

• I agree with you here, using portals is not a universal solution, so the main solution should be the way to go.
• getComputedStyles won't be used, I have sent you the implementation details in Slack.
• There was/is a plan to use images for emojis but that's not in the near future besides this is also not a universal solution.

The results look promising 👀

Kooha-2023-03-12-00-44-09.mp4

---

Let's pause (non-proposal) comments here not to flood the GH until things get cleared up 👍

[eVoloshchak]

I still think we should hold this for #15194, it's older and there is a lot more discussion. They're essentially the same, so they both will be fixed by a single proposal

[maddylewis]

noted! thanks for pointing that out @eVoloshchak.

do you think we're good to close out this particular issue in favor of #15194?

[eVoloshchak]

do you think we're good to close out this particular issue in favor of #15194?

Let's wait for a PR to be merged and verify it fixes our issue

[maddylewis]

changing to weekly while we wait for the other PR to merge.

[maddylewis]

still holding!

[Beamanator]

Still on hold

[maddylewis]

still on hold - switching to monthly until its off hold

[melvin-bot]

@Beamanator, @eVoloshchak, @maddylewis, this Monthly task hasn't been acted upon in 6 weeks; closing.

If you disagree, feel encouraged to reopen it -- but pick your least important issue to close instead.

[stitesExpensify]

It looks like this was actually fixed, so removing the not a priority label