[$1000] The mini context menu hides when the last comment of a new chat is copied on Safari

[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 in Safari browser
2. Click on the search icon
3. Enter any email with which you don't have any chat history
4. Click on the email to start the chat
5. Send any comments
6. Hover over the last comment
7. Click on Copy to clipboard icon

Expected Result:

The mini context menu doesn't hide

Actual Result:

The mini context menu hides

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

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.3.12.0
Reproducible in staging?: y
Reproducible in production?: y
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

Screen.Recording.2023-05-05.at.7.32.45.PM.mov

Recording.186.mp4

Expensify/Expensify Issue URL:
Issue reported by: @adeel0202
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1683297242361699

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0142123b8c491008a1
• Upwork Job ID: 1656309577984794624
• Last Price Increase: 2023-05-17

[melvin-bot]

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

[melvin-bot]

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

[kushu7]

Proposal

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

TextArea getting focused when we click copy to clipboard.

What is the root cause of that problem?

We are getting Composer as ancorNode here when we use window.getSelection() while copying last message.
isComposer is set to true.

App/src/libs/Clipboard/index.js

Lines 31 to 33 in b730027

	const selection = window.getSelection();
	const firstAnchorChild = selection.anchorNode && selection.anchorNode.firstChild;
	const isComposer = firstAnchorChild instanceof HTMLTextAreaElement;

and here we try to set back selection to Composer. so that field gets focused resulting closing of ContextMenuActions

App/src/libs/Clipboard/index.js

Lines 64 to 68 in b730027

	if (isComposer) {
	firstAnchorChild.setSelectionRange(originalSelection.start, originalSelection.end, originalSelection.direction);
	} else {
	selection.setBaseAndExtent(originalSelection.anchorNode, originalSelection.anchorOffset, originalSelection.focusNode, originalSelection.focusOffset);
	}

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

To fix this we can avoid setSelectionRange on safari desktop browser.

if (Browser.isMobileSafari()) { firstAnchorChild.setSelectionRange(originalSelection.start, originalSelection.end, originalSelection.direction); }

Screen.Recording.2023-05-10.at.1.27.14.AM.mov

What alternative solutions did you explore? (Optional)

NA

[NicMendonca]

asking clarification here: https://expensify.slack.com/archives/C049HHMV9SM/p1683728665329569?thread_ts=1683297242.361699&cid=C049HHMV9SM

[NicMendonca]

More info: https://expensify.slack.com/archives/C049HHMV9SM/p1683729730901239?thread_ts=1683297242.361699&cid=C049HHMV9SM

[NicMendonca]

I understand this now. Agree we should fix to keep it consistent across platforms

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~0142123b8c491008a1

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[kushu7]

My proposal above

[dukenv0307]

Proposal

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

The mini context menu hides when the last comment of a new chat is copied on Safari

What is the root cause of that problem?

App/src/libs/Clipboard/index.js

Lines 31 to 34 in 1d951e6

	const selection = window.getSelection();
	const firstAnchorChild = selection.anchorNode && selection.anchorNode.firstChild;
	const isComposer = firstAnchorChild instanceof HTMLTextAreaElement;
	let originalSelection = null;

We are checking if firstAnchorChild is HTMLTextAreaElement we will focus on it

App/src/libs/Clipboard/index.js

Lines 64 to 68 in 1d951e6

	if (isComposer) {
	firstAnchorChild.setSelectionRange(originalSelection.start, originalSelection.end, originalSelection.direction);
	} else {
	selection.setBaseAndExtent(originalSelection.anchorNode, originalSelection.anchorOffset, originalSelection.focusNode, originalSelection.focusOffset);
	}

As mentioned here

Window.getSelection() will either give the selected range or the cursor position. As we are running the syn operation on Safari, it is called before the composer is actually blurred thus the selection contains the cursor position.

It is the reason why when clicking Copy to Clipboard Icon on the last comment, selection.anchorNode will be the main composer and isComposer is true

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

App/src/libs/Clipboard/index.js

Line 14 in 7378626

function setHTMLSync(html, text) {

App/src/libs/Clipboard/index.js

Line 78 in 7378626

const setHtml = (html, text) => {

In both setHtml and setHTMLSync functions, we can define new param is called blurActiveinput. At the beginning of SetHTMLSync, we will call document.activeElement.blur() if blurActiveinput is true.
And pass blurActiveinput is true in

App/src/pages/home/report/ContextMenu/ContextMenuActions.js

Line 180 in 7378626

Clipboard.setHtml(content, plainText);

What alternative solutions did you explore? (Optional)

App/src/libs/Clipboard/index.js

Line 33 in 1d951e6

const isComposer = firstAnchorChild instanceof HTMLTextAreaElement;

Here, we should add one more condition to check if the user selects text by clicking Copy to Clipboard Icon. If true, isComposer will be false
const isComposer = firstAnchorChild instanceof HTMLTextAreaElement && selection.type === 'Range';
Because when clicking Copy to Clipboard Icon, selection.type === 'Caret';

Result

Just checked again and It works well

Screen-Recording-2023-05-11-at-17.11.37.mp4

[robertjchen]

Thanks for all the proposals so far, we will evaluate this week.

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[NicMendonca]

@robertjchen any update here?

[parasharrajat]

My bad, Catching up here. Give me sometime to review these.

[parasharrajat]

Thanks for the proposals. Both of proposals' root cause is incomplete. We should try to explain Why as well.

1. Anchor Node is incorrect but Why?

---

So this happens on Safari. On Safari we are using SetHTMLSync which is a synchronous method. Window.getSelection() will either give the selected range or the cursor position. As we are running the syn operation on Safari, it is called before the composer is actually blurred thus the selection contains the cursor position.
On another browser, we are using async Clipboard API which is called after the composer is blurred so the selection object no more has a cursor position.

This was the reason I renamed this function to setHtmlSync so that these bugs are easier to catch. #13396 (comment)

---

About the solutions.
@kushu7 's proposal breaks the existing functionality.
@dukenv0307 I am still thinking about your solution. Does it fit well with the original implementation #14506?

IMO, we can blur the composer before the sync clipboard set, it should resolve the issue. document.activeElement.blur() can do that.

So an alternative will be:

1. from CopyToClipboardAction pass down the argument saying blurActiveinput and then call ``document.activeElement.blur()in the SetHTMLSync when this argument istrue`.

[dukenv0307]

@parasharrajat Thanks for your explanation. I just updated proposal as your suggestion and test again. It works well
One small question, could we always call document.activeElement.blur() at the beginning of SetHTMLSync function and don't need more params

[tjferriss]

Yes, agreed. @parasharrajat @adeel0202 I have sent you both an offer in Upworks for the job.

@dukenv0307 I couldn't find you in Upworks. Can you please apply here: https://www.upwork.com/jobs/~0142123b8c491008a1

[dukenv0307]

@tjferriss applied, thank you!

[tjferriss]

Reporting bonus paid to @adeel0202

Waiting on @parasharrajat and @dukenv0307 to accept offers to be paid. Then we can close the issue.

[parasharrajat]

@tjferriss Done.

[robertjchen]

waiting on @dukenv0307

[dukenv0307]

@robertjchen I've accepted, thanks!

[melvin-bot]

@robertjchen @parasharrajat @NicMendonca @dukenv0307 this issue is now 4 weeks old and preventing us from maintaining WAQ, can you:

• Decide whether any proposals currently meet our guidelines and can be approved as-is today
• If no proposals meet that standard, please take this issue internal and treat it as one of your highest priorities
• If you have any questions, don't hesitate to start a discussion in #expensify-open-source

Thanks!

[melvin-bot]

Current assignee @parasharrajat is eligible for the Internal assigner, not assigning anyone new.

[parasharrajat]

@NicMendonca Can we please take the necessary actions on this and close it?

[robertjchen]

Thanks all, I think this can be closed.

[parasharrajat]

@robertjchen Payments are pending on this for me.

[parasharrajat]

Bump @tjferriss

[NicMendonca]

I've paid everyone out! 🎉 thank you!