Status

()

For bugs in Firefox Desktop, the Mozilla Foundation's web browser. For Firefox user interface issues in menus, developer tools, bookmarks, location bar, and preferences. Many Firefox bugs will either be filed here or in the Core product. (more info)

For issues dealing with helper applications, and guessing Content Types when they aren't specified/known (ftp:, file:, jar:, but generally not http:). This component does not cover: backend networking issues, such as those covered by Networking: FTP or Networking: File, nor does it cover the Download Manager which has its own component in the Toolkit product.

We're past string freeze at this point, so unless we can do this using existing strings, I won't be able to approve it.
I would also want the design to be:
which is a: 5.21 MB Binary File
which is a: 24 KB PDF
etc.

Yeah, alas we won't be able to take this for Firefox 4, but we can as soon as we ship 4 (And we're moving to faster release cycles, which means this could then be shipping in a release in as soon as a few months instead of a whole year).
Please also update to reflect to requested format in comment 7, and I think you'll also need to support the case of the file size being unknown (since not all servers provide that info).

Comment on attachment 506667[details][diff][review]
Updated Patch
sdwilsh: see last line! :p
>+++ b/toolkit/mozapps/downloads/src/nsHelperAppDlg.js.in 2011-01-25 14:39:52.820469641 +0800
Seems like nsHelperAppDlg.js.in no longer exists? (Seems to have gone ".in"-less")
http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/downloads/nsHelperAppDlg.js>+ showFileSize: function (bytes) {
>+ let [number, unit] = DownloadUtils.convertByteUnits(bytes)
>+ return number + " " + unit + " ";
String concatenation is a big no-no for localization. The existing strings that this dialog uses are here:
http://mxr.mozilla.org/mozilla-central/source/toolkit/locales/en-US/chrome/mozapps/downloads/unknownContentType.properties#40
Using the string format that you've got in the patch, it would look something like..
%2$S %3$S %1$S
Where the localization note indicates 1 is the filetype string, 2 is the size, 3 is units
But it should probably be something different as in the screenshot, it shows:
"which is a: Binary File, 5.21MB"
We could change the prefix string "which is a:" or just make sure to fit the existing string. So maybe
%1$S, %2$S %3$S
or %1$S (%2$S %3$S)
?
>@@ -624,7 +630,7 @@
>+ type.value = this.mLauncher.contentLength ? (this.showFileSize(this.mLauncher.contentLength) + typeString) : typeString;
Just pointing out if you didn't notice, just a few lines above this, there's a branch:
603 typeString = this.dialogElement("strings").getFormattedString("fileType", [primaryExtension.toUpperCase()]);
That'll convert something like "pdf" to "PDF file" according to the "fileType" string for the locale:
http://mxr.mozilla.org/mozilla-central/source/toolkit/locales/en-US/chrome/mozapps/downloads/unknownContentType.properties#51
The showFileSize might not be necessary and you could just do things inline while avoiding really long lines:
if (contentLength) {
let size = ..;
type.value = format(type, size, unit);
}
else
type.value = type;
sdwilsh: We don't have a good way for testing this? I suppose we could go ahead with the function so that it could be callable.. somehow.... Or maybe push it into DownloadUtils.jsm where DownloadUtils.getSizeAndType(numberOfBytes, MimeInfo) returns a string? That would be much easier to test.

(In reply to comment #16)
> Using the string format that you've got in the patch, it would look something
> like..
> %2$S %3$S %1$S
>
> But it should probably be something different as in the screenshot, it shows:
> "which is a: Binary File, 5.21MB"
Oh nevermind this comment. I see comment #7 and reading actual text instead of numbers, it makes more sense. ;)

(In reply to comment #21)
> swdlish: The UI for review shows:
> Binary File, 5.21 MB
> Whereas the patch I developed will show:
> 5.21 MB Binary File
>
> I created patch according to comment 7.
Can you upload an image with the correct output then please?

Comment on attachment 523847[details]
Screenshot of Show File Size Patch
Limi's currently on vacation, bouncing over to faaborg.
I suggest making it read slightly differently, though. Instead of:
which is a: 632 KB PDF document
change to
which is a: PDF document (634 KB)

I try to download a file which is 4,194,304 bytes big. This is exactly 4 MiB. Fx shows instead:
BIN file (4,0 MB)
IMHO Fx should use one of the two space characters between the number and the unit to indicate that "M" actually means 1024². Cf. http://en.wikipedia.org/wiki/Mebi#Adoption_by_IEC_and_NIST