[HOLD for payment 2023-02-10] [$1000] Attachment - Several "Loading" messages are displayed when loading a large PDF file

[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. Go to URL https://staging.new.expensify.com/
2. Login with any account
3. On a conversation, click on the + button in the compose box PDF
4. Click on add attachment
5. Upload a PDF
6. Verify a preview of the PDF is displayed before uploading to the conversation
7. Click on upload
8. Verify you can see the PDF in the conversation
9. Click on the PDF preview

PDF file

Вудсторія.pdf

Expected Result:

Only one "Loading" message is displayed when loading a large PDF file

Actual Result:

Several "Loading" messages are displayed when loading a large PDF file

Workaround:

Unknown

Platforms:

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

• Android / Chrome
• iOS / Safari
• Web/ Chrome

Version Number: 1.2.55.0

Reproducible in staging?: Yes

Reproducible in production?: Yes

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

Bug5899104_loading_android_chrome_1601.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~011a277bda6d8495b8
• Upwork Job ID: 1620158726929661952
• Last Price Increase: 2023-01-30

[adelekennedy]

Hmm - I don't even get a preview when I upload the example PDF in staging

[adelekennedy]

@kbecciv I can't seem to reproduce this - is it still happening for you?

[adelekennedy]

@kbecciv do you mind trying to reproduce it again? I can't seem to by following the above steps

[kbecciv]

@adelekennedy Checking with team, will update you shortly

[kbecciv]

@adelekennedy Issue is reproduced with build 1.2.60.0

pdf.2701.mp4

[marcaaron]

ah pretty sure this is some kind of bug with the pdf library we are using on the web side.

It's also barely noticeable 👴 🔍

[adelekennedy]

@marcaaron I also don't find it super noticeable and happening with the PDF library should we close for now or leave it open and make it internal?

[marcaaron]

Tbh I think it might be something worth fixing but it is so far down my list of priorities that I'd say make it External and see what happens.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~011a277bda6d8495b8

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[Prince-Mendiratta]

Proposal

RCA

The root cause why so many Loading Page... messages appear is because for each page that we render in the pdf, we use a Page component from the react-pdf library. We use a custom FullScreenLoadingIndicator to denote the loading of the document with the Document component from react-pdf. The Page components are the children of the Document component. For each page in the document, we map it using the Page component. When the size of the PDF is too large, it takes almost a second for all the pages to render after the FullScreenLoadingIndicator has finished, thus leading to so many Loading Page... messages appear/blink for just the slightest time.

As the number of pages in the PDF increase, the error becomes more and more visible, especially for long receipts / year long transaction documents and summaries. This is reproducible for all components that make use of the PDFView, except Android, since android uses react-native-pdf for displaying PDFs. As far as I've tested, as the number of pages in the PDF go beyond 50, this error becomes noticeable.

Solution

We can fix this by explicitly telling the library to not show the Loading Page... text for individual pages. This is the best case since it does not affect any functionality and the text is barely displayed anyways.

diff --git a/src/components/PDFView/index.js b/src/components/PDFView/index.js
index f44baa00c5..c3c62d138f 100644
--- a/src/components/PDFView/index.js
+++ b/src/components/PDFView/index.js
@@ -148,6 +148,7 @@ class PDFView extends Component {
                                 width={pageWidth}
                                 key={`page_${index + 1}`}
                                 pageNumber={index + 1}
+                                loading=""
                             />
                         ))}
                     </Document>

Result

Before Fix (Slowed to 0.25x for clarity)

before.mp4

After Fix (Slowed to 0.25x for clarity)

after.mp4

[Prince-Mendiratta]

PR is ready for review!

Applied in Upwork

cc @danieldoglas @aimane-chnaif

[adelekennedy]

@Prince-Mendiratta hired!

[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.2.64-7 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• [PDFView] no separate text for loading page #14728

If no regressions arise, payment will be issued on 2023-02-10. 🎊

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

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[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 / @danieldoglas] The PR that introduced the bug has been identified. Link to the PR:
• [@aimane-chnaif / @danieldoglas] 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 / @danieldoglas] 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:
• [@Prince-Mendiratta] Propose regression test steps to ensure the same bug will not reach production again.
• [@adelekennedy] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[Prince-Mendiratta]

Propose regression test steps to ensure the same bug will not reach production again.

Regression Test steps

1. Login to NewDot.
2. Open up any chat.
3. Click on the + icon in the composer.
4. Click on Add Attachment.
5. Choose the PDF from the document selector.
6. Notice that a PDF preview will open. It'll show the full screen loading icon and then preview the PDF. Ensure that no messages are displayed between the microseconds when the loading icon disappears and the preview begins.
7. Send the PDF in the chat.
8. Open the attached PDF in the chat.
9. Notice that a PDF preview will open. It'll show the full screen loading icon and then preview the PDF. Ensure that no messages are displayed between the microseconds when the loading icon disappears and the preview begins.

[MelvinBot]

@danieldoglas, @Prince-Mendiratta, @adelekennedy, @aimane-chnaif Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[danieldoglas]

@adelekennedy Did we issue the payment for this one? There was a comment about regression here, but there were no regressions, just another PR created to add a comment.

[Prince-Mendiratta]

gentle bump, @danieldoglas @adelekennedy, any updates here?

thanks

[danieldoglas]

@Prince-Mendiratta checking this, sorry for the delay!

[adelekennedy]

Hey sorry - I was waiting for the checklist to be filled out, @danieldoglas are you saying we can skip that? If so I'll move forward with payment plus a bonus for my delay!

[danieldoglas]

are you saying we can skip that?

Yes :)

[adelekennedy]

Payment sent @Prince-Mendiratta, pending acceptance from @aimane-chnaif

[Prince-Mendiratta]

@adelekennedy thanks! I think this qualifies for the timeline bonus as well, can you please check?

[MelvinBot]

📣 @Prince-Mendiratta! 📣

Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
2. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
3. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.

Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[aimane-chnaif]

Payment sent @Prince-Mendiratta, pending acceptance from @aimane-chnaif

@adelekennedy I accepted your offer. I haven't received payment yet. Can you please check?

[aimane-chnaif]

@adelekennedy I got payment now. Thanks