[Paid: Final Steps] [$1000] Web - Chat - When copying part of a multiline message, the entire message is copied

[kbecciv]

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 ND on the mac/Safari
2. Send multiline message
3. Select one word of the msg, click on the selected text via right button: context menu is opened
4. Click on the Copy icon
5. Paste the text

Expected Result:

Only selected text should be copied

Actual Result:

The whole message has been copied at the Safari

Workaround:

Unknown

Platforms:

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

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

Version Number: 1.3.4.0

Reproducible in staging?: Yes

Reproducible in production?: Yes

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: Any additional supporting documentation

Bug6028917_Screen_Recording_2023-04-23_at_00.24.37.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01075913f2cc61e719
• Upwork Job ID: 1650506933540429824
• Last Price Increase: 2023-05-08

[MelvinBot]

Triggered auto assignment to @conorpendergrast (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

[tienifr]

Proposal

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

In Safari, when we try to select a part of a multiline message, then right click the selected text and press Copy to clipboard, it copies the whole message instead of the selected text

What is the root cause of that problem?

Let's look at the flow of the executeSecondaryInteractionOnContextMenu() function, which would be triggered when we right click a message. Basically, the current logic is blurring all active components -> showing the popover menu

App/src/components/PressableWithSecondaryInteraction/index.js

Lines 38 to 48 in 931e15f

	/**
	* This component prevents the tapped element from capturing focus.
	* We need to blur this element when clicked as it opens modal that implements focus-trapping.
	* When the modal is closed it focuses back to the last active element.
	* Therefore it shifts the element to bring it back to focus.
	* https://github.com/Expensify/App/issues/14148
	*/
	if (this.props.withoutFocusOnSecondaryInteraction && this.pressableRef) {
	this.pressableRef.blur();
	}
	this.props.onSecondaryInteraction(e);

The inconsistency lies in this piece of code. The blur() function has different behavior on Safari and Chrome. On Safari, the blur() function would de-select the user-selected text, causing the window.getSelection() function to return null, while it would not on Chrome. When the line this.pressableRef.blur() is triggered, the Safari browser would de-select the selected text, affecting the logic that was used to get the current selected text. Here, because the text was de-selected, and we are opening the popover after blurring, the window.getSelection() function would return a null component. The copied text now would be the whole message, which is similar to the behavior when we right click on the message without selecting anything.

In short, after blurring the text, the selection on Safari is cleared, while it is not on Chrome. Therefore, when the showPopover() function is triggered afterwards, the selected component is null on Safari, which make the app understand that the whole long message should be copied.

I believe this is a regression from #15375, where the blurring logic was added. A very similar issue of the selection API inconsistency after a component is blurred on Chrome and Safari (in another repo) can be found here. A possibility is that there is a difference between the way Safari and Chrome handle the blurring event.

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

Changing the order from blurring -> showPopover to showPopover -> blurring will solve this issue

-        if (this.props.withoutFocusOnSecondaryInteraction && this.pressableRef) {
-           this.pressableRef.blur();
-        }
-        this.props.onSecondaryInteraction(e);
+        this.props.onSecondaryInteraction(e);
+        if (this.props.withoutFocusOnSecondaryInteraction && this.pressableRef) {
+           this.pressableRef.blur();
+        }
      

I think this is a reasonable change, since

1. It's minimal. The changes is small.
2. It's logical. The onSecondaryInteraction() function is now using the user-selected text before blurring the current element, so we don't have to worry about any unexpected behavior that affect the user-selected text.
3. It will not re-introduce the bug that was fixed in Disable the focus trap for report page context menu #15375.

What alternative solutions did you explore? (Optional)

Alternatively, we could add a setTimeout around the blur() function. IMO this is a workaround and not a good way to do it, but given the blur() function has some kind of a small delay, and we want to preserve the selected component when the onSecondaryInteraction() is called, this is worth mentioning.

+            setTimeout(() => {
                this.pressableRef.blur();
+            }, 0)

Records

Working well after the fix

Screen.Recording.2023-04-24.at.15.30.07.mov

[conorpendergrast]

This isn't a bug: the context menu applies to all parts of the message, not individual components within the message.
eg: you can't select part of the message and delete just part of the message, or mark just part of the message as unread.

Closing as it's not a bug!

[tienifr]

@conorpendergrast
I don't agree with it. Even if it's not a bug, the behavior is inconsistent in different browsers. On Chrome or Desktop, selecting a part of the multiline text then right click -> copy would copy the selected only. Meanwhile in Safari, it would copy the whole text.

[conorpendergrast]

Oh that's very weird - I didn't spot that this was inconsistent in the OP (there's no platforms selected)

[conorpendergrast]

Reopening as this is an inconsistency that we should resolve either as copying the full message, or copying the selected part of the message

[conorpendergrast]

Interesting, I was doubly wrong! When you select text and then click Copy to Clipboard, that should copy only the selected text #2665

[conorpendergrast]

Updated the OP to include the platform!