Closed Bug 589725 Opened 10 years ago Closed 9 years ago

Fennec should support showing remain time for download.

Categories

(Firefox for Android Graveyard :: General, defect)

x86
Linux
defect
Not set

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: junmin.zhu, Unassigned)

Details

Attachments

(1 file, 4 obsolete files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.8) Gecko/20100722 Firefox/3.6.8 ( .NET CLR 3.5.30729)
Build Identifier: 

After you make sure to download, the download management not only provide the progress bar, but also provide approximated remaining time.

Reproducible: Always

Steps to Reproduce:
1.launch the fennec
2.switch to download management panel

Actual Results:  
if downloading something, download management do not provide remaining time.

Expected Results:  
download management should provide remaining time for downloading item.
here is a patch, based on mobile tip: af67cbcea09d
Is it possible to use the helper function from DownloadUtils instead of rewriting one?

http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/downloads/DownloadUtils.jsm#242
Status: UNCONFIRMED → NEW
Ever confirmed: true
I think it's possible. btw, it's that means this feature will be accepted into trunk after implement?
Comment on attachment 468244 [details] [diff] [review]
patch for showing remain time for download.

>diff --git a/chrome/content/downloads.js b/chrome/content/downloads.js

>+  _convertTime: function dv__convertTime(time) {

As Vivien said, try to use the code in DownloadUtils.jsm

>+    let Time;

let time;  (use lowercase)


>diff --git a/locales/en-US/chrome/browser.properties b/locales/en-US/chrome/browser.properties

> # Download Manager
> # LOCALIZATION NOTE (Status): â is the "em dash" (long dash)
> # #1 download size for FINISHED or download state; #2 host (e.g., eTLD + 1, IP)

Update the comment to describe what #3 is for

>-downloadsStatus=#1 â #2
>+downloadsStatus=#1 â #2 â #3

You need to rename the entity, downloadsStatus, to something else so our l10n tools know a string was changed and will notify the localizers.

downloadsStatus2


I am not 100% sure we need to display the time information. There is a progressbar to help see how fast the download is moving and how much of it remains.

Why do you want to see the time information?
Also, make sure you edit the properties file using UTF-8, so any special characters are not lost.
(In reply to comment #4)

> 
> I am not 100% sure we need to display the time information. There is a
> progressbar to help see how fast the download is moving and how much of it
> remains.
> 
> Why do you want to see the time information?

Thanks for your comment, I think progress bar can only provide download percent, not the remain time, because remain time is strongly related to net speed.

Further more, remain time can give user a approximated time, let user free to do other things, and back to check download result in remaining time.
According to Vivien and Mark's comment, refined the patch.
based on mobile tip: b2bdb51a4527
Is the "remaining time" number reliable?
(In reply to comment #8)
> Is the "remaining time" number reliable?

As much as it could be with computer, it varies depending on network speed. So we can consider it to be reliable if you don't move too much.
Attachment #470394 - Flags: review?(mark.finkle)
Attachment #468244 - Attachment is obsolete: true
Refine this patch. Based on mobile tip:d4f1702e7903
Attachment #470394 - Attachment is obsolete: true
Attachment #483091 - Flags: review?(mark.finkle)
Attachment #470394 - Flags: review?(mark.finkle)
Because we use "——" for downloadsTime to differentiate with downloadsStatus
So I think we still need downloadsTime string.

In the last patch, I miss use "——" in the code file not in the localization file.
So I refine the patch. Based on mobile tip:d4f1702e7903.

Finkle, would you help to review?
Attachment #483091 - Attachment is obsolete: true
Attachment #483101 - Flags: review?(mark.finkle)
Attachment #483091 - Flags: review?(mark.finkle)
Comment on attachment 483101 [details] [diff] [review]
patch for showing left time for download

>diff --git a/chrome/content/downloads.js b/chrome/content/downloads.js

>+    let time = "";
>+    let totalTime;
>+    let leftTime;

These are only used inside the "if" block, so let's just declare them inside, when they are used.
Attachment #483101 - Flags: review?(mark.finkle) → review+
Whiteboard: [fennec-checkin-postb2]
As mark's comment, declare variable in 'if' block.
and simplify the patch.
Attachment #483716 - Flags: review?(mark.finkle)
Comment on attachment 483716 [details] [diff] [review]
patch for showing left time for download

We need to wait until after beta 2 for this to land.
Attachment #483716 - Flags: review?(mark.finkle) → review+
Attachment #483101 - Attachment is obsolete: true
(In reply to comment #14)
> Comment on attachment 483716 [details] [diff] [review]
> patch for showing left time for download
> 
> We need to wait until after beta 2 for this to land.

ok, thanks
pushed:
http://hg.mozilla.org/mobile-browser/rev/a2abeaf75024
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [fennec-checkin-postb2]
Verified; pause, cancel, delete, retry tested:

Mozilla/5.0 (Maemo;Linux armv71; rv:2.0b8pre) Gecko/20101104 Firefox/4.0b8pre Fennec/4.0b3pre
Mozilla/5.0 (Android; Linux armv71; rv2.0b8pre) Gecko/20101104 Firefox/4.0b8pre Fennec/4.0b3pre
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.