mozilla-mobile / firefox-ios

Firefox for iOS
Mozilla Public License 2.0
12.08k stars 2.88k forks source link

Downloading PDF of Accounts Recovery Key breaks Accounts Recovery flow #17175

Open data-sync-user opened 8 months ago

data-sync-user commented 8 months ago

Check out the FxA bug with the steps to reproduce in the bug description: https://mozilla-hub.atlassian.net/browse/FXA-6086

See potential fixes explored in this comment: https://mozilla-hub.atlassian.net/browse/FXA-6086?focusedCommentId=696074

FxA team tried to fix this issue by enabling the pdf to download of the recovery key - instead of the text file - but this problem persists with PDFs too.

See this comment for impact of the current behavior: https://mozilla-hub.atlassian.net/browse/FXA-6086?focusedCommentId=780099

Is there a way to fix this within Firefox iOS?

┆Issue is synchronized with this Jira Bug

data-sync-user commented 5 months ago

➤ Nishant Bhasin commented:

I am looking into a solution which will allow users to share this PDF, by sharing they should be able to download it.

  1. Tapping on download loads the PDF !image-20240113-001516.png|width=390,height=542!

  2. Share button shows up that would allow the user to download this PDF or share it to their files app, this is common practice !image-20240113-001734.png|width=386,height=547!

Hi, Jessilyn Davis I do have a question regarding the url of this PDF, is there a way you can connect me to the person who deals with the route. The url isn’t of type PDF and would prefer if it is. It would be good to understand why this is the case as I always get a blob: urls

Hi, Daniela Arcese Since you originally assigned me there are few things for this approach

a) Need to find out if the top right icon I added is OKay with UX, who would be the person to contact in that case?

b) This may / may not make it into v122 (depends on the route etc.) so are you good with it being part of v123. I can try to aim for it to be in dot release but its not just UX at this point

data-sync-user commented 5 months ago

➤ Daniela Arcese commented:

I believe there is another problem here to solve (from this comment ( https://mozilla-hub.atlassian.net/browse/FXA-6086?focusedCommentId=780099 )) which is the inability to continue the flow after you view or download the PDF. Nishant Bhasin is there a way we can add a simple navigation bar to the bottom so you can move back and forward through the webview? Or when you click download can we popup a separate window for downloading (maybe straight to the save to files flow) so that the user gets returned to the flow when they are done downloading.

data-sync-user commented 5 months ago

➤ Nishant Bhasin commented:

Daniela Arcese Yeah possible, could we add a separate ticket for that as this whole view is very self contained and not part of our regular tab logic so it doesn’t really support any nav bar. It actually never supported a nav bar.

This ticket we can keep it for sharing and downloading the pdf for users

data-sync-user commented 5 months ago

➤ Jessilyn Davis commented:

Valerie Pomerleau - can you help answer this question (from an earlier comment)?

{quote}Hi, Jessilyn Davis I do have a question regarding the url of this PDF, is there a way you can connect me to the person who deals with the route. The url isn’t of type PDF and would prefer if it is. It would be good to understand why this is the case as I always get a blob: urls{quote}

data-sync-user commented 5 months ago

➤ Valerie Pomerleau commented:

Hi Nishant Bhasin the URL is a blob because the PDF is created client-side and stored in browser memory - it’s not a PDF that we can access via a URL. If you have an alternate suggestion, would be happy to discuss!

data-sync-user commented 5 months ago

➤ Nishant Bhasin commented:

I talked with Valerie Pomerleau today and showed how iOS has poor support for blob: urls for sharing pdf. Infact anything for blob: gets rendered on webkit but sharing is another story. Its not an urgent request from FxA but it can be annoying to deal with if users are stuck

Next steps:

data-sync-user commented 5 months ago

➤ Jessilyn Davis commented:

Thanks Nishant Bhasin ! QQ: is this related to this open bug at all? https://github.com/mozilla-mobile/firefox-ios/issues/8635 ( https://github.com/mozilla-mobile/firefox-ios/issues/8635|smart-link ) (maybe there is a way to fix this in iOS for more than just accounts?)

data-sync-user commented 5 months ago

➤ Nishant Bhasin commented:

Jessilyn Davis blob: urls on iOS WebKit aren’t supported for download directly but WebKit can load certain items like pdf, text etc. to show the user. The drawback is we can perform certain operations like showing the content to the user but downloading it becomes challenging.

If its a pure direct link and no authentication then I can perform a JS injection to download PDF or Text but that isn’t the case here so we are unfortunately limited to the resources at hand.

data-sync-user commented 5 months ago

➤ Jessilyn Davis commented:

Ahhh - thanks for the clarification Nishant Bhasin !

data-sync-user commented 2 months ago

➤ Alexandru Farcasanu commented:

Nishant Bhasin may you give me some hints in order to what should I implement or investigate, here? Thank you.

cc: Norberto Andres Furlan

data-sync-user commented 1 month ago

➤ Alexandru Farcasanu commented:

Nishant Bhasin

I successfully did something for this issue. I was able to get the PDF file from that blob URL and share it or saving it locally. Check out this video and let me know what do you think.

Is just a demo, the implementation in not ready for a PR, at this moment.

Also, because we already see a preview of that PDF file (0:09 min), I think is no more necessary to display the PDF in a new screen. Just display the option to download/share it and thats all.

!keys.mov|width=936,height=2012,alt="keys.mov"!

cc: Jessilyn Davis

data-sync-user commented 1 month ago

➤ Nishant Bhasin commented:

Alexandru Farcasanu I like it, good work!

This when we discussed makes sense to me as it gives the option for the user to save it which is whats important.

data-sync-user commented 1 month ago

➤ Jessilyn Davis commented:

This looks great Alexandru Farcasanu ! Thank you for the tag 🙌 (Check out the demo in the comment below Vesta Zare Valerie Pomerleau 😍 )

data-sync-user commented 1 month ago

➤ Valerie Pomerleau commented:

Seconded, the demo looks fantastic Alexandru Farcasanu! 🔥

data-sync-user commented 1 month ago

➤ Alexandru Farcasanu commented:

Thank you all for the feedback. I am going forward with the implementation.

data-sync-user commented 1 month ago

➤ Adina Petridean commented:

Verified as fixed on v9000(42096) with iPhone 14 Pro Max (iOS 16.2).

!RecoveryKey.mp4|width=886,height=1920,alt="RecoveryKey.mp4"!

data-sync-user commented 1 month ago

➤ Valerie Pomerleau commented:

So happy to see this fix implemented!! Thank you Alexandru Farcasanu Nishant Bhasin 💗

data-sync-user commented 1 month ago

➤ Valerie Pomerleau commented:

Hello! We’ve had a possibly related issue reported for Android (https://mozilla-hub.atlassian.net/browse/FXA-9718 ( https://mozilla-hub.atlassian.net/browse/FXA-9718|smart-link ) ) Wondering if Nishant Bhasin Alexandru Farcasanu might know who I could reach out to on mobile to share learnings from the fix on this PDF issue? Thanks in advance 🙏

data-sync-user commented 1 month ago

➤ Nishant Bhasin commented:

Hi 👋 Valerie Pomerleau you can reach out to Alexandru Farcasanu or myself for android support. Let me comment on the ticket as well.

data-sync-user commented 2 weeks ago

➤ Andrei Bodea commented:

Verified as fixed on v128 (42870) with iPhone 15 Pro (17.5.1).