593113 - Show progress bar in download notification

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Ayan Shah[:ashah]]

On the android browser, while downloading a file, we can see the progress bar for the download in the android status bar. In Fennec, right now we just show the notification that the file is downloading. It would be good to have the progress bar feature for Fennec as well. Also we can show the percentage of the file downloaded (e.g. Downloading... 40% of 5 MB )

Comment 2

[Alex Pakhotin (:alexp)]

I don't think this bug is a duplicate. Bug 592088 implements the API, but currently Fennec does not use that API to display the download progress - this is still a TODO.

Comment 3

[Alex Pakhotin (:alexp)]

Attachment: Fix

Added a download progress listener, which updates the notification.

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 480839 [details] [diff] [review]
Fix


>+  _progressSB: null,

progressAlert

>+    this._dlmgr = Cc["@mozilla.org/download-manager;1"].getService(Ci.nsIDownloadManager);
>+
>+#ifdef ANDROID
>+    this._progressSB = new StatusbarDownloadProgressListener();
>+    this._dlmgr.addListener(this._progressSB);
>+#endif

We don't want to move the download manager into the init method. We have it in the delayedInit method because it can slow things down

Can you move this to the code that displays the "start downloading" alert? if _dlmgr is null, set it and then just do what you are doing here, but do it there.


>   _delayedInit: function dv__delayedInit() {
 
>-    this._dlmgr = Cc["@mozilla.org/download-manager;1"].getService(Ci.nsIDownloadManager);
>     this._progress = new DownloadProgressListener();

Keep this one as a fallback

>+#ifdef ANDROID
>+// StatusbarDownloadProgressListener is used to display progress in status bar notifications.
>+function StatusbarDownloadProgressListener() { }

StatusbarDownloadProgressListener  ->  AlertDownloadProgressListener

r- for moving out of "init"

Comment 5

[Alex Pakhotin (:alexp)]

Attachment: Fix v2

Fixed review comments.

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 481284 [details] [diff] [review]
Fix v2

Nice. This tested out OK on Android I assume.

Comment 7

[Alex Pakhotin (:alexp)]

(In reply to comment #6)
> Nice. This tested out OK on Android I assume.

Sure! Works well on my Nexus One.

Comment 8

[Mark Finkle (:mfinkle) (use needinfo?)]

pushed:
http://hg.mozilla.org/mobile-browser/rev/110b5d1edaea

Comment 9

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Already a part of : 
https://litmus.mozilla.org/show_test.cgi?searchType=by_id&id=13648
https://litmus.mozilla.org/show_test.cgi?searchType=by_id&id=13647

Comment 10

[Cristian Nicolae (:xti)]

Verified fixed on:
Mozilla/5.0 (Android;Linux armv7l;rv:9.0a1)Gecko/20110915
Firefox/9.0a1 Fennec/9.0a1
Device: Samsung Galaxy S
OS: Android 2.2