save as pdf does not provide status notification

VERIFIED FIXED

Status

()

Firefox for Android
General
--
major
VERIFIED FIXED
6 years ago
6 years ago

People

(Reporter: lmandel, Assigned: Margaret)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [testday-20111111])

Attachments

(1 attachment, 1 obsolete attachment)

When I click Save as PDF a new download item appears in my notifications bar. Typically, download notifications contain a status bar that indicates progress. The Save as PDF download does not have a status bar. This notification also does not disappear until I remove it manually.
(Assignee)

Comment 1

6 years ago
(In reply to Lawrence Mandel [:lmandel] from comment #0)
> When I click Save as PDF a new download item appears in my notifications
> bar. Typically, download notifications contain a status bar that indicates
> progress. The Save as PDF download does not have a status bar.

Since PDFs are small, the download probably completes before you get the change to see the progress indicator.

> This notification also does not disappear until I remove it manually.

This is bug 696911.
(In reply to Margaret Leibovic [:margaret] from comment #1)
> > This notification also does not disappear until I remove it manually.
> 
> This is bug 696911.

To be clear, in my case the PDF did not open on completion and the downloading notification was still listed on my phone.
Assignee: nobody → madhava

Comment 3

6 years ago
(In reply to Lawrence Mandel [:lmandel] from comment #2)
> (In reply to Margaret Leibovic [:margaret] from comment #1)
> > > This notification also does not disappear until I remove it manually.
> > 
> > This is bug 696911.
> 
> To be clear, in my case the PDF did not open on completion and the
> downloading notification was still listed on my phone.

This is very easy to reproduce.
In other words, if I was to file this bug it would be "save as PDF is 99% broken". Being 1% that it has told me that it has downloaded without being able to view it/open it.
Severity: normal → major
Brian - What's the deal here? Once the download completes, we should open the pDF, or at least open when you tap the system notification.
Assignee: madhava → bnicholson
(Assignee)

Comment 5

6 years ago
(In reply to Mark Finkle (:mfinkle) from comment #4)
> Brian - What's the deal here? Once the download completes, we should open
> the pDF, or at least open when you tap the system notification.

It seems like there is a problem with opening PDFs from the download completion notification. I found that if I tap on a notification for a PDF download from the web, it also does not open, but tapping on a notification for an APK download does the right thing.
(Assignee)

Comment 6

6 years ago
Looking into this more, I found that when the save as PDF code got ported over from XUL fennec, the "Browser:SaveAs:Return" part was left out, which handled notifying observers that the download was done (http://mxr.mozilla.org/mozilla-central/source/mobile/xul/chrome/content/browser-ui.js#1060).

Brian, if you don't mind, I'm just going to take this bug, since I think I know how to fix it.
Assignee: bnicholson → margaret.leibovic
(Assignee)

Updated

6 years ago
Duplicate of this bug: 701798
(Assignee)

Comment 8

6 years ago
Created attachment 576093 [details] [diff] [review]
patch (use download manager API in saveAsPDF)

Instead of executing our own queries against the downloads database, we should just be using the download manager API. With this change, the download progress notifications work as though this were a normal download (a progress bar now actually shows up in the notification!).
Attachment #576093 - Flags: review?
(Assignee)

Updated

6 years ago
Attachment #576093 - Flags: review? → review?(mark.finkle)
(Assignee)

Comment 9

6 years ago
Created attachment 576094 [details]
patch to fix progress notifications for printing

In order to use the download manager to track printing status, we need to fire STATE_IS_NETWORK notifications, since that's what the download manager expects. I'm not sure if this fix is hacky or not, but it doesn't look like it will cause any trouble.
Attachment #576094 - Flags: review?(bzbarsky)
Comment on attachment 576094 [details]
patch to fix progress notifications for printing

Review of attachment 576094 [details]:
-----------------------------------------------------------------

::: layout/printing/nsPrintData.cpp
@@ +138,1 @@
>      DoOnProgressChange(0, 0, true, nsIWebProgressListener::STATE_START|nsIWebProgressListener::STATE_IS_DOCUMENT);

Untested, but could these changes just use another bitwise-or on the current |DoOnProgressChange| call? For example:

using namespace nsIWebProgressListener;
DoOnProgressChange(0, 0, true, STATE_START|STATE_IS_DOCUMENT|STATE_IS_NETWORK);
When a webpage is saved as PDF and only the Downloading notification was displayed, the PDF file is created in (local)/download, but its size is 0 Kb. If the app is closed and reopened and if a any webpage is opened and is saved as PDF then the first saved PDF file will change its size to the double of the normal size. Also the first PDF file could not be opened. 
Because of this bug, if two or more webpages are saved as PDF, Fennec will download 2 ore more files with the same content, but different names (PDF file names are saved accordingly to the opened webpages).
Heh. I did this same thing for XUL Fennec once (bug 646262), but distracted myself with rewriting the Download Manager API and never finished it.
Attachment #576093 - Flags: review?(mark.finkle) → review+
(Assignee)

Comment 13

6 years ago
(In reply to Jared Wein [:jwein and :jaws] from comment #10)
> Comment on attachment 576094 [details] [review]
> patch to fix progress notifications for printing
> 
> Review of attachment 576094 [details] [review]:
> -----------------------------------------------------------------
> 
> ::: layout/printing/nsPrintData.cpp
> @@ +138,1 @@
> >      DoOnProgressChange(0, 0, true, nsIWebProgressListener::STATE_START|nsIWebProgressListener::STATE_IS_DOCUMENT);
> 
> Untested, but could these changes just use another bitwise-or on the current
> |DoOnProgressChange| call? For example:
> 
> using namespace nsIWebProgressListener;
> DoOnProgressChange(0, 0, true,
> STATE_START|STATE_IS_DOCUMENT|STATE_IS_NETWORK);

I haven't tested this either, but I think that this doesn't really make given the semantics of these flags, and I couldn't find anywhere else where we do something like this. I realize what we're doing here is already kind of a hack, but at least we can make it a semantically correct hack :)
(Assignee)

Comment 14

6 years ago
(In reply to Margaret Leibovic [:margaret] from comment #13)

> I haven't tested this either, but I think that this doesn't really make
> given the semantics of these flags...

s/make/make sense/
(In reply to Margaret Leibovic [:margaret] from comment #13)
> (In reply to Jared Wein [:jwein and :jaws] from comment #10)
> > Comment on attachment 576094 [details] [review] [diff] [details] [review]
> > patch to fix progress notifications for printing
> > 
> > Review of attachment 576094 [details] [review] [diff] [details] [review]:
> > -----------------------------------------------------------------
> > 
> > ::: layout/printing/nsPrintData.cpp
> > @@ +138,1 @@
> > >      DoOnProgressChange(0, 0, true, nsIWebProgressListener::STATE_START|nsIWebProgressListener::STATE_IS_DOCUMENT);
> > 
> > Untested, but could these changes just use another bitwise-or on the current
> > |DoOnProgressChange| call? For example:
> > 
> > using namespace nsIWebProgressListener;
> > DoOnProgressChange(0, 0, true,
> > STATE_START|STATE_IS_DOCUMENT|STATE_IS_NETWORK);
> 
> I haven't tested this either, but I think that this doesn't really make
> given the semantics of these flags, and I couldn't find anywhere else where
> we do something like this. I realize what we're doing here is already kind
> of a hack, but at least we can make it a semantically correct hack :)

Maintaining semantic correctness would probably entail that we don't state the progress has changed from 100 to 100 as well (same for 0 to 0).

(In reply to Cristian Nicolae (:xti) from comment #11)
> When a webpage is saved as PDF and only the Downloading notification was
> displayed, the PDF file is created in (local)/download, but its size is 0
> Kb. If the app is closed and reopened and if a any webpage is opened and is
> saved as PDF then the first saved PDF file will change its size to the
> double of the normal size. Also the first PDF file could not be opened. 
> Because of this bug, if two or more webpages are saved as PDF, Fennec will
> download 2 ore more files with the same content, but different names (PDF
> file names are saved accordingly to the opened webpages).

Cristian: Did you see this behavior with or without applying these patches?
(Assignee)

Comment 16

6 years ago
I landed the first patch because it definitely improves save as PDF download notifications, so I didn't want to let it sit:

https://hg.mozilla.org/projects/birch/rev/ff5faa59c065

I'll keep this bug open until the second patch gets reviewed/lands. Without that patch, the download completed notification still won't appear, so that is a known issue.
(Assignee)

Updated

6 years ago
Depends on: 704691
(Assignee)

Comment 17

6 years ago
Comment on attachment 576094 [details]
patch to fix progress notifications for printing

I moved this patch to a follow-up bug (bug 704691).
Attachment #576094 - Attachment is patch: false
Attachment #576094 - Flags: review?(bzbarsky)
(Assignee)

Comment 18

6 years ago
(Marking this fixed, since the rest of the work can be done in the follow-up.)
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
(Assignee)

Updated

6 years ago
Attachment #576094 - Attachment is obsolete: true

Comment 19

6 years ago
When I save a file as PDF the only notification I get is the one with downloading the file. When opening the system notification there is a status bar for the PDF file but after the download is completed the notification disappears. Is this ok so far? And the rest will be done in the follow-up bug so I can mark this bug as verified?

Mozilla /5.0 (Android;Linux armv7l;rv:11.0a1) Gecko/20111124 Firefox/11.0a1 Fennec/11.0a1
Device: LG Optimus 2X (Android 2.2)
(In reply to Mark Finkle (:mfinkle) from comment #4)
> Brian - What's the deal here? Once the download completes, we should open
> the pDF, or at least open when you tap the system notification.

To follow-up on this comment (that I had previously missed), I don't expect or want the PDF to open automatically in this situation. I'm have already viewed the content in the browser.

Comment 21

6 years ago
I'm having several inconsistencies (20111125 build).

I have been able to hit these problems.

Problem A - only hit it once
* I click on "Save as PDF" and nothing is notified
* I check /sdcard/Downloads and nothing was there.
* I tried several times with several sites. It seems that closing the browser or having downloaded a normal file could have fixed it.

Problem B - I hit it all the time
* I click on "Save as PDF" and I see "Downloading ...pdf"
* When it finishes the "Download...pdf"  completely dissapears
** Normal downloads they stay there until I actually clear it
* When it finishes there is no message saying "...pdf has finished downloading"
** Normal downloads they have that

Not sure if bug 704691 is the follow up of problem B or not.

FTR I have not lost the the ability to download files.
(Assignee)

Comment 22

6 years ago
(In reply to Andreea Pod from comment #19)
> When I save a file as PDF the only notification I get is the one with
> downloading the file. When opening the system notification there is a status
> bar for the PDF file but after the download is completed the notification
> disappears. Is this ok so far? And the rest will be done in the follow-up
> bug so I can mark this bug as verified?

Yes, this will be fixed by bug 704691.

(In reply to Armen Zambrano G. [:armenzg] - Release Engineer from comment #21)

> Problem A - only hit it once
> * I click on "Save as PDF" and nothing is notified
> * I check /sdcard/Downloads and nothing was there.
> * I tried several times with several sites. It seems that closing the
> browser or having downloaded a normal file could have fixed it.

You should file a new bug if you can reproduce this.

> Problem B - I hit it all the time
> * I click on "Save as PDF" and I see "Downloading ...pdf"
> * When it finishes the "Download...pdf"  completely dissapears
> ** Normal downloads they stay there until I actually clear it
> * When it finishes there is no message saying "...pdf has finished
> downloading"
> ** Normal downloads they have that
> 
> Not sure if bug 704691 is the follow up of problem B or not.

Yes, it will fix that problem. See comment 16.

Comment 23

6 years ago
Marking this as verified fixed, build: Mozilla /5.0 (Android;Linux armv7l;rv:11.0a1) Gecko/20111127 Firefox/11.0a1 Fennec/11.0a1
Device: LG Optimus 2X (Android 2.2)
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.