[$250] BUG: Mac/Firefox - Opening the protect PDF modal and pressing TAB doesn’t focus on enter the password reported by @dhairyasenjaliya

[kavimuru]

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

---

Action Performed:

1. Go to any conversation
2. Send Protected PDF (if required)
3. Open Protected PDF
4. Press TAB key simultaneously try to navigate to enter the password with TAB
5. Notice it will focus only on Close Modal /Download PDF

Expected Result:

On pressing TAB it should be able to focus on enter the password

Actual Result:

On pressing TAB it does not focus on enter the password

Workaround:

unknown

Platform:

Where is this issue occurring?

• Web - Mac Firefox only

Version Number: 1.2.18-4 Reproducible in staging?: y Reproducible in production?: y Email or phone of affected tester (no customers): Logs: https://stackoverflow.com/c/expensify/questions/4856 Notes/Photos/Videos:

https://user-images.githubusercontent.com/43996225/197225996-c89a3cca-4019-4e1c-8008-e1013d7f2508.mov

https://user-images.githubusercontent.com/43996225/197226295-9146d9c4-8228-4951-8049-589f12a3484f.mp4

PDF used : password-protected.pdf

Expensify/Expensify Issue URL: Issue reported by: @dhairyasenjaliya Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1666236736080999

View all open jobs on GitHub

[melvin-bot[bot]]

Triggered auto assignment to @zanyrenney (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

[mallenexpensify]

@zanyrenney new BZ chore SO is here, we want to ensure BZ stays assigned to all issues so we can get to BugZero,

[melvin-bot[bot]]

@zanyrenney Whoops! This issue is 2 days overdue. Let's get this updated quick!

[zanyrenney]

Hey @kavimuru - Are you created the password protected PDF in Adobe? That's the only way I see how to recreate this. If you can attach an example of the PDF to this GH issue, i can test this properly.

[kavimuru]

@zanyrenney Added the pdf used for this test in the GH.

[zanyrenney]

I need the password to be able to test too though, please? @kavimuru

[kavimuru]

@zanyrenney Password is 1234 and attached another PDF

[zanyrenney]

Thanks!

[zanyrenney]

I can't reproduce this on Mac/Chrome - is it only happening on FireFox @kavimuru ?

[dhairyasenjaliya]

@zanyrenney this bug is only related to firefox

[zanyrenney]

Thanks. Asking in slack if we should be testing and fixing these super edge cases: https://expensify.slack.com/archives/C01SKUP7QR0/p1666963361304399

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[parasharrajat]

No proposals yet.

[zanyrenney]

waiting on proposals.

[zanyrenney]

Waiting on proposals, tomorrow we can up the price!

[huzaifa-99]

Proposal

Root Cause

This is happening because of the TextLink component used in PDFInfoMessage. https://github.com/Expensify/App/blob/17219657fd7a044abdd7b08adfa4717856113e59/src/components/PDFView/PDFInfoMessage.js#L32-L34

1. It is not capturing focus on FireFox/Safari with Tab
2. It captures focus on chrome with Tab but when enter is pressed the focus is lost without doing the actual press/click and moving to password screen

Solution

Wrapping TextLink in TouchableOpacity fixes the issue

<TouchableOpacity
    onPress={props.onShowForm}
>
    <TextLink onPress={props.onShowForm}>
        {` ${props.translate('attachmentView.pdfPasswordForm.linkText')} `}
    </TextLink>
</TouchableOpacity>

Demo with proposed solution

https://user-images.githubusercontent.com/68777211/199773111-66233dba-d9d1-45a5-a96d-88be613fd088.mp4

Suggestion

I believe the TextLink component should not be used here because it seems that it is used to render links and enter the password doesn't seem to be a link.

Currently hovering over it with a mouse points to the current url address (I don't know if this is the expected behaviour). Notice the bottom left side and the current url address, in this demo

https://user-images.githubusercontent.com/68777211/199774077-e8c507fc-3613-4891-9922-096103fd3607.mp4

We can remove this (url address on bottom left) by using a normal Text component wrapped inside TouchableOpacity (also applying the link style on Text)

<TouchableOpacity
    onPress={props.onShowForm}
>
    <Text style={[styles.link]}>
        {` ${props.translate('attachmentView.pdfPasswordForm.linkText')} `}
    </Text>
</TouchableOpacity>

Pressable would also work in place of TouchableOpacity if that is preferred.

---

Please let me know your thoughts @zanyrenney @parasharrajat

[huzaifa-99]

Waiting on proposals, tomorrow we can up the price!

Oops!. Didn't saw this, I should have waited till tomorrow 😂

[0xmiroslav]

I think this is duplicated with https://github.com/Expensify/App/issues/11354 or hold until https://github.com/Expensify/App/pull/12034 is merged

[dhairyasenjaliya]

@0xmiroslav this issue here is on pressing tab it does not focus on "Enter the password" only on firefox the issue you mentioned is of pressing enter needs to fire action which previously was not triggering

[parasharrajat]

The only reason that TextLink is used is that it does not create a block element in the middle of the text. @huzaifa-99 I liked your suggestion of using Pressable instead of this. But it will create the mentioned problem. I think the same goes for TouchableOpacity.

But I agree using either of these will fix both this issue and https://github.com/Expensify/App/issues/11354

[parasharrajat]

The only reason that TextLink is used is that it does not create a block element in the middle of the text. Block view will break the text wrapping and alignment on native.

@huzaifa-99 what do you think the best solution might be to fix this without causing this issue?

[michaelhaxhiu]

Hey @zanyrenney just dropping a note as a reminder to keep the pressure on to find a contributor and get this one closed out. If it helps incentivize @huzaifa-99 and @parasharrajat to finish the proposal / review and submit a PR sooner, I would even consider doubling the price now (even with an active proposal in review) 😉

[zanyrenney]

@huzaifa-99 and @parasharrajat price is doubled, hopefully we can get this one reviewed and submit the PR sooner: https://www.upwork.com/jobs/~0149cf7c7b989c99f5 🙏🏼

[parasharrajat]

WAiting for proposals.

[roryabraham]

Pressable would also work in place of TouchableOpacity if that is preferred.

Yes, Pressable is preferred.

https://github.com/Expensify/App/pull/12034 was just deployed to staging btw, so the Enter issue shouldn't exist anymore. Can we just add focusable=true to the TextLink component here?

[parasharrajat]

Yeah, that should do it.

[roryabraham]

Separately, why is this link even a thing? Why not just show the text input right away???

[parasharrajat]

This was added to remove the confusion between the two buttons. Password Confirm and Send attachment. The design team came up with this idea.