70859 - Saving page gives "NaN:NaN" as elapsed time

Status: VERIFIED FIXED

(Core Graveyard :: File Handling, defect)

Description

[Oliver Klee]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; 0.9) Gecko/20010303
BuildID:    2001030308

If I save a page (e.g. the Mozilla start page), the download dialog displays
"NaN:NaN" as the elapsed time.

Reproducible: Always

Steps to Reproduce:
1. Visit any URL, e.g. the Mozilla site.
2. Press CTRL+S to save or use the file menu.
3. Enter file name

Actual Results:  Download dialog with "NaN:NaN" as the elapsed time.

Expected Results:  The Dialog should display the elapsed time in number form.

This behavior is rather new and not theme specific. I don't know if this is
influenced by the connection speed (I'm on a 768k ADSL line).

Comment 1

[Oliver Klee]

Adding regression keyword.

Comment 2

[Oliver Klee]

This only happens on HTML pages, not on binary content or FTP sites. The exact
text is:

"Download complete (elapsed time was NaN:NaN)"

Comment 3

[sairuh (rarely reading bugmail)]

->bill.

Comment 4

[sairuh (rarely reading bugmail)]

hrm, cannot repro this using 2001.03.05.09 opt comm bits on winnt. is this still
a problem for you?

Comment 5

[Oliver Klee]

This still is happening for me on 2001030620 on Win2k.

Comment 6

[sairuh (rarely reading bugmail)]

seems to be limited to win2k, then? terri also sees this.

bill, i cannot remember if you have win2k, but d'you see this?

Comment 7

[Bill Law]

I do have Win2k; I haven't seen this.  I do have another bug.

My theory is that if the download completes fast enough (before the dialog
appears), then this happens.

Comment 8

[Oliver Klee]

That makes sense. I usually get this bug with disk cache + memory cache turned
on. With both caches turned off, I get 00:00 as elapsed time for saving.

Comment 9

[sairuh (rarely reading bugmail)]

bill, what's the other bug you have on this? i'll go ahead and dup it if it's appropeiate. thx!

bryner sees this on linux, so marking all/all... in fact, i *think* i also recently saw this on linux when i did a save from the context menu [would need to double-check with a more recent build]...

Comment 10

[doctor__j]

Happens on Win32 build 2001041704.

Comment 11

[sairuh (rarely reading bugmail)]

tentatively nominating... again, if this is really a dup, let us know. thx!

Comment 12

[Håkan Waara]

it's useless, and now it doesn't even work. remove the time elapsed textfield 
altogether!

Comment 13

[Viswanath Ramachandran]

nav triage: not nsbeta1 or catfood.

Comment 14

[sairuh (rarely reading bugmail)]

renominate for catfood --the user shouldn't have to see NaN:NaN in the download
progress dialog, ever, imho.

Comment 15

[Doron Rosenberg (IBM)]

*** Bug 78877 has been marked as a duplicate of this bug. ***

Comment 16

[Oliver Klee]

In
http://lxr.mozilla.org/seamonkey/source/xpfe/components/xfer/resources/downloadProgress.js#328:

function onCompletion( status ) {
// Note that we're done (and can ignore subsequent progress notifications).
completed = true;
// Indicate completion in status area.
var msg = getString( "completeMsg" );
msg = replaceInsert( msg, 1, formatSeconds( elapsed/1000 ) );

If *elapsed* is not initialized, probably the "NaN" is the output. *elapsed*
gets initialized with 0 in 

http://lxr.mozilla.org/seamonkey/source/xpfe/components/xfer/resources/downloadProgress.js#182:

function onProgress( bytes, max ) {
// Check for first time.
if ( !started ) {
// Initialize download start time.
started = true;
startTime = ( new Date() ).getTime();
}
// Get current time.
var now = ( new Date() ).getTime();
// If interval hasn't elapsed, ignore it.
if ( now - lastUpdate < interval
&&
max != "-1"
&&
eval(bytes) < eval(max) ) {
return;
}
// Update this time.
lastUpdate = now;
// Update download rate.
elapsed = now - startTime;

If on Progress is not called due to a real fast download, *elapsed* never gets
initialized. We should initialize it with the definition as in
http://lxr.mozilla.org/seamonkey/source/xpfe/components/ucth/resources/helperAppDldProgress.js#38
that works the same way:

var elapsed = 0;

So line
http://lxr.mozilla.org/seamonkey/source/xpfe/components/xfer/resources/downloadProgress.js#149
should be changed.

(Sorry for the lengthy description, I still have to get aquainted with diffs.)

Comment 17

[Oliver Klee]

Nominating for 0.9.2 (assuming that my comments help in creating a fix).

Comment 18

[Avi Rappoport]

Still happening with version 0.9.3 on Mac OS 9.1.  Quite disconcerting.

Comment 19

[Oliver Klee]

Is there any way I can keep the download window from closing after the download
has finished? It closes so fast that I cannot see if this bug is still there.

Comment 20

[Håkan Waara]

*** Bug 95359 has been marked as a duplicate of this bug. ***

Comment 21

[sairuh (rarely reading bugmail)]

oliver: if you're saving via the main menu or context menu [or accel+S], there
alas isn't a way [afaik] to make the progress dialog persist. however, if you're
downloading via the helper app dialog, the progress dialog resulting from there
does have a checkbox you can select to keep the dialog up. what i do [if the
checkbox had been deselected] is go an download a large-ish file, so that the
progress dialog stays up for more than a second, then quickly select that
checkbox. :)

Comment 22

[Oliver Klee]

Attachment: patch v1

Comment 23

[Oliver Klee]

Attached fix. Whom should I cc: for comments/r/sr?

As this is my first patch, I'd greatly appreciate help and comments on how to
make good (or better) patches.

Comment 24

[Bernd]

http://www.mozilla.org/hacking/code-review-faq.html
http://mozilla.org/hacking/reviewers.html

should give you the necessary information.

look at lxr who has changed the file recently, and ask them via mail.

Comment 25

[Bill Law]

Your patch is great; thanks.  I'll review it, find a super-reviewer, and get it 
checked in.

Comment 26

[Bill Law]

Comment on attachment 49534 [details] [diff] [review]
patch v1

r=law

Comment 27

[sairuh (rarely reading bugmail)]

spam: over to File Handling.

Comment 28

[Blake Ross]

Comment on attachment 49534 [details] [diff] [review]
patch v1

sr=blake, but please don't add that comment, I don't see the usefulness of it. (It'd be nice if all those global variables were prefixed with g, as has become something of a convention in js files; that would be enough of an indicator).

Comment 29

[Bill Law]

fixed

Comment 30

[Håkan Waara]

Is there a reason for keeping this ASSIGNED rather than FIXED?

Comment 31

[Bill Law]

Yes.  I forgot to push that "Resolve bug, changing resolution to FIXED" radio
button.  But I think I got it this time :-).  Thanks.

Comment 32

[Boris Zbarsky [:bzbarsky]]

*** Bug 104653 has been marked as a duplicate of this bug. ***

Comment 33

[sairuh (rarely reading bugmail)]

have not seen this recently, so vrfy'ing fixed.