Closed Bug 997723 Opened 6 years ago Closed 5 years ago

Regression: Unable to "Save as PDF"

Categories

(Firefox for Android :: General, defect)

31 Branch
ARM
Android
defect
Not set

Tracking

()

RESOLVED WORKSFORME
Tracking Status
firefox29 --- unaffected
firefox30 --- unaffected
firefox31 --- affected
firefox32 --- affected

People

(Reporter: TeoVermesan, Assigned: james.gilbertson)

References

Details

(Keywords: regression, reproducible)

Attachments

(1 file)

Build: Firefox for Android 31.0a1 (2014-04-17)
Steps to reproduce:
1. Open a website
2. Go to Page -> Save as PDF

Expected results: The PDF file should be downloaded.
Actual results: There is no action.

Note: The yesterday's build (2014-04-16) is not affected
tracking-fennec: --- → ?
Keywords: reproducible
Summary: Unable to "Save as PDF" → Regression: Unable to "Save as PDF"
James, do you have time to help us investigate this?
Flags: needinfo?(james.gilbertson)
(In reply to :Margaret Leibovic from comment #2)
> James, do you have time to help us investigate this?

I'll have time after today.
Flags: needinfo?(james.gilbertson)
Assignee: nobody → james.gilbertson
tracking-fennec: ? → 31+
BrowserApp.saveAsPDF was using a nsIDownload as it's progress listener. This patch converts it to using a custom nsIWebProgressListener, modeled on DownloadNotifications.jsm.

The notification is a bit wonky right now (at least for me in the emulator), but the PDF is saved.

This leaves me with the question of how to actually access the PDF once it's been saved. I suspect that the download manager was being used for that purpose, since otherwise I'm not sure why it was involved. If that's case, I can add code to register the PDF once it has been saved.
Attachment #8410113 - Flags: feedback?(wjohnston)
Comment on attachment 8410113 [details] [diff] [review]
WIP - Convert BrowserApp.saveAsPDF from nsIDownload

Review of attachment 8410113 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/android/chrome/content/browser.js
@@ +1082,5 @@
> +    Downloads.getSystemDownloadsDirectory()
> +             .then(d => OS.Path.join(d, fileName))
> +             .then(p => OS.File.openUnique(p, { humanReadable: true }))
> +             .then(r => {
> +               printSettings.toFileName = r.path;

Haven't had a chance to test, but it doesn't look like this creates an entry in the DownloadsManager. I think we want that so that there's a way to get back to this file.

::: mobile/android/modules/PrintNotifications.jsm
@@ +20,5 @@
> +const strings = Services.strings.createBundle("chrome://browser/locale/browser.properties");
> +
> +const CANCEL_BUTTON = {
> +  buttonId: "cancel",
> +  // TODO: localize?

I'm fine reusing this string. Its unlikely users would have much time to cancel anyway...
Attachment #8410113 - Flags: feedback?(wjohnston) → feedback+
Status: NEW → ASSIGNED
Since these bugs are on Aurora, we need to move to get them fixed soon. I played with this a bit today, but didn't find any way to add things to the download lists that weren't real downloads. i.e. You can't do things like write a custom Saver, or just add a Download object to the list. Paolo, can you provide any guidence on how you'd like this fixed?
Flags: needinfo?(paolo.mozmail)
(In reply to Wesley Johnston (:wesj) from comment #6)
> Since these bugs are on Aurora, we need to move to get them fixed soon. I
> played with this a bit today, but didn't find any way to add things to the
> download lists that weren't real downloads. i.e. You can't do things like
> write a custom Saver, or just add a Download object to the list. Paolo, can
> you provide any guidence on how you'd like this fixed?

The current approach would be to create an instance of nsITransfer from "@mozilla.org/transfer;1", similarly to what we do when saving web pages:

http://mxr.mozilla.org/mozilla-central/source/toolkit/content/contentAreaUtils.js#386

This creates a new Download object that uses DownloadLegacySaver. nsITransfer inherits from nsIWebProgressListener, though I'm not sure whether you can pass it directly to the "print" method or you need to provide an intermediate object to process the notifications.
Flags: needinfo?(paolo.mozmail)
"Save as PDF" works for me on latest Nightly (05/19) on LG Nexus 4 (Android 4.4.2)
tracking-fennec: 31+ → ---
This works for me on Aurora (31).
This should be fixed with the patches that landed in bug 901360.

Aaron, can you verify the new patches in bug 901360 didn't cause a regression here? If things are looking good, we can just close this out.
Flags: needinfo?(aaron.train)
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Flags: needinfo?(aaron.train)
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.