[$250] Split - App crashes when tapping Copy to clipboard on code block context menu

[lanitochka17]

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

---

Version Number: 9.0.22-7
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Email or phone of affected tester (no customers): applausetester+kh050806@applause.expensifail.com
Issue reported by: Applause - Internal Team

Action Performed:

1. Launch New Expensify app
2. Go to group chat
3. Tap + > Spli expense > Manual
4. Create a split manual expense with code block as the description
5. Tap on the split preview
6. Long press on the code block
7. Tap Copy to clipboard

Expected Result:

App will not crash

Actual Result:

App crashes when tapping Copy to clipboard on code block context menu in split menu

Workaround:

Unknown

Platforms:

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

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6577380_1724181250000.Screen_Recording_20240821_030954_One_UI_Home.mp4

logs (2).txt

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01a78f1cc82b94611b
• Upwork Job ID: 1827396760466325645
• Last Price Increase: 2024-08-24
• Automatic offers:
• hoangzinh | Reviewer | 103731517

Issue Owner

Current Issue Owner: @hoangzinh

[melvin-bot]

Triggered auto assignment to @kevinksullivan (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[lanitochka17]

@kevinksullivan FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

[lanitochka17]

We think that this bug might be related to #vip-split

[Krishna2323]

Edited by proposal-police: This proposal was edited at 2024-08-20 19:46:28 UTC.

Proposal

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

Split - App crashes when tapping Copy to clipboard on code block context menu

What is the root cause of that problem?

Context menu is not disabled when action and report are undefined.

App/src/components/HTMLEngineProvider/HTMLRenderers/PreRenderer.tsx

Lines 42 to 44 in dd32073

	onLongPress={(event) =>
	showContextMenuForReport(event, anchor, report?.reportID ?? '-1', action, checkIfContextMenuActive, ReportUtils.isArchivedRoom(report, reportNameValuePairs))
	}

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

• We should not call showContextMenuForReport when report or action is undefined. The check can be added before calling showContextMenuForReport or here.
• We should also check other html renderers with same bug.

What alternative solutions did you explore? (Optional)



When report or action is not present, we can render TDefaultRenderer without PressableWithoutFeedback.

        <View style={isLast ? styles.mt2 : styles.mv2}>
            <ShowContextMenuContext.Consumer>
                {({anchor, report, reportNameValuePairs, action, checkIfContextMenuActive}) =>
                    report && action ? (
                        <PressableWithoutFeedback
                            onPress={onPressIn ?? (() => {})}
                            onPressIn={onPressIn}
                            onPressOut={onPressOut}
                            onLongPress={(event) => {
                                if (!report || !action) {
                                    return;
                                }
                                showContextMenuForReport(event, anchor, report?.reportID ?? '-1', action, checkIfContextMenuActive, ReportUtils.isArchivedRoom(report, reportNameValuePairs));
                            }}
                            shouldUseHapticsOnLongPress
                            role={CONST.ROLE.PRESENTATION}
                            accessibilityLabel={translate('accessibilityHints.prestyledText')}
                        >
                            <View>
                                {/* eslint-disable-next-line react/jsx-props-no-spreading */}
                                <TDefaultRenderer {...defaultRendererProps} />
                            </View>
                        </PressableWithoutFeedback>
                    ) : (
                        <View>
                            {/* eslint-disable-next-line react/jsx-props-no-spreading */}
                            <TDefaultRenderer {...defaultRendererProps} />
                        </View>
                    )
                }
            </ShowContextMenuContext.Consumer>
        </View>

Result

[daledah]

Proposal

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

A long press on the description field show the context menu actions.

What is the root cause of that problem?

• The code block is displayed as the pre tag:
  

  App/src/components/HTMLEngineProvider/HTMLRenderers/PreRenderer.tsx

  Line 29 in dd32073

  function PreRenderer({TDefaultRenderer, onPressIn, onPressOut, onLongPress, ...defaultRendererProps}: PreRendererProps) {

• When long pressing on it, we will call:
  

  App/src/components/HTMLEngineProvider/HTMLRenderers/PreRenderer.tsx

  Line 43 in dd32073

  showContextMenuForReport(event, anchor, report?.reportID ?? '-1', action, checkIfContextMenuActive, ReportUtils.isArchivedRoom(report, reportNameValuePairs))

• There is the case the report or action data is undefined, because there is no ShowContextMenuContext.Provider wrapped in it, this issue for example.

• As a result, call any context menu action, such as "Copy to clipboard" will throw the error "Cannot read properties of null (reading 'actionName')".

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

• There are 4 types of context menu:

    CONTEXT_MENU_TYPES: {
        LINK: 'LINK',
        REPORT_ACTION: 'REPORT_ACTION',
        EMAIL: 'EMAIL',
        REPORT: 'REPORT',
    },

we must not allow user to open context menu type REPORT_ACTION by long pressing on the description field. Because they only can open context menu type REPORT_ACTION by long pressing on the report action.

• Additionally, the onLongPress in:
  

  App/src/components/HTMLEngineProvider/HTMLRenderers/PreRenderer.tsx

  Lines 42 to 44 in dd32073

  	onLongPress={(event) =>
  	showContextMenuForReport(event, anchor, report?.reportID ?? '-1', action, checkIfContextMenuActive, ReportUtils.isArchivedRoom(report, reportNameValuePairs))
  	}

was initially added in this PR to ensure that long-pressing on a link or email triggers context menu type LINK or EMAIL. However, this is redundant because if an email or link is wrapped in a code block like:

abcdef@gmail.com

or

https://dev.new.expensify.com:8082/

They are displayed as code, not as an active email or link. Therefore, long-pressing on them should not trigger the context menu type LINK or EMAIL.

• So the solution is just need to remove the above onLongPress.

What alternative solutions did you explore? (Optional)

• In ShowContextMenuContext, we can introduce a new value in its context value, named canOpenContextMenu and its default value is false.

• So this:
  

  App/src/components/HTMLEngineProvider/HTMLRenderers/PreRenderer.tsx

  Lines 42 to 44 in dd32073

  	onLongPress={(event) =>
  	showContextMenuForReport(event, anchor, report?.reportID ?? '-1', action, checkIfContextMenuActive, ReportUtils.isArchivedRoom(report, reportNameValuePairs))
  	}

  
  will be:

                        onLongPress={(event) => {
                            if (!canOpenContextMenu) {
                                return;
                            }
                            showContextMenuForReport(event, anchor, report?.reportID ?? '-1', action, checkIfContextMenuActive, ReportUtils.isArchivedRoom(report, reportNameValuePairs));
                        }}

and just pass canOpenContextMenu: true in where in need through the ShowContextMenuContext.provider.

[wildan-m]

Proposal

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

Context menu shown in code block in menu item

What is the root cause of that problem?

The context menu shouldn't be shown in codeblock in menu item

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

If codeblocks are unnecessary in the menu item description, we can remove the shouldParseTitle parameter from MenuItemWithTopDescription.

src/components/ReportActionItem/MoneyRequestView.tsx (or other similar parts)

                <OfflineWithFeedback pendingAction={getPendingFieldAction('comment')}>
                    <MenuItemWithTopDescription
                        description={translate('common.description')}
                        shouldParseTitle
                        title={updatedTransactionDescription ?? transactionDescription}

What alternative solutions did you explore? (Optional)

N/A

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01a78f1cc82b94611b

[melvin-bot]

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

[hoangzinh]

Thanks for proposals, everyone. Will review today

[hoangzinh]

Thanks for your patience, everyone. After reviewing, here is my feedbacks

• @Krishna2323's proposal: Can you add more details on your RCA next time? It's too short to understand the root cause. But your solution is most viable to me.
• @daledah's proposal: Love your RCA, it's clear and helpful to me. But your main solution will cause a regression bug when long press on mobile devices, it doesn't open the context menu. Your alternative solution looks promising, but it will cause passing unnecessary context value. Because I think if report or action is undefined (like @Krishna2323 described in his proposal), it means it's not on a report action.

Click here to view recording

Screen.Recording.2024-08-27.at.09.05.20.mov

• @wildan-m's proposal: I don't think your solution is our expectation.

---

Based on the above, I think we can go with @Krishna2323's proposal #47743 (comment)

Updated: changed my suggestion base on this #47743 (comment)

🎀👀🎀 C+ reviewed~

[melvin-bot]

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

[daledah]

@hoangzinh

when long press on mobile devices, it doesn't open the context menu

I think it is expected, long press on the money request view's description should not display the context menu action. For example, In the video below, we still can open the context menu action in the confirmation page:

Screen.Recording.2024-08-27.at.12.22.42.mov

[daledah]

cc @lakchote, do you have any thoughts on my comment above?

[daledah]

Added evidence in comment.

[hoangzinh]

@daledah I mean with your solution, on the report page, it won't open the context menu when user long presses on the codeblock messages

Step to reproduce on native apps:

1. Go to any reports
2. Send a codeblock message
3. Long press on that message
4. Expect: the context menu will be opened.

[hoangzinh]

Ah you can also check my recording here for reference #47743 (comment)

[daledah]

• Thanks @hoangzinh . I followed your test steps and It still works well with my changes:

Screen.Recording.2024-08-27.at.15.29.20.mov

• FYI, we already have a logic to show the context menu actions on long press:
  

  App/src/pages/home/report/ReportActionItem.tsx

  Line 347 in d5cfecf

  ReportActionContextMenu.showContextMenu(

[hoangzinh]

Can you go back and revisit the report again? When I test, I have to do the same to reproduce the issue if you've already opened the report page

Click here to view recording

Screen.Recording.2024-08-27.at.17.41.34.mov

[daledah]

Can you go back and revisit the report again?

• I was only able to test on the web due to an issue with running the iOS app. I noticed that if I long press on a container area outside the code block, the context menu opens. However, if I long press directly on the code block, the context menu does not open.

Screen.Recording.2024-08-27.at.18.06.10.mov

• I my alternative solution can fix two bugs because it fixed the RCA:

I mean with your solution, on the report page, it won't open the context menu when user long presses on the codeblock messages

and

we still can open the context menu action in the confirmation page

[hoangzinh]

@daledah Yeah. That's why I mentioned in my feedback comment here #47743 (comment). Your alternative solution looks promise and explicit, but I do more like @Krishna2323's solution in this case.

[melvin-bot]

Current assignee @lakchote is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

[lakchote]

I do agree with you, let's go with @daledah's alternative solution.

Thanks for reviewing.

[melvin-bot]

📣 @hoangzinh 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[melvin-bot]

📣 @daledah You have been assigned to this job!
Please apply to the Upwork job and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Once you apply to this job, your Upwork ID will be stored and you will be automatically hired for future jobs!
Keep in mind: Code of Conduct | Contributing 📖

[daledah]

@hoangzinh PR is ready.

[kevinksullivan]

PR still in review. Looping in another BZ member as I am going OOO

[melvin-bot]

Triggered auto assignment to @isabelastisser (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[isabelastisser]

No updates yet.

[melvin-bot]

@hoangzinh, @lakchote, @kevinksullivan, @isabelastisser, @daledah Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[lakchote]

Discussion is still happening in the PR.

[melvin-bot]

@hoangzinh, @lakchote, @kevinksullivan, @isabelastisser, @daledah 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

[hoangzinh]

PR has been deployed to staging. Awaiting for testing

[hoangzinh]

@lakchote @isabelastisser can you help to add the "Weekly" label back for this issue? It seems to be reverted to "Daily"

[lakchote]

@lakchote @isabelastisser can you help to add the "Weekly" label back for this issue? It seems to be reverted to "Daily"

Done.