405720 - Strange message with download manager for file with unknown filesize

Status: VERIFIED FIXED

(Toolkit :: Downloads API, defect, P1)

Description

[JD]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b2pre) Gecko/2007112413 Firefox/3.0b2pre (Swiftfox)
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b2pre) Gecko/2007112413 Firefox/3.0b2pre (Swiftfox)

I just downloaded A file with an unknown filesize and the Download Manager responded strangely. In the titlebar for the download manager it said: "100% of 1 file" the whole time the file was downloading. I'm quite sure it's not this way on Firefox 2, and i'm just letting you know...I'll upload a screenshot so you can see what was going on.

Reproducible: Always

Steps to Reproduce:
1. Go to http://deluge-torrent.org/downloads-ubuntu and select the Ubuntu 7.10 i386 deb download option.
2. Look at the titlebar of the download manager when the download is ongoing
3.

Comment 1

[JD]

Attachment: screenshot of what happens with this bug

Comment 2

[Stephen Donner [:stephend] Not actively reading bugmail]

Yeah, I can totally reproduce with current trunk on Mac; JD, thanks for the bug report!

Comment 3

[Ed Lee :Mardak]

What should this be? In firefox 2, it just displayed "Downloads" in the title for the unknown file size transfer.

Comment 4

[Ed Lee :Mardak]

It seems like for firefox 2, it'll report the number of downloads only if there's a reasonable percentage to show.

So..

1 active unknown transfer => "Downloads"
1 active 30% => "30% of 1 downloads"
1 active 30% + unknown => "30% of 2 downloads"
2 active unknown => "Downloads"

I suppose we could have 3 strings: static title, # downloads, % and # downloads
"Downloads"
"5 downloads"
"30% of 2 downloads"


I suppose this could make use of the "better time" (bug 394516) plural handling for download vs downloads...

Comment 5

[JD]

(In reply to comment #4)
> It seems like for firefox 2, it'll report the number of downloads only if
> there's a reasonable percentage to show.
> 
> So..
> 
> 1 active unknown transfer => "Downloads"
> 1 active 30% => "30% of 1 downloads"
> 1 active 30% + unknown => "30% of 2 downloads"
> 2 active unknown => "Downloads"
> 
> I suppose we could have 3 strings: static title, # downloads, % and # downloads
> "Downloads"
> "5 downloads"
> "30% of 2 downloads"

Sounds good to me. I wish there was a bit better way to do that, but i honestly can't think of one so....

Comment 6

[Ed Lee :Mardak]

Ack. This will need string changes. How does l10n-late work?

Currently there's 2 strings that need to get changed to maybe.. 2 strings
downloadsTitle=%S%% of 1 file - Downloads
downloadsTitleMultiple=%S%% of %S files - Downloads

to
downloadsTitleFiles=One file - Downloads;#1 files - Downloads
downloadsTitlePercent=#2% of one file - Downloads;#2% of #1 files - Downloads

Something along those lines.. at least it fixes up a poorly plural localizable string.

Comment 7

[Ed Lee :Mardak]

Attachment: v1

I went with the number 1 instead of One.. because it looked strange. :p

"One file - Downloads" as a title

Comment 8

[Shawn Wilsher :sdwilsh]

Comment on attachment 299424 [details] [diff] [review]
v1

>+  // Use the default title and reset "last" values if there's no downloads
>+  if (numActiveDownloads == 0) {
nit: 0 == numActiveDownloads

>+  // Calculate the percent transferred, unless we don't have a total file size
>+  let title = gStr.downloadsTitlePercent;
>+  if (base == 0)
nit: 0 == base

r=sdwilsh

Comment 9

[Ed Lee :Mardak]

Comment on attachment 299424 [details] [diff] [review]
v1

Requesting string change approval for blocking bug.

Comment 10

[Ed Lee :Mardak]

Checking in toolkit/locales/en-US/chrome/mozapps/downloads/downloads.properties;
/cvsroot/mozilla/toolkit/locales/en-US/chrome/mozapps/downloads/downloads.properties,v  <--  downloads.properties
new revision: 1.22; previous revision: 1.21
done
Checking in toolkit/mozapps/downloads/content/downloads.js;
/cvsroot/mozilla/toolkit/mozapps/downloads/content/downloads.js,v  <--  downloads.js
new revision: 1.133; previous revision: 1.132
done

Comment 11

[JD]

Thanks for getting this merged in, guys!

Comment 12

[Ed Lee :Mardak]

Forgot to Cu.import("resource://gre/modules/PluralForm.jsm");

Checking in toolkit/mozapps/downloads/content/downloads.js;
/cvsroot/mozilla/toolkit/mozapps/downloads/content/downloads.js,v  <--  downloads.js
new revision: 1.135; previous revision: 1.134
done

(But I don't seem to get the problem.. so.. ?)

I suppose we also forgot to delete downloadsTitle and downloadsTitleMultiple.

Comment 13

[Axel Hecht [:Pike]]

Note, removing unused strings is fine, as that's not breaking localizations.

Comment 14

[Shawn Wilsher :sdwilsh]

Edward - let's get a patch to remove those then.

Comment 15

[Ed Lee :Mardak]

Attachment: followup v1

Comment 16

[Shawn Wilsher :sdwilsh]

Comment on attachment 299566 [details] [diff] [review]
followup v1

r=sdwilsh

Comment 17

[Reed Loden [:reed]]

Drivers don't see approval requests in closed bugs. :(

Comment 18

[Ed Lee :Mardak]

Checking in toolkit/locales/en-US/chrome/mozapps/downloads/downloads.properties;
/cvsroot/mozilla/toolkit/locales/en-US/chrome/mozapps/downloads/downloads.properties,v  <--  downloads.properties
new revision: 1.23; previous revision: 1.22
done

Comment 19

[Henrik Skupin [:whimboo][⌚️UTC+1]]

I just wanted to verify this bug but I'm not able to find a download which states an unknown file size. Anyone has a link?

Comment 20

[Stephen Donner [:stephend] Not actively reading bugmail]

(In reply to comment #19)
> I just wanted to verify this bug but I'm not able to find a download which
> states an unknown file size. Anyone has a link?

Yeah; http://www.mp3.com/free-music/free-mp3s.  Just single-click on the orange "mp3" button and quickly save the file (I'm on a fast connection, and due to the way we start saving the file to disk immediately even before we've chosen a destination, you have to be quick with this).

Comment 21

[Al Billings [:abillings - ex-MoCo]]

Verified in Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9pre) Gecko/2008050904 Minefield/3.0pre.