Add the possibility to save a PDF
Categories
(Fenix :: General, enhancement)
Tracking
(firefox110 wontfix, firefox111 fixed)
People
(Reporter: calixte, Assigned: calixte)
References
Details
Attachments
(2 files)
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 ?
|
Assignee | |
Updated•2 years ago
|
|
||
Updated•2 years ago
|
|
||
Comment 1•2 years ago
|
||
(In reply to Calixte Denizet (:calixte) from comment #0)
Right now, if we use the
Save as PDFfeature, 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.
|
|
Assignee | |
Comment 2•2 years ago
|
||
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.
|
|
Assignee | |
Comment 3•2 years ago
|
||
|
|
Assignee | |
Comment 4•2 years ago
|
||
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 ?
|
|
||
Comment 5•2 years ago
|
||
(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 patchGeckoEngineSessiontoo.
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).
|
|
||
Comment 6•2 years ago
|
||
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.
|
||
Comment 7•2 years ago
|
||
|
||
Comment 9•2 years ago
|
||
Backed out for causing geckoview failures on PdfCreationTest
- backout: https://hg.mozilla.org/integration/autoland/rev/a304318cc0474d1360f0fa2bedd2b5eb5b9ff123
- push: https://treeherder.mozilla.org/jobs?repo=autoland&selectedTaskRun=XJGCI-IBSL-sOBF94LvpSA.0&revision=a20d64d6501c1027dcd0f80bb441d07bb40fe151
- failure log: https://treeherder.mozilla.org/logviewer?job_id=404947810&repo=autoland&lineNumber=10049
[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
|
|
Assignee | |
Comment 10•2 years ago
|
||
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
|
||
Comment 11•2 years ago
|
||
|
||
Comment 12•2 years ago
|
||
| bugherder | ||
|
||
Updated•2 years ago
|
| Comment hidden (collapsed) |
Description
•