Closed Bug 798280 Opened 7 years ago Closed 7 years ago

Save As PDF busted - (NS_ERROR_XPC_NOT_ENOUGH_ARGS @ nsIDownloadManager.addDownload)

Categories

(Firefox for Android :: General, defect)

ARM
Android
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 19
Tracking Status
firefox16 --- unaffected
firefox17 --- unaffected
firefox18 --- verified
firefox19 --- verified
fennec 18+ ---

People

(Reporter: ioana.chiorean, Assigned: bnicholson)

References

Details

(Keywords: regression)

Attachments

(2 files, 1 obsolete file)

Build ID: 18.0 (2012-10-04) Nightly Channel
Device: Samsung Galaxy Nexus
OS: Android 4.1

Steps to reproduce:
1. Open Fennec -> Go to gmail.com
2. Tap Menu -> Tools -> Save as PDF

Expected result:
- page should be saved as pdf

Actual result: 
- save as pdf doesn't trigger any action

Last build working : 
http://hg.mozilla.org/integration/mozilla-inbound/rev/31ae286fff78

First build not working: 
http://hg.mozilla.org/integration/mozilla-inbound/rev/9a71c92af5a4
E/GeckoConsole( 4384): [JavaScript Error: "NS_ERROR_XPC_NOT_ENOUGH_ARGS: Not enough arguments [nsIDownloadManager.addDownload]" {file: "chrome://browser/content/browser.js" line: 825}]
tracking-fennec: --- → ?
Keywords: regression
Summary: Save as pdf is not working → Save As PDF busted - (NS_ERROR_XPC_NOT_ENOUGH_ARGS @ nsIDownloadManager.addDownload)
Looks like the call to addDownload() for saveAsPDF() wasn't updated in bug 795065.
Assignee: nobody → bnicholson
Blocks: 795065
tracking-fennec: ? → 18+
Comment on attachment 670455 [details] [diff] [review]
Pass privacy status to addDownload() in saveAsPDF

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 795065
User impact if declined: save as pdf won't work
Testing completed (on m-c, etc.): 
Risk to taking this patch (and alternatives if risky): low risk
String or UUID changes made by this patch: none
Attachment #670455 - Flags: approval-mozilla-aurora?
Comment on attachment 670455 [details] [diff] [review]
Pass privacy status to addDownload() in saveAsPDF

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

>+    let isPrivate = BrowserApp.getTabForBrowser(aBrowser).isPrivate;

This might be faster?
  let isPrivate = aBrowser.docShell.QueryInterface(Ci.nsILoadContext).usePrivateBrowsing

>+  this.isPrivate = false;

I think we should make a getter for this. The property could change under us. I want it to be 'live':

get isPrivate() {
  if (this.browser)
    return this.browser.docShell.QueryInterface(Ci.nsILoadContext).usePrivateBrowsing;
  return false;
}

>-    let isPrivate = ("isPrivate" in aParams) && aParams.isPrivate;
>-    if (isPrivate) {
>+    this.isPrivate = ("isPrivate" in aParams) && (aParams.isPrivate == true);
>+    if (this.isPrivate) {
>       this.browser.docShell.QueryInterface(Ci.nsILoadContext).usePrivateBrowsing = true;
>     }

Reverting this should be OK with the getter

r+ with changes
Attachment #670455 - Flags: review?(mark.finkle) → review+
Isn't the privacy flag only on trunk?
http://hg.mozilla.org/integration/mozilla-inbound/rev/e15c81a5bf1d

(In reply to Ehsan Akhgari [:ehsan] from comment #6)
> Isn't the privacy flag only on trunk?

I changed the patch to just call aBrowser.docShell.QueryInterface() directly in saveAsPDF(), so this same patch should work on Aurora now.
(In reply to comment #7)
> http://hg.mozilla.org/integration/mozilla-inbound/rev/e15c81a5bf1d
> 
> (In reply to Ehsan Akhgari [:ehsan] from comment #6)
> > Isn't the privacy flag only on trunk?
> 
> I changed the patch to just call aBrowser.docShell.QueryInterface() directly in
> saveAsPDF(), so this same patch should work on Aurora now.

Oh, ok.  yeah that's better anyways.
(In reply to Brian Nicholson (:bnicholson) from comment #7)
> http://hg.mozilla.org/integration/mozilla-inbound/rev/e15c81a5bf1d

    1.12 +    let isPrivate = aBrowser.docShell.QueryInterface(Ci.nsILoadContext).usePrivateBrowsing;

Instead of doing this, can you please use PrivateBrowsingUtils?  You should do something like:

let isPrivate = PrivateBrowsingUtils.isWindowPrivate(aBrowser.currentWindow);

(assuming that currentWindow is a property that retrieves the current window object for the focused tab)
Attachment #670610 - Flags: review?(ehsan)
Comment on attachment 670610 [details] [diff] [review]
Follow-up for using PrivateBrowsingUtils

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

You need to import PrivateBrowsingUtils.jsm here.

Also, while you're here, would you mind converting over the other usage of the usePrivateBrowsing property here as well?

Thanks!
Attachment #670610 - Flags: review?(ehsan) → review-
Imported the PB jsm. Can't change the other usePrivateBrowsing since we're setting the flag there.
Attachment #670610 - Attachment is obsolete: true
Attachment #670635 - Flags: review?(ehsan)
Attachment #670635 - Flags: review?(ehsan) → review+
Whiteboard: [leave open]
Whiteboard: [leave open]
https://hg.mozilla.org/mozilla-central/rev/76071e66c36b

Should this have a test?
Status: NEW → RESOLVED
Closed: 7 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → Firefox 19
Status: RESOLVED → VERIFIED
(In reply to Ryan VanderMeulen from comment #15)
> https://hg.mozilla.org/mozilla-central/rev/76071e66c36b
> 
> Should this have a test?

It already has a testcase https://moztrap.mozilla.org/manage/cases/?filter-id=1024#caseversion-id-8619
Flags: in-testsuite? → in-testsuite+
in-testsuite refers to automated tests that run on checkin
Flags: in-testsuite?
Flags: in-testsuite+
Flags: in-qa-testsuite+
Attachment #670455 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Save as PDF is working now on Firefox 18.

--
Firefox 18.0a2 (2012-11-19)
Device: Galaxy S2
OS: Android 4.0.3
You need to log in before you can comment on or make changes to this bug.