78605 - display bytes [size] in Status when download is complete

Status: RESOLVED FIXED

(Core Graveyard :: File Handling, defect, P2)

Description

[sairuh (rarely reading bugmail)]

it would be useful to show the amount of bytes transferred [file size] when
download is finished. this info would appear in the Status field, along with the
total elapsed time.

Comment 1

[sairuh (rarely reading bugmail)]

spam: over to File Handling. i have not changed the assigned developer [or the
other fields for that matter], so if anyone realizes that a bug should have a
more appropriate owner, go ahead and change it. :)

Comment 2

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: screenshot

ok, I have a patch which makes the dialog look like this when the download is
finished

I'll attach the patch in a second

Comment 3

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: patch

Comment 4

[Christian :Biesinger (don't email me, ping me on IRC)]

taking bug

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 113950 [details] [diff] [review]
patch

Do we want to report the size of the resulting file?  Or the number of bytes we
actually got off the network (think streaming decompression and such)?

Comment 6

[Christian :Biesinger (don't email me, ping me on IRC)]

Hmm. Note that comment 0 is not clear on this :)

I vote for the actual filesize, because this is easier to implement, and it's
what I guess users expect (Why did the dialog show 200 KB when the file is
actually 400 KB large?)

Comment 7

[Boris Zbarsky [:bzbarsky]]

On the other hand, as the download progresses we show bytes downloaded off the
network.  So you'll see things like "200kb, 99%" and then "400 kb downloaded".

Comment 8

[Christian :Biesinger (don't email me, ping me on IRC)]

hmm, you could make an argument for both possibilities I guess.

I propose we make this a pref. *ducks*

just kidding.

I guess it is better to not suddenly jump to a much larger number, then. So I
would store the aCurTotalProgress from onProgressChange in a member variable and
use that in setCompleted?

Comment 9

[Boris Zbarsky [:bzbarsky]]

yeah, that may be better.... I'd try both and see how they look.

Comment 10

[Christian :Biesinger (don't email me, ping me on IRC)]

all that just for a comparatively rare case...

Do you have an url which sends compressed data?

Comment 11

[Boris Zbarsky [:bzbarsky]]

http://cvs.php.net/co.php/php4/ext/pgsql/pgsql.c?r=1.141&p=1 for example. 

Comment 12

[Christian :Biesinger (don't email me, ping me on IRC)]

And, er, is onProgressChange sometimes not called?

Comment 13

[Boris Zbarsky [:bzbarsky]]

No idea.  I've never really figured out the whole progress notifications cabal.

Comment 14

[Christian :Biesinger (don't email me, ping me on IRC)]

sigh... ok... on the url you mentioned, the content length is unknown. that
means that in the end, onProgressChange gets -1 for the progress (for all 4
progresses). which is of course not useful.

I'm more and more inclined to just stay with the attached patch.

Comment 15

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 113950 [details] [diff] [review]
patch

Sorry, no.  We get the same problem with binhex files; there the content-length
off the server _is_ known and is _not_ equal to the filesize on disk.

As in, if we're going to add UI, let's do it in a way that won't get bugs filed
on it immediately...

Comment 16

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 113950 [details] [diff] [review]
patch

I was wrong, it seems.	This should be good.

Comment 17

[jag (Peter Annema)]

Comment on attachment 113950 [details] [diff] [review]
patch

sr=jag

Comment 18

[Christian :Biesinger (don't email me, ping me on IRC)]

Checking in locale/en-US/nsProgressDialog.dtd;
/cvsroot/mozilla/embedding/components/ui/progressDlg/locale/en-US/nsProgressDial
og.dtd,v  <--  nsProgressDialog.dtd
new revision: 1.8; previous revision: 1.7
done

Comment 19

[Christian :Biesinger (don't email me, ping me on IRC)]

hm, cvs didn't check in the other file before... now done:
Checking in nsProgressDialog.js;
/cvsroot/mozilla/embedding/components/ui/progressDlg/nsProgressDialog.js,v  <--
 nsProgressDialog.js
new revision: 1.31; previous revision: 1.30
done

Comment 20

[[e-mail]]

Comment on attachment 113949 [details]
screenshot

Whose idea was it to duplicate the elapsed time?

Comment 21

[Christian :Biesinger (don't email me, ping me on IRC)]

(In reply to comment #20)
> (From update of attachment 113949 [details] [edit])
> Whose idea was it to duplicate the elapsed time?

that was not added by this patch. in fact, rev. 1.1 of that dtd already has the
elapsed time in this text, and presumably also has the separate elapsed time
display. I did not check further to find the earlier version of the file.