[HOLD FOR #23546][$500] mWeb - PDF - When typing password, revealing it and confirm, another attachment is displayed

[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!

---

Version Number: 1.4.32-2
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4238270
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:

Pre-requisite: the user must be logged in.

1. Go to any chat.
2. Send a password protected PDF.
3. Send another attachment.
4. Tap on the password protected PDF preview.
5. Type the password.
6. Tap on the eye icon button to reveal it (Important step).
7. Tap on "Confirm".

Expected Result:

The password protected PDF should be displayed.

Actual Result:

Another file is displayed.

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

Bug6355154_1706218295008.Xfnu9744_1_.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~014ec322edab15924f
• Upwork Job ID: 1750639602223542272
• Last Price Increase: 2024-02-08

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~014ec322edab15924f

[melvin-bot]

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

[melvin-bot]

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

[kbecciv]

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

[jeremy-croff]

Seems like a side effect of #23546
I noticed it diddn't even open the correct image on initial load.

[ikevin127]

Proposal

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

Another file is displayed after inputting the password for a password protected pdf file, tapping the reveal password "Eye" then pressing the Confirm.

What is the root cause of that problem?

I found the culprit, the line which for some weird unknown reason causes this issue, only after tapping the reveal password "Eye" then pressing the Confirm button. It was introduced by this PR #26025 and this is the line:

App/src/components/PDFView/PDFPasswordForm.js

Lines 145 to 146 in 1439ac0

	// Keep focus on the TextInput effectively keeping keyboard open
	onMouseDown={(e) => e.preventDefault()}

I know this is not enough to pass as an acceptable RCA since the reason why it's only happening when the reveal password "Eye" is tapped, is still unknown. But since I found a solution to the issue I thought I should mention it 🌞

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

Removing the line mentioned above fixes the issue but that wound also mean removing the functionality added by the mentioned PR which is: keyboard should stay open (keeping input focused) when clicking the Confirm button.

If we really want this fixed even though the full root cause is unknown, and we're fine with not having the "keyboard should stay open" functionality on iOS: mWeb Safari - we can create a library that will target only iOS: mWeb Safari, keeping the PR's functionality for all other platforms while fixing the current issue in Safari.

Videos

iOS: mWeb Safari

Before	After
Screen.Recording.2024-01-26.at.16.34.53.mov	Screen.Recording.2024-01-26.at.16.36.06.mov

MacOS: Safari

Before	After
Screen.Recording.2024-02-05.at.17.23.07.mov	Screen.Recording.2024-02-05.at.17.24.38.mov

[c3024]

Thanks for digging.

That did work. Let's see if we can identify the root cause.

[jeremy-croff]

I also had identified the mouseEvent tergerring the wrong image. And after this not making sense identified that initially the correct image did not even op in the video. The mouse event I believe is just a side effect of the original safari bug and the FlatList component

[ikevin127]

Indeed, @kidroca already identified a similar issue and raised it here: necolas/react-native-web#2544, more details: #23546 (comment)

From the react-native-web issue:

When opening a Modal that includes a ScrollView or a VirtualList spanning multiple pages, the ScrollView automatically scrolls to the first element. This issue arises from the modal focus trap logic, even when the initialScrollIndex is set to a value other than the index of the first element.

For sure part of the root cause here is related to the FlatList edge case experienced on Safari - the modal's focus trap logic.

We just don't know exactly why does the reveal password together with the preventDefault on the onMouseDown of thr Confirm button produces the current issue.

I'll try doing some more digging in hopes of finding the root cause of the current issue and possibly get back with a better solution 🚀

[CortneyOfstad]

@c3024 thoughts on the proposals above? Thanks!

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[CortneyOfstad]

@c3024 bump on the proposals above? Thanks!

[c3024]

Thanks for the bump. I'll update in a few hours.

[c3024]

Asked here on Slack

[c3024]

@c3024 I wanted to mention the proposal also corrects the image not getting loaded correctly when more than one image is loaded, in desktop safari incorrect image is loaded.

I think you mean this solution fixes another bug as well. Could you explain this preferably with a video? Thanks. @Sourcecodedeveloper

[CortneyOfstad]

In the linked GH, there is discussion around next steps, so going to keep this at weekly until the PR is resolved.

[CortneyOfstad]

Bumped the other GH — will continue to keep an eye on things 👍

[CortneyOfstad]

Back from Ooo! Bumped the other GH and requested an update by EOD today. Will follow up here when received 👍

[CortneyOfstad]

Realized I forgot to add the wave/VIP — adjusted this to match the on-hold issue 👍

Bumped the GH again here — Rajat is working to try and recreate under the latest issue, while Peter is continuing to work on a solution that will resolve multiple reported issues under this umbrella of images and attachments displaying incorrectly 👍

[CortneyOfstad]

Peter has found that the issue is affecting web browsers, so fix will be focused there 👍

[CortneyOfstad]

Bumped on the on-hold GH here — will update as soon as I hear back 👍

[CortneyOfstad]

Draft PR created here!

[CortneyOfstad]

Reached out for additional guidance/next steps here

[CortneyOfstad]

PR is in review now — #39930

[CortneyOfstad]

PR is still in review 👍

[CortneyOfstad]

Bumped a comment on the PR here

[CortneyOfstad]

PR is continuing to be reviewed with comments as recent as <2 hours ago 👍

[CortneyOfstad]

PR has been reviewed and should be marked for merging based on the comment here

[CortneyOfstad]

We're on a merge freeze until Wednesday, so this will be delayed 👍

[CortneyOfstad]

Merge freeze should end today 🤞

[CortneyOfstad]

Went into product:ion 4 days ago! 😍

[CortneyOfstad]

On hold issue has been resolved/completed, so this can be closed! 🎉