[HOLD for payment 2023-12-29] [$500] Android - Attachment - Application crashes when trying to scroll PDF preview

[kbecciv]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Version Number: 1.4.14-2
Reproducible in staging?: y
Reproducible in production?: n
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
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:

Action Performed:

Prerequisites:
File for example https://drive.google.com/file/d/1a_8roG0lxsaBAWAo33pLSs8pAfN2nCY9/view?usp=drive_link
Steps:

1. Open New Expensify app
2. Log in under any account
3. Navigate to any conversation
4. Click on Compose Box - Add attachment
5. Add the PDF file from the example
6. Scroll PDF file in preview

Expected Result:

The application should not crash when you try to scroll through the PDF preview.

Actual Result:

Application crashes when trying to scroll PDF preview

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

Bug6321235_1703078349555.Record_2023-12-20-19-01-18.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01d227608e2ee7d100
• Upwork Job ID: 1737627480242479104
• Last Price Increase: 2023-12-21

[github-actions]

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[melvin-bot]

Triggered auto assignment to @srikarparsi (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[mkhutornyi]

Proposal

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

Android app crashes when scroll PDF in preview

What is the root cause of that problem?

Crash log:
ERROR ReanimatedError: Cannot read property 'shouldPagerScroll' of null, js engine: reanimated

So it happens on this line: attachmentCarouselPagerContext.shouldPagerScroll.value = (attachmentCarouselPagerContext is null)

App/src/components/Attachments/AttachmentView/AttachmentViewPdf/index.android.js

Lines 21 to 29 in f33d674

	if (offsetX.value !== 0 && offsetY.value !== 0) {
	// if the value of X is greater than Y and the pdf is not zoomed in,
	// enable the pager scroll so that the user
	// can swipe to the next attachment otherwise disable it.
	if (Math.abs(evt.allTouches[0].absoluteX - offsetX.value) > Math.abs(evt.allTouches[0].absoluteY - offsetY.value) && scaleRef.value === 1) {
	attachmentCarouselPagerContext.shouldPagerScroll.value = true;
	} else {
	attachmentCarouselPagerContext.shouldPagerScroll.value = false;
	}

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

add null safety check
if (offsetX.value !== 0 && offsetY.value !== 0 && attachmentCarouselPagerContext) {

[srikarparsi]

Hey @mkhutornyi, are you available to work on this fix?

[mkhutornyi]

yes, I am

[srikarparsi]

Awesome, assigning you to the issue, can you create a PR and link it here?

[mkhutornyi]

@srikarparsi PR is ready

[melvin-bot]

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

[melvin-bot]

Current assignee @aimane-chnaif is eligible for the External assigner, not assigning anyone new.

[melvin-bot]

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

[srikarparsi]

Hey @muttmuure, @aimane-chnaif and @mkhutornyi helped with this deploy blocker so added the bug label for payment whenever you have a chance.

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.15-5 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• fix crash when trying to scroll pdf preview #33383

If no regressions arise, payment will be issued on 2023-12-29. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

For reference, here are some details about the assignees on this issue:

• @aimane-chnaif requires payment (Needs manual offer from BZ)
• @mkhutornyi requires payment (Needs manual offer from BZ)

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@aimane-chnaif] The PR that introduced the bug has been identified. Link to the PR:
• [@aimane-chnaif] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@aimane-chnaif] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@aimane-chnaif] Determine if we should create a regression test for this bug.
• [@aimane-chnaif] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@muttmuure] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[melvin-bot]

Payment Summary

Upwork Job

• : @aimane-chnaif paid $() via Upwork ()
• : @mkhutornyi paid $() via Upwork ()

BugZero Checklist (@muttmuure)

• I have verified the correct assignees and roles are listed above
• I have verified that there are no duplicate or incorrect contracts on Upwork (https://www.upwork.com/ab/applicants/1737627480242479104/hired)
• I have paid out the Upwork contracts or cancelled the ones that are incorrect
• I have verified the payment summary above is correct

[melvin-bot]

@muttmuure, @srikarparsi, @aimane-chnaif, @mkhutornyi Eep! 4 days overdue now. Issues have feelings too...

[muttmuure]

Manual offers sent

[muttmuure]

All paid up