[HOLD for payment 2022-09-15] [$500] Feature Request - Attachment- Unable to cancel the requested password for 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 https://staging.new.expensify.com/
2. Log in with any account and select any chat
3. Click on plus button
4. Select add attachment > select Document.
5. Choose any password protected PDF.
6. Put password and select Send
7. Open again the PDF attachment and click Cancel

You can use this protected PDF file to test: password-protected.pdf

The password is 1234

Expected Result:

Able to cancel the requested password for PDF file

Actual Result:

Unable to cancel the requested password for PDF file

Workaround:

Unknown

Platform:

Where is this issue occurring?

• Web
• iOS
• Android
• Desktop App
• Mobile Web

Version Number: 1.1.54.0

Reproducible in staging?: Yes

Reproducible in production?: Yes

Email or phone of affected tester (no customers): applausetester+0901abb@applause.expensifail.com

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos: Any additional supporting documentation

Image.from.iOS.14.MP4

Expensify/Expensify Issue URL:

Issue reported by: Applause @parasharrajat

Slack conversation:

View all open jobs on GitHub

[melvin-bot]

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

[aldo-expensify]

Added password-protected pdf file to the issue description. Note that his problem also is there as soon as you are uploading the file because we have a preview at that stage too.

[melvin-bot]

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

[aldo-expensify]

I noticed that the preview for password-protected pdf files doesn't work the same in the desktop, it just shows "Failed to load PDF file" without asking for the password. We should have consistent behaviour if possible.

[kbecciv]

@aldo-expensify We logged a separate issue for Failed the PDF file #8593

[kadiealexander]

Posted to Upwork!

External: https://www.upwork.com/jobs/~017391a0b1899e031e
Internal: https://www.upwork.com/ab/applicants/1513700366393270272/job-details

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @Santhosh-Sellavel (Exported)

[melvin-bot]

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

[parasharrajat]

I think I reported this issue two weeks back https://expensify.slack.com/archives/C01GTK53T8Q/p1649156147168719.

[phivh]

Proposal

1. Due to limitations from react-pdf we need to add a custom handler for the password that suggested by the 3rd author (Prompt for password for password protected PDF keeps popping up wojtekmaj/react-pdf#581 (comment))
2. In src/components/PDFView/index.js, add a custom onPassword function:

onHandleCallback(callback, password) {
    // Cancel button handler
    if (password === null) {
        return this.props.onCancel();
    }

    callback(password);
}

onPassword(callback, reason) {
    const { PasswordResponses } = pdfjs;
    switch (reason) {
        case PasswordResponses.NEED_PASSWORD: {
            const password = prompt('Enter the password to open this PDF file.');
            this.onHandleCallback(callback, password);
            break;
        }
        case PasswordResponses.INCORRECT_PASSWORD: {
            const password = prompt('Invalid password. Please try again.');
            this.onHandleCallback(callback, password);
            break;
        }
        default:
            break;
    }
}

Apply to

<Document
   ....
    onPassword={this.onPassword}
>
   ...
</Document>

By applying this solution we are able to resolve issue #8593 as well.

Demo

mWeb

mWeb.mp4

Web

web.mp4

[tgolen]

@phivh thank you for the proposal! I am mostly on board with that, but I have a few requests:

• Can you please rename this.onPassword -> this.promptForPassword?
• Where does pdfjs come from in const { PasswordResponses } = pdfjs;?
• Where does prompt() come from?
• I kind of think that onHandleCallback feels unnecessary. Can that code be moved into promptForPassword() at all? I'm not quite following how it all goes

[Santhosh-Sellavel]

@tgolen 🚀 fast here.

[phivh]

@tgolen

• Can you please rename this.onPassword -> this.promptForPassword?
  => it can be, will change the name when create PR
• Where does pdfjs come from in const { PasswordResponses } = pdfjs;?
  => It's from react-pdf module, it should call right above the class component, I just want to put it inside the function more clearly.
• Where does prompt() come from?
  => prompt() is another way to call the confirm dialog just like window.promt(). FYI https://developer.mozilla.org/en-US/docs/Web/API/Window/prompt#example
• I kind of think that onHandleCallback feels unnecessary. Can that code be moved into promptForPassword() at all? I'm not quite following how it all goes
  => No, it is necessary to handle action cancel or ok then check the password Null or not then execute the default callback()

[Santhosh-Sellavel]

@phivh

I kind of think that onHandleCallback feels unnecessary. Can that code be moved into promptForPassword() at all? I'm not quite following how it all goes
=> No, it is necessary to handle action cancel or ok then check the password Null or not then execute the default callback()

I too feel onHandleCallback is unnecessary, we could refactor like below.

promptForPassword(callback, reason) {
    const {PasswordResponses} = pdfjs;
    let password;

    if (reason == PasswordResponses.NEED_PASSWORD) {
        password = prompt('Enter the password to open this PDF file.');
    } else if (reason == PasswordResponses.INCORRECT_PASSWORD) {
        password = prompt('Invalid password. Please try again.');
    }

    if (!password) {
        return this.props.onCancel();
    }

    callback(password);
}

cc: @tgolen

[tgolen]

OK, thanks! I agree with @Santhosh-Sellavel that it looks much better like that.

It's from react-pdf module, it should call right above the class component, I just want to put it inside the function more clearly.

I'm not sure I'm following this. Is it imported from react-pdf like this?

import {Document, Page, pdfjs} from 'react-pdf/dist/esm/entry.webpack';

I think anything being imported from a module should always be at the top of the file and never inline.

prompt() is another way to call the confirm dialog just like window.promt()

Hm, I guess that works since this is desktop/web, but are there any alternatives to using it? It's just such an ugly UX.

[melvin-bot]

⚠️ Looks like this issue was linked to a possible regression on PRODUCTION here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a production regression has occurred a Root Cause Analysis is required. Please follow the instructions here.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[parasharrajat]

In Progress....

[tgolen]

This was just merged today!

[melvin-bot]

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

• Frenkield 8595 pdf password #9141

If no regressions arise, payment will be issued on 2022-09-15. 🎊

[kadiealexander]

Reassigning as I'm ooo for the next 9 days.

[melvin-bot]

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

[mvtglobally]

Issue not reproducible during KI retests. (First week)

[Christinadobrzyn]

PR in product with no regressions for 7 days - paying @frenkield $1500 based on the convo here.

Since this original Upwork job automatically closed.

I made a new job posting and hired @parasharrajat for payment.

Internal posting - https://www.upwork.com/ab/applicants/1570596159667929088/job-details
External posting- https://www.upwork.com/jobs/~010327ce806aef815b

[Christinadobrzyn]

Hey @super-soft-dev - thanks for reaching out! This project is complete but feel free to reach out to us at contributors@expensify.com with a link to your Upwork profile and our team can add you to Slack to help with open jobs.

[AkihiroIna]

ok Thanks

[Christinadobrzyn]

@parasharrajat I just paid out this job $500 for the original amount and $1500 as the bonus. Please feel free to reach out with any questions! Thank you!