Closed Bug 1815195 Opened 2 years ago Closed 2 years ago

Add the possibility to save a PDF

Categories

(Fenix :: General, enhancement)

Product:
Fenix
Component:
General
Version:
Firefox 111
Platform:
All
Android
Type:
enhancement

Tracking

(firefox110 wontfix, firefox111 fixed)

Status:
RESOLVED FIXED
Milestone:
111 Branch
Tracking Flags:
Tracking Status
firefox110 --- wontfix
firefox111 --- fixed

People

(Reporter: calixte, Assigned: calixte)

References

Details

Attachments

(2 files)

Bug 1815195 - Get the bytes from pdf.js when saving a PDF in GeckoView r=#geckoview-reviewers
48 bytes, text/x-phabricator-request
Details | Review
[mozilla-mobile/firefox-android] Bug 1815195 - Avoid a redundant file extension when saving a real pdf file (#727)
58 bytes, text/x-github-pull-request
Details | Review

Right now, if we use the Save as PDF feature, the pdf viewer is printed into a pdf, which is not what the user wants.

In bug 1810761, we added some new api in GV in order to be able to get a byte array for the current viewed pdf.

Consequently we should be able to plug this new stuff in:
https://github.com/mozilla-mobile/firefox-android/blob/9de5ab8cc098674aad5190ce8b00ae6be65e9bd0/android-components/components/browser/engine-gecko/src/main/java/mozilla/components/browser/engine/gecko/GeckoEngineSession.kt#L218

In case the viewed document is a pdf:
https://searchfox.org/mozilla-central/rev/ddbeacddc15008936e8f619c8c3a05fac6eab8d8/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoSession.java#2375
we can just use:
https://searchfox.org/mozilla-central/rev/ddbeacddc15008936e8f619c8c3a05fac6eab8d8/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoSession.java#2337

Adding that in the actual requestPdfToDownload will fix the current wrong behaviour and will avoid to add a new button or whatever.

:olivia, what do you think ?

Flags: needinfo?(ohall)
Assignee: nobody → cdenizet
Status: NEW → ASSIGNED
Blocks: 1809429, 1810106, 1811716
Depends on: 1810761

(In reply to Calixte Denizet (:calixte) from comment #0)

Right now, if we use the Save as PDF feature, the pdf viewer is printed into a pdf, which is not what the user wants.

In bug 1810761, we added some new api in GV in order to be able to get a byte array for the current viewed pdf.

Thanks for this new API!

Consequently we should be able to plug this new stuff in:
https://github.com/mozilla-mobile/firefox-android/blob/9de5ab8cc098674aad5190ce8b00ae6be65e9bd0/android-components/components/browser/engine-gecko/src/main/java/mozilla/components/browser/engine/gecko/GeckoEngineSession.kt#L218

Branching off here at GeckoResult<InputStream> saveAsPdf() on the GeckoView side to return that PDF.js byte array as an InputStream might be ideal. I'm still working on window.print(), but one of the approaches I'm looking at leverages that method for getting a PDF ready to send to Android for printing (Android print API uses PDFs). We could add in another exception for PDF.js too, if needed.

Flags: needinfo?(ohall)

Yes it'll be likely better to do that here (I mean in saveAsPdf): I thought about that too but I thought that it could interfere with the kind of pdf you expect to return from this function.
For desktop, pdf printing is done thanks window.print and a specific feature mozPrintCallback (see bug 745025 for the details).
If the android print apis is able to take whatever pdf and print it, then it should be easy to print but if the pdf has to be very simple (e.g. each page has an image) we'd need to deal with the mozPrintCallback thing, hence it's why I thought having the saving in GeckoEngineSession.kt.

With the patch in D168972, saving is working correct but the name of the file is incorrect: for example with tracemonkey.pdf, on saving the final name is tracemonkey.pdf.pdf.
Hence we should either patch guessFileName or make an api change in GeckoSession::saveAsPdf in order to return something like GeckoResult<PdfSaveResult> (see PdfSaveResult) in order to have the correct filename which could be null in the normal case and then guess later which would require to patch GeckoEngineSession too.
:olivia, any other idea ? if not, what do you prefer between the two proposed solutions ?

(In reply to Calixte Denizet (:calixte) from comment #4)

With the patch in D168972, saving is working correct but the name of the file is incorrect: for example with tracemonkey.pdf, on saving the final name is tracemonkey.pdf.pdf.

Thanks for the patch and testing!

Hence we should either patch guessFileName or make an api change in GeckoSession::saveAsPdf in order to return something like GeckoResult<PdfSaveResult> (see PdfSaveResult) in order to have the correct filename which could be null in the normal case and then guess later which would require to patch GeckoEngineSession too.

I think updating guessFileName might be the preferred way to go. It seems odd that it didn't parse it as expected. A reason against changing the saveAsPdf signature is one of the Print Delegate signatures uses an InputStream and, also, like you mentioned, that would be a public API change, so it would cause changes in AC as well (and other GV embedders).

I forgot to mention, GeckoView Example also has that PrintDelegate connected using saveAsPdf as an example in the dot menu as "Print Page". (This is more like how the print feature implementation could be in bug 1808755.) I did a quick test using the patch with printing in GVE and it looks like it works well with Android Printing, really neat.

Pushed by cdenizet@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/a20d64d6501c Get the bytes from pdf.js when saving a PDF in GeckoView r=geckoview-reviewers,ohall

Backed out for causing geckoview failures on PdfCreationTest

[task 2023-02-08T01:02:11.980Z] 01:02:11     INFO -  TEST-START | org.mozilla.geckoview.test.PdfCreationTest#saveAPdfDocument
[task 2023-02-08T01:02:47.905Z] 01:02:47     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: numtests=1009
[task 2023-02-08T01:02:47.907Z] 01:02:47     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: stream=
[task 2023-02-08T01:02:47.908Z] 01:02:47     INFO -  org.mozilla.geckoview.test | Error in saveAPdfDocument(org.mozilla.geckoview.test.PdfCreationTest):
[task 2023-02-08T01:02:47.909Z] 01:02:47     INFO -  org.mozilla.geckoview.test | org.mozilla.geckoview.test.util.UiThreadUtils$TimeoutException: Timed out after 30000ms
[task 2023-02-08T01:02:47.909Z] 01:02:47     INFO -  org.mozilla.geckoview.test | 	at org.mozilla.geckoview.test.rule.GeckoSessionTestRule.waitUntilCalled(GeckoSessionTestRule.java:1791)
[task 2023-02-08T01:02:47.909Z] 01:02:47     INFO -  org.mozilla.geckoview.test | 	at org.mozilla.geckoview.test.rule.GeckoSessionTestRule.waitForPageStops(GeckoSessionTestRule.java:1544)
[task 2023-02-08T01:02:47.909Z] 01:02:47     INFO -  org.mozilla.geckoview.test | 	at org.mozilla.geckoview.test.rule.GeckoSessionTestRule.waitForPageStop(GeckoSessionTestRule.java:1519)
[task 2023-02-08T01:02:47.909Z] 01:02:47     INFO -  org.mozilla.geckoview.test | 	at org.mozilla.geckoview.test.BaseSessionTest.waitForPageStop(BaseSessionTest.kt:192)
[task 2023-02-08T01:02:47.909Z] 01:02:47     INFO -  org.mozilla.geckoview.test | 	at org.mozilla.geckoview.test.PdfCreationTest.saveAPdfDocument$lambda-11(PdfCreationTest.kt:151)
[task 2023-02-08T01:02:47.909Z] 01:02:47     INFO -  org.mozilla.geckoview.test | 	at org.mozilla.geckoview.test.PdfCreationTest.$r8$lambda$8dLvIz-uQcP-K8MWnOjkWG_Ou9I(PdfCreationTest.kt)
[task 2023-02-08T01:02:47.909Z] 01:02:47     INFO -  org.mozilla.geckoview.test | 	at org.mozilla.geckoview.test.PdfCreationTest$$ExternalSyntheticLambda2.perform(D8$$SyntheticClass)
[task 2023-02-08T01:02:47.909Z] 01:02:47     INFO -  org.mozilla.geckoview.test | 	at androidx.test.core.app.ActivityScenario.lambda$onActivity$2$ActivityScenario(ActivityScenario.java:660)
[task 2023-02-08T01:02:47.909Z] 01:02:47     INFO -  org.mozilla.geckoview.test | 	at androidx.test.core.app.ActivityScenario$$Lambda$4.run(ActivityScenario.java:652)
[task 2023-02-08T01:02:47.909Z] 01:02:47     INFO -  org.mozilla.geckoview.test | 	at androidx.test.core.app.ActivityScenario.onActivity(ActivityScenario.java:670)
[task 2023-02-08T01:02:47.909Z] 01:02:47     INFO -  org.mozilla.geckoview.test | 	at org.mozilla.geckoview.test.PdfCreationTest.saveAPdfDocument(PdfCreationTest.kt:149)
[task 2023-02-08T01:02:47.910Z] 01:02:47     INFO -  org.mozilla.geckoview.test |
[task 2023-02-08T01:02:47.910Z] 01:02:47     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: id=AndroidJUnitRunner
[task 2023-02-08T01:02:47.910Z] 01:02:47     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: test=saveAPdfDocument
[task 2023-02-08T01:02:47.910Z] 01:02:47     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: class=org.mozilla.geckoview.test.PdfCreationTest
[task 2023-02-08T01:02:47.910Z] 01:02:47     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: stack=org.mozilla.geckoview.test.util.UiThreadUtils$TimeoutException: Timed out after 30000ms
[task 2023-02-08T01:02:47.910Z] 01:02:47     INFO -  org.mozilla.geckoview.test | 	at org.mozilla.geckoview.test.rule.GeckoSessionTestRule.waitUntilCalled(GeckoSessionTestRule.java:1791)
[task 2023-02-08T01:02:47.910Z] 01:02:47     INFO -  org.mozilla.geckoview.test | 	at org.mozilla.geckoview.test.rule.GeckoSessionTestRule.waitForPageStops(GeckoSessionTestRule.java:1544)
[task 2023-02-08T01:02:47.910Z] 01:02:47     INFO -  org.mozilla.geckoview.test | 	at org.mozilla.geckoview.test.rule.GeckoSessionTestRule.waitForPageStop(GeckoSessionTestRule.java:1519)
[task 2023-02-08T01:02:47.910Z] 01:02:47     INFO -  org.mozilla.geckoview.test | 	at org.mozilla.geckoview.test.BaseSessionTest.waitForPageStop(BaseSessionTest.kt:192)
[task 2023-02-08T01:02:47.911Z] 01:02:47     INFO -  org.mozilla.geckoview.test | 	at org.mozilla.geckoview.test.PdfCreationTest.saveAPdfDocument$lambda-11(PdfCreationTest.kt:151)
[task 2023-02-08T01:02:47.911Z] 01:02:47     INFO -  org.mozilla.geckoview.test | 	at org.mozilla.geckoview.test.PdfCreationTest.$r8$lambda$8dLvIz-uQcP-K8MWnOjkWG_Ou9I(PdfCreationTest.kt)
[task 2023-02-08T01:02:47.911Z] 01:02:47     INFO -  org.mozilla.geckoview.test | 	at org.mozilla.geckoview.test.PdfCreationTest$$ExternalSyntheticLambda2.perform(D8$$SyntheticClass)
[task 2023-02-08T01:02:47.911Z] 01:02:47     INFO -  org.mozilla.geckoview.test | 	at androidx.test.core.app.ActivityScenario.lambda$onActivity$2$ActivityScenario(ActivityScenario.java:660)
[task 2023-02-08T01:02:47.911Z] 01:02:47     INFO -  org.mozilla.geckoview.test | 	at androidx.test.core.app.ActivityScenario$$Lambda$4.run(ActivityScenario.java:652)
[task 2023-02-08T01:02:47.911Z] 01:02:47     INFO -  org.mozilla.geckoview.test | 	at androidx.test.core.app.ActivityScenario.onActivity(ActivityScenario.java:670)
[task 2023-02-08T01:02:47.911Z] 01:02:47     INFO -  org.mozilla.geckoview.test | 	at org.mozilla.geckoview.test.PdfCreationTest.saveAPdfDocument(PdfCreationTest.kt:149)
[task 2023-02-08T01:02:47.911Z] 01:02:47     INFO -  org.mozilla.geckoview.test |
[task 2023-02-08T01:02:47.912Z] 01:02:47     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: current=500
[task 2023-02-08T01:02:47.912Z] 01:02:47     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS_CODE: -2
[task 2023-02-08T01:02:47.912Z] 01:02:47     INFO -  Printing logcat for test:
[task 2023-02-08T01:02:48.753Z] 01:02:48     INFO -  None
[task 2023-02-08T01:02:48.753Z] 01:02:48  WARNING -  TEST-UNEXPECTED-FAIL | org.mozilla.geckoview.test.PdfCreationTest#saveAPdfDocument | org.mozilla.geckoview.test.util.UiThreadUtils$TimeoutException: Timed out after 30000ms
[task 2023-02-08T01:02:48.753Z] 01:02:48     INFO -  TEST-INFO took 36774ms
Flags: needinfo?(cdenizet)

The added test was running correctly locally but maybe the pdf is bit large and the emulator a bit slow.
Consequently, I slightly changed the test to use a smaller pdf and made a try:
https://treeherder.mozilla.org/jobs?repo=try&revision=0fea7d40c197929263e5157e68b393bdd9ee7fb7&selectedTaskRun=OuOT4EhRQk-u-VD0JfMoBg.0

Flags: needinfo?(cdenizet)
Pushed by cdenizet@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/e972c072cb00 Get the bytes from pdf.js when saving a PDF in GeckoView r=geckoview-reviewers,ohall

https://hg.mozilla.org/mozilla-central/rev/e972c072cb00

Status: ASSIGNED → RESOLVED
Closed: 2 years ago
status-firefox111: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 111 Branch
Regressions: 1815872
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: