[Awaiting payment] [$250] Web - Chat - Error in JS console when right-clicking in the app

[izarutskaya]

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

---

Found when validating PR : #37316

Version Number: v1.4.48-0
Reproducible in staging?: Y
Reproducible in production?: Y
Email or phone of affected tester (no customers): almexp2+777@gmail.com
Logs: https://stackoverflow.com/c/expensify/questions/4856
Issue reported by: Applause-Internal team

Action Performed:

1. Go to any chat with some attachments
2. Open JS console and refresh the page
3. Right click on any attachment
4. Refresh the page to clear your console
5. Right-click on a chat message
6. Observe the console error
7. Refresh again
8. Right-click on a chat row in the LHN
9. Observe the console error

Expected Result:

No errors appear in the JS console.

Actual Result:

Error in JS console when right-clicking.
Error: 'Cannot record touch end without a touch start. Source.

Workaround:

N/A, right-clicking still works functionally.

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

Bug6404745_1709769300071.Staging_-_error_in_JS_console_when_right_clicking_any_attachment.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0122ffd00715c18b6e
• Upwork Job ID: 1765696373749649408
• Last Price Increase: 2024-03-07
• Automatic offers:
• dukenv0307 | Contributor | 0

[melvin-bot]

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

[izarutskaya]

@trjExpensify 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.

We think that this bug might be related to #vip-vsb
CC @quinthar

[trjExpensify]

Hey!

1. #vip-vsb is correct as a chat bug. I can reproduce it reliably.
2. It doesn't seem to be causing a functional issue. You can still right-click, so it has no impact on usability.
3. Ideally we don't have JS warnings in the console though. This warning isn't limited to right-clicking attachments, but rather anywhere you right-click in the app (i.e a chat message, LHN chat row etc).
4. Looks like it's something to do with the recordTouchEnd function and RNW, so I don't see why it can't be external.

CC: @mountiny quick hip check. Shall we proceed to find a fix for this? I'm not aware of a logical reason for not recording a touch start with the right-click action.

[mountiny]

I agree with the assessment, I think its best to get rid of the error so I would export this issue for contributors for $250

[melvin-bot]

⚠️ Could not update price automatically because there is no linked Upwork Job ID. The BZ team member will need to update the price manually in Upwork.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~0122ffd00715c18b6e

[melvin-bot]

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

[trjExpensify]

Sounds good!

[dukenv0307]

Proposal

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

Error in JS console when right-clicking.
Error: 'Cannot record touch end without a touch start. Source.

What is the root cause of that problem?

In here, we detect only primary or no button (0 or 1), that means in the case of secondary button (buttons === 2), it will not trigger saving the event history.

So when we right click, the mouseup will occur and unable to find the initial right click event, leading to the warning.

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

In here, add || buttons === 2 case so it will work with right clicks.

We can rename the method to isPrimaryOrSecondaryPointerDown as that's more suitable now.

What alternative solutions did you explore? (Optional)

We can remove the button and buttons check here so that any type of mousedown without modifiers will register the event in history.

An alternative is to additionally check isPrimaryPointerDown(domEvent) only when updating the trackedTouchCount here so it shouldn't affect the recordTouchTrack operation

[trjExpensify]

What do you think, @mananjadhav?

[mananjadhav]

While @dukenv0307's proposal looks promising, I first tried reproducing it and I can't. I tried on v1.4.50-2, MacOS Chrome and Safari both.

js-error.mov

[dukenv0307]

While @dukenv0307's proposal looks promising, I first tried reproducing it and I can't. I tried on v1.4.50-2, MacOS Chrome and Safari both.

@mananjadhav I can reproduce every time on latest staging on Chrome, can you try refreshing the report (with attachment) multiple times?

[trjExpensify]

Yeah, I just retested and it's still present on all of the below actions after a refresh and right-click.

Ideally we don't have JS warnings in the console though. This warning isn't limited to right-clicking attachments, but rather anywhere you right-click in the app (i.e a chat message, LHN chat row etc).

[mananjadhav]

Sorry my bad I was checking the Errors only and not warnings.

additionally check isPrimaryPointerDown(domEvent) only when updating the trackedTouchCount here so it shouldn't affect the recordTouchTrack operation

I am inclined to have this solution, but I can see isPrimaryPointerDown is used only at one place so we might just update the method to isPrimaryOrSecondaryPointerDown.

Going to take a feedback from internal engineer. But @dukenv0307's proposal/alternative works for me.

🎀 👀 🎀 C+ reviewed

[melvin-bot]

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

[melvin-bot]

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

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[deetergp]

I think renaming it to isPrimaryOrSecondaryPointerDown is fine. I've assigned @dukenv0307.

[trjExpensify]

Cool, looking forward to the PR, @dukenv0307!

[deetergp]

PR is currently being tested on staging

[mananjadhav]

This was deployed on production 4 days ago but the payout date wasn't updated.

[mananjadhav]

@trjExpensify This is ready for payout.

We don't have any offending PR and we don't need any regression test for this.

[mananjadhav]

@trjExpensify Can you help here?

[trjExpensify]

Yup! Payment summary as follows:

• $250 to @dukenv0307 for the fix (Paid!)
• $250 to @mananjadhav for the C+ review (feel free to submit a request!)

Thanks!

[JmillsExpensify]

$250 approved for @mananjadhav