[$1000] Web - On copy paste on workspace invite message page, app pastes text on compose box

[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 app
2. Open settings
3. Open workspaces
4. Open any workspace
5. Open members
6. Reload the page
7. Remove any user
8. Click on invite and select any issue
9. Click Next
10. On invite message page, copy the current text and try to paste same text after it, observe that text is pasted in compose box behind

Expected Result:

App should paste text in invite message field if we copy and paste text while invite message field is in focus

Actual Result:

App pastes the text in compose box when we reload on members page, remove 1 member and invite any other and reach invite message paste

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.3.42-18
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

invite.message.paste.in.compose.mp4

Recording.3757.mp4

Expensify/Expensify Issue URL:
Issue reported by: @dhanashree-sawant
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1689703579147449

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0105c6892023aa08e8
• Upwork Job ID: 1683525862926856192
2023-07-24
• Automatic offers:
• dhanashree-sawant | Reporter | 25841637

[melvin-bot]

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

[samh-nl]

Proposal

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

Copy-paste on the workspace invite page leads to the text being pasted in the compose box.

What is the root cause of that problem?

When pasting to the composer, it checks checkComposerVisibility to ensure that the composer is visible:

App/src/pages/home/report/ReportActionCompose.js

Lines 703 to 706 in 0fcfe75

	checkComposerVisibility() {
	const isComposerCoveredUp = EmojiPickerActions.isEmojiPickerVisible() || this.state.isMenuVisible || this.props.modal.isVisible;
	return !isComposerCoveredUp;
	}

Normally, when a modal is open, this function blocks due to the this.props.modal.isVisible inside the if-statement.

However, when deleting a member in a workspace, you are presented with a confirmation modal asking you whether you're sure. When this modal closes, this.props.modal.isVisible becomes false, therefore checkComposerVisibility returns true. This leads to the incorrect pasting behavior.

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

When closing a modal, we should not set modal.isVisible to false if there are other modals opened.

What alternative solutions did you explore? (Optional)

Option 1. For ensuring that the composer is visible, we can check that the active route is NAVIGATORS.CENTRAL_PANE_NAVIGATOR, instead of checking whether a modal is active or not.

Option 2. When opening a modal, we determine the value of modal.isVisible. Subsequently, when closing the modal, we only set it to false if it was false initially. If a modal was already opened, we will not change its value.

Option 3. We could set the shouldSetModalVisibility prop to false in the ConfirmModal when deleting the member, however this may not be an elegant solution to the problem as this issue may also be present in other modals. We would need to identify where else a similar problem may arise.

[yh-0218]

Proposal

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

Copy-paste on the workspace invite page leads to the text being pasted in the compose box.

What is the root cause of that problem?

After reload, composer gets paste event and check value of checkComposerVisibility function.

App/src/pages/home/report/ReportActionCompose.js

Lines 703 to 706 in 0fcfe75

	checkComposerVisibility() {
	const isComposerCoveredUp = EmojiPickerActions.isEmojiPickerVisible() || this.state.isMenuVisible || this.props.modal.isVisible;
	return !isComposerCoveredUp;
	}

when closing remove member Dialog, this.propos.modal.isVisible becomes to false.
so composer gets the incorrect paste.

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

When closing Modal, we should not set ONYXKEYS.MODAL value.

App/src/pages/workspace/WorkspaceMembersPage.js

Lines 422 to 431 in fa473e9

	<ConfirmModal
	danger
	title={props.translate('workspace.people.removeMembersTitle')}
	isVisible={removeMembersConfirmModalVisible}
	onConfirm={removeUsers}
	onCancel={() => setRemoveMembersConfirmModalVisible(false)}
	prompt={props.translate('workspace.people.removeMembersPrompt')}
	confirmText={props.translate('common.remove')}
	cancelText={props.translate('common.cancel')}
	/>

we can add this ConfirmModal of WorkspaceMembersPage

shouldSetModalVisibility={false}

What alternative solutions did you explore? (Optional)

[makiour]

Proposal

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

The issue here is that the paste command is not working when trying to paste into invite message after deleting a user.

Investigation
After I looked into the error, I found that a paste issue is occuring also when we try to paste in the edited comment as it is a composer component same as the main chat composer.
When we try to paste into an edited comment, it also pastes into the main composer of the report open. Here is a video illustrating the issue.

bugPasting.webm

Thus, the root cause should be fixing the whole pasting mechanism as it occurs in multiple instances of components.

What is the root cause of that problem?

The first thing I did is investigating the paste function, and I found that there was a global paste event listener which is always called and triggered when the component mount here:

App/src/components/Composer/index.js

Lines 176 to 178 in fa473e9

	// We need to add paste listener manually as well as navigation focus event is not triggered on component mount
	document.addEventListener('paste', this.handlePaste);
	}

And will unmount here:

App/src/components/Composer/index.js

Line 210 in fa473e9

document.removeEventListener('paste', this.handlePaste);

My first thought is that a general paste listener is not needed as we have handlePaste that will do the job, however, the issue here is if we delete the global pasting listener, this will not allow attachment or HTML pasting and they are handled differently.

The root cause, then, is that we are making the listener global for the entire HTML document, which affects all elements/components of the page. Thus, we need to handle it specifically for the input we are dealing with:

--> We need to use the textInput element istead of document where the listener then will be associated with only to the specific input!

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

We need to change componentDidMount line:

App/src/components/Composer/index.js

Line 177 in fa473e9

document.addEventListener('paste', this.handlePaste);

to

this.textInput.addEventListener('paste', this.handlePaste);

and componentWillUnmount line:

App/src/components/Composer/index.js

Line 210 in fa473e9

document.removeEventListener('paste', this.handlePaste);

To

this.textInput.removeEventListener('paste', this.handlePaste);

What alternative solutions did you explore? (Optional)

N/A

[dukenv0307]

it seems that this PR causes regression #21583

[johncschuster]

Heads up, @robertKozik

[ginsuma]

Proposal

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

Web - On copy paste on workspace invite message page, app pastes text on compose box.

What is the root cause of that problem?

With normal flow, paste event will be removed when report page blurred by here. But when we use direct link, it won't happen and paste event still listen by here.

In handlePaste, we check checkComposerVisibility with the condition of modal.isVisible here. When we cancel ConfirmModal, it become false then checkComposerVisibility is true => not early return to block paste event to main composer.

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

I think we shouldn't rely on modal key to focus or set value for composer.
We will do it when the active route is CentralPaneNavigator.
Creating a function in Navigation like below:

function isCPNRoute() {
    const rootState = navigationRef.getRootState();
    const lastRoute = _.last(rootState.routes);
    return lastRoute.name === NAVIGATORS.CENTRAL_PANE_NAVIGATOR;
}

Then change checkComposerVisibility to:

checkComposerVisibility() {
        const isComposerCoveredUp = !Navigation.isCPNRoute() || EmojiPickerActions.isEmojiPickerVisible() || this.state.isMenuVisible
        return !isComposerCoveredUp;
}

What alternative solutions did you explore? (Optional)

We can use isCPNRoute to other issues about focusing main composer here.

[robertKozik]

thanks @johncschuster for mentioning. Looking into it as indeed it's regression

[robertKozik]

Proposal

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

Web - user can paste into composer when RHN is open after deeplinking into this page

What is the root cause of that problem?

when accessing the page via deeplink, the Composer is not removing the paste event listner as it happens only on navigation blur event. This is not the case when accessing the page via the normal means.

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

I'm proposing to keep the modal key check as it's covering most of the modal cases. In addition to that I propose to check wheter the COmposer is covered up by checking the route. Composer is not covered up by HRN when the url points directly to the report page. To check it we can use two methods: Navigation's getActiveRoute to get current route state and ROUTE's parseReportRouteParams to get access to current reportID and sub-report page status. When reportID is not present or isSubReportPageRoute is true, we can assume that the Composer is covered up and we cannot paste into composer.

checkComposerVisibility() {
        const {reportID, isSubReportPageRoute} = ROUTES.parseReportRouteParams(Navigation.getActiveRoute());
        const isComposerCoveredUp = EmojiPickerActions.isEmojiPickerVisible() || this.state.isMenuVisible || this.props.modal.isVisible || !reportID || isSubReportPageRoute;
        return !isComposerCoveredUp;
    }

In addition to that I propose to move event.preventDefault() in handlePaste in Composer/index.js to be placed after the initial cover up check. This will allow to paste into textInputs if the composer is covered up (Before we won't pass event further).

What alternative solutions did you explore? (Optional)

N/A

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~0105c6892023aa08e8

[melvin-bot]

Current assignee @johncschuster 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)

[johncschuster]

@parasharrajat do you agree with @robertKozik's proposal above, or do you think there are better/different ways we can resolve the issue?

[makiour]

Hey @johncschuster!

I hope you are doing good. I just want to make sure if you guys review all proposals above?

Thank you!

[parasharrajat]

Reviewing proposals. Need to test a few things myself. Found the root cause #21583.

[parasharrajat]

Bump @Santhosh-Sellavel.

[Santhosh-Sellavel]

This is fixed now, can you still reproduce this?

[Santhosh-Sellavel]

This ends up as a duplicate as all share a common root cause.
@parasharrajat I'll pick this up from you anyways as it is a regression,
@johncschuster Please assign me this one.

I'll post the next steps here, thanks!

[melvin-bot]

📣 @parasharrajat Please request via NewDot manual requests for the Reviewer role ($1000)

[melvin-bot]

📣 @Santhosh-Sellavel Please request via NewDot manual requests for the Contributor role ($1000)

[melvin-bot]

📣 @dhanashree-sawant 🎉 An offer has been automatically sent to your Upwork account for the Reporter role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[abdulrahuman5196]

@Santhosh-Sellavel This issue also seems to be related - #23319

[melvin-bot]

@johncschuster, @parasharrajat, @Santhosh-Sellavel Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[johncschuster]

@Santhosh-Sellavel how's it going? Do you have next steps for us?

[mvtglobally]

Issue not reproducible during KI retests. (First week)

[melvin-bot]

@johncschuster @parasharrajat @Santhosh-Sellavel this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[johncschuster]

It seems the issue is no longer reproducible. Holding.

[Santhosh-Sellavel]

Just on hold, for now, discussion in Slack here

[johncschuster]

Thanks for the heads up, @Santhosh-Sellavel!

[melvin-bot]

@johncschuster @parasharrajat @Santhosh-Sellavel this issue is now 3 weeks old. There is one more week left before this issue breaks WAQ and will need to go internal. What needs to happen to get a PR in review this week? Please create a thread in #expensify-open-source to discuss. Thanks!

[Santhosh-Sellavel]

@johncschuster We decided not to revert the PR, so we can close this one and just issue reporting bonus here. As this one is already fixed.

[johncschuster]

Payment has been issued to the bug reporter (@dhanashree-sawant). Closing!