Implement PDF printing for "Save As PDF" on Android



9 years ago
7 years ago


(Reporter: mbrubeck, Assigned: blassey)


Bug Flags:
in-litmus +



(2 attachments, 5 obsolete attachments)



9 years ago
With the patch for bug 595737 applied, saving a page as PDF on Android results in the following exception being thrown from the "Browser:SaveAs" handler in content.js:

CC[';1'] is undefined


9 years ago
tracking-fennec: --- → ?

Comment 1

9 years ago
Created attachment 474774 [details] [diff] [review]

We need to add an PrintOptions factory to widget/src/nsWidgetFactory.cpp.  This attempt at a minimal implementation does not work yet.  Saving as PDF fails with NS_ERROR_FACTORY_NOT_REGISTERED in newPrintSettings.

Comment 2

9 years ago
Created attachment 475440 [details] [diff] [review]

Still missing a few required factories.
Assignee: nobody → mbrubeck
Attachment #474774 - Attachment is obsolete: true

Comment 3

9 years ago
Created attachment 475506 [details] [diff] [review]

Getting farther.  This fails to build; looks like maybe the Cairo feature flags are not getting picked up for some reason:

/home/mbrubeck/src/mozilla/android/gfx/thebes/gfxPDFSurface.cpp:60: error: '_moz_cairo_pdf_surface_create_for_stream' was not declared in this scope
Attachment #475440 - Attachment is obsolete: true

Comment 4

9 years ago
Created attachment 475531 [details] [diff] [review]

This builds and runs, but the PDF it produced was blank.
Attachment #475506 - Attachment is obsolete: true


9 years ago
Summary: "Save As PDF" fails on Android: CC[';1'] is undefined → Implement PDF printing for "Save As PDF" on Android
tracking-fennec: ? → 2.0b2+
tracking-fennec: 2.0b2+ → 2.0+
Note to tester : check Bug 569497 along with this.

Comment 6

9 years ago
This won't use the file picker from bug 569497.  The Android file picker is for uploads only.  Save As PDF will save directly to the default download folder (bug 595737).
Assignee: mbrubeck → blassey.bugs
tracking-fennec: 2.0+ → 2.0b3+
Created attachment 490527 [details] [diff] [review]

this works for printing from the chrome process. However, the content process in Fennec tries to save the pdf to the sdcard's downloads folder, which it doesn't have permissions for, so that fails at the end.

Gotta figure out what to do with that last bit
Attachment #475531 - Attachment is obsolete: true
Can the content process save the file to any location? If so, we could save it somewhere temporarily in the content process, send back the file name to chrome, and then do a final move.

Comment 9

8 years ago
Do we just need to ask for permissions to that folder?
Created attachment 490713 [details] [diff] [review]
m-b patch
Attachment #490713 - Flags: review?(mark.finkle)
Attachment #490527 - Flags: review?(mwu)
Comment on attachment 490713 [details] [diff] [review]
m-b patch

>diff --git a/chrome/content/browser-ui.js b/chrome/content/browser-ui.js

>+        var appInfo = Cc[";1"];

Looks like this is not needed

>+        let tmpDir = 
>+          Components.classes[";1"]
>+          .getService(Components.interfaces.nsIProperties)
>+          .get("TmpD", Components.interfaces.nsIFile);  

Use Cc and Ci and it will work OK on a single line. I know the platform guys are line-wrappers :)

>+        let tmpFile = tmpDir.clone();
>+        tmpFile.append(dlFile.leafName);
>+        // we sometimes race with the content process, so make sure its finished
>+        // creating/writing the file
>+        while (!tmpFile.exists());

Add a blank line before the comment

>@@ -1333,6 +1351,8 @@ var PageActions = {
> #ifdef ANDROID

// Create the final destination file location

>     let file = downloadsDir.clone();
>     file.append(fileName);
>+    file.createUnique(file.NORMAL_FILE_TYPE, 0x666);

// The filename is used below to save the file to a temp location in the content process. Make sure it's up to date.

>+    fileName = file.leafName;

Add some comments so we remember what's happening here (use my examples if you want)

>     Services.obs.notifyObservers(download, "dl-start", null);
>+#ifdef ANDROID

Keep the blank line. I like having some separation in the code. Makes it easier to read.

>+    let tmpDir = Components.classes[";1"]
>+                   .getService(Components.interfaces.nsIProperties)
>+                   .get("TmpD", Components.interfaces.nsIFile);

Cc and Ci and one line

>+    file.append(fileName);
>     let data = {
>       type: Ci.nsIPrintSettings.kOutputFormatPDF,

Put the blank line outside the #endif

r+ with nits. Yay, we have PDF on Android! Thanks guys!
Attachment #490713 - Flags: review?(mark.finkle) → review+
Created attachment 490799 [details] [diff] [review]
m-b patch for checkin
Attachment #490713 - Attachment is obsolete: true
Attachment #490799 - Flags: review+
Comment on attachment 490527 [details] [diff] [review]

Sorry I don't feel qualified enough to review this...
Attachment #490527 - Flags: review?(mwu) → review?(vladimir)
Comment on attachment 490527 [details] [diff] [review]

this looks reasonable to me, though would be nice to have a MOZ_PDF_PRINTING define somewhere instead of doing:

-#if defined(MOZ_ENABLE_GTK2) || defined(XP_WIN) || defined(XP_OS2) || defined(MOZ_WIDGET_QT)
+#if defined(MOZ_ENABLE_GTK2) || defined(XP_WIN) || defined(XP_OS2) || defined(MOZ_WIDGET_QT) || defined (ANDROID)

I also think that there's another place where this needs to be added, but I can't find it now.. hm.  If it works then it's probably ok!
Attachment #490527 - Flags: review?(vladimir) → review+
pushed and
Last Resolved: 8 years ago
Resolution: --- → FIXED
verified FIXED on build:
Mozilla/5.0 (Android; Linux armv71; rv:2.0b8pre) Gecko/20101117 Namoroka/4.0b8pre Fennec/4.0b3pre
Flags: in-litmus?

Comment 17

7 years ago
Testcases update in litmus:
Flags: in-litmus? → in-litmus+
You need to log in before you can comment on or make changes to this bug.