97408 - download progress information is garbled

Status: RESOLVED WORKSFORME

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

Description

[J Luh]

With Mozilla 2001082805 on Mac OS X 10.0.4:

Visit the URL and choose to Save this file to Disk.

The download progress dialog shows nonsense numbers under Status, Time Left, and
Elapsed Time.

Comment 1

[J Luh]

Looks like this doesn't happen all the time, but it happens sometimes.

Comment 2

[sairuh (rarely reading bugmail)]

the next time you see this, could you pls attach a screenshot?

i don't think i've seen this recently...

Comment 3

[J Luh]

Attachment: screen shot of garbled information

Comment 4

[J Luh]

Here's a screen shot of the problem (with 2001082905). Notice that the kb/sec is
negative, and there are minus signs in the time left and elapsed time columns.
The seconds column of elapsed time seems to count up -59, -58, -57 .... I didn't
really catch any other patterns.

This doesn't happen with every download. I don't know what conditions cause this
to happen.

Comment 5

[J Luh]

Evidently Mozilla can get in a state where some of the time values that should
be positive integers come up negative.

The 0-0 format comes about when the formatSeconds function adds leading 0s to
the negative numbers.

http://lxr.mozilla.org/seamonkey/source/xpfe/components/ucth/resources/helperAppDldProgress.js

Comment 6

[Greg K.]

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

Comment 7

[Greg K.]

Confirming due to second report. (I have not been able to duplicate however.)

Not sure if this is the right component, though. Blake?

Comment 8

[sairuh (rarely reading bugmail)]

->trudelle for triage.

pink, have you seen this?

Comment 9

[Mike Pinkerton (not reading bugmail)]

i have not seen this, but i haven't played much with this dialog. pchen is 
probably a likely owener for this bug.

Comment 10

[Paul Chen]

marking p3 and mozilla1.0

Comment 11

[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 12

[sairuh (rarely reading bugmail)]

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

Comment 13

[Paul Chen]

moving to mozilla0.9.8

Comment 14

[Paul Chen]

Pushing out to mozilla1.0.1

Comment 15

[J Luh]

This still happens with Mozilla 0.9.6 on Mac OS X 10.1.1. Don't know about newer
code.

Comment 16

[Boris Zbarsky [:bzbarsky]]

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

Comment 17

[nnooiissee]

changing platform to all since at least two of the duped bugs (including the one
from me) are on mac os 9.x.

i have seen this more often since switching to mac os x (10.1.x). it seems to be
very likely after having mozilla open for several hours, and with an opperating
system and a browser that don't crash i don't see many other reasons to ever
quit moz.

Comment 18

[nnooiissee]

Attachment: picture showing startTime (Saving From:) and now (To:)

i poked at the code a little bit, and have an idea for a quick fix, i think.
now stays good, but startTime does not get set correctly. i think i am going to
do something like if (startTime > now) startTime = now; which will give
unreasonably high  KB per second at the begining of a transfer, like the old
code will, but it seems like a good backup to me.

Comment 19

[nnooiissee]

Attachment: workaround patch

I put together a simple patch. It checks to see if startTime > now, and if so
sets StartTime = now and sets missedBytes to the amount downloaded so far.
Download speed now factors in missedBytes.

I still don't know what is causing a bad value with startTime, but this gives
accurate results even when startTime is wrong.

Comment 20

[nnooiissee]

i doubt pchen will be of much help on this. reassigning to default owner.

Comment 21

[Timothy Sipples]

Attachment: Dialog with Bad Times (Mozilla 2002022108 OS/2)

This bug is still present in Mozilla 2002022108 (OS/2), apparently.  Screen
shot (download.jpg, attached) shows the problem on this platform/build.

Comment 22

[Boris Zbarsky [:bzbarsky]]

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

Comment 23

[nnooiissee]

Timothy: could you try my patch and see whether that fixes it for you?

Comment 24

[Boris Zbarsky [:bzbarsky]]

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

Comment 25

[nnooiissee]

Attachment: freshening my patch

Comment 26

[Boris Zbarsky [:bzbarsky]]

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

Comment 27

[Boris Zbarsky [:bzbarsky]]

As a note, a lot of the dups are on OS/2.  What do OS/2 and mac have in common?

Comment 28

[nnooiissee]

[e-mail]: would you be willing to try my patch? anyone on OS/2?
Don't all speak up at once now.

Comment 29

[jhp (no longer active)]

I am taking a look at this for OS/2.  Yes, mStartTime is getting set
incorrectly, at nsProgressDialog.js:95 ("set startTime(newval)   { return
this.mStartTime = newval/1000; }").  The mStartTime value gets set correctly at
line 55 (constuctor), but someone else is passing in a bogus value that is
greater than the current time, such that elaspedTime becomes negative.

I am trying to put dump() statements in the code, but I cannot get it to work. 
I enabled the pref in Pref->Debug, but nothing shows up in either the console or
JavaScript console.  Any ideas on how I can make dump() work?

Comment 30

[jhp (no longer active)]

OK, this single line patch fixes the issue for me:

Index: nsProgressDialog.js
===================================================================
RCS file: /cvsroot/mozilla/embedding/components/ui/progressDlg/nsProgressDialog.js,v
retrieving revision 1.5
diff -u -r1.5 nsProgressDialog.js
--- nsProgressDialog.js	2002/02/20 07:54:04	1.5
+++ nsProgressDialog.js	2002/03/14 21:34:39
@@ -92,7 +92,7 @@
     get observer()          { return this.mObserver; },
     set observer(newval)    { return this.mObserver = newval; },
     get startTime()         { return this.mStartTime; },
-    set startTime(newval)   { return this.mStartTime = newval/1000; }, //
PR_Now() is in microseconds, so we convert.
+    set startTime(newval)   { return; },
     get lastUpdate()        { return this.mLastUpdate; },
     set lastUpdate(newval)  { return this.mLastUpdate = newval; },
     get interval()          { return this.mInterval; },

I think the incorrect value that is being passed in comes from
nsExternalHelperAppService.cpp:1394.  I'll look into this next.

Comment 31

[jhp (no longer active)]

OK, I took a look at nsExternalHelperAppService.cpp, and the values that they
are passing to SetStartTime() are correct.  I think the bug is actually in the
xptcall code.  Somehow, the value is getting mangled when it goes from C++ to
JavaScript.  Maybe both OS/2 and Mac don't handle 'long long' well.

Comment 32

[jhp (no longer active)]

Attachment: OS/2 patch

The problem on OS/2 was in the xptcall code.  We were not properly handling the
high bit of the long long.  

I have not looked at the Mac code, but I think that it may also have an issue
in xptcall.

Comment 33

[Mike Kaply [:mkaply]]

Comment on attachment 74234 [details] [diff] [review]
OS/2 patch

r=mkaply
sr=blizzard (platform specific code)

Comment 34

[John Bandhauer]

cc'ing beard on the question of Mac PPC xptcall stubs code perhaps not properly
handling long long.

Comment 35

[Asa Dotzler [:asa]]

Comment on attachment 74234 [details] [diff] [review]
OS/2 patch

a=asa (on behalf of drivers) for checkin to the 1.0 trunk

Comment 36

[Asa Dotzler [:asa]]

has the OS/2 patch landed? time is running out.

Comment 37

[Mike Kaply [:mkaply]]

Os/2 patch has landed

I don't understand why we have heard nothing from the Mac folks on this.

Comment 38

[nnooiissee]

Because on the mac it not easily reproduceable, and truth be told i have not
seen it for quite some time.

It would be nice if someone followed in your footsteps and killed this thing on
mac too, but i'm not sure if anyone at netscape has ever seen this (on mac), and
i don't really have the capabilities to dl/compile a package the size of moz.

Comment 39

[Mike Pinkerton (not reading bugmail)]

beard understands the xpt stuff. over to him for some mac luvin

Comment 40

[Patrick C. Beard]

I haven't seen this for a long time on Mac builds either.

Comment 41

[Mike Pinkerton (not reading bugmail)]

yes, this does still happen. my 3/31 build is doing it as we speak on osx.

Comment 42

[eero.juhola]

Just downloaded OS/2 build 20020402 and can report that the problem 
is now gone on this platform at least. 

Comment 43

[Boris Zbarsky [:bzbarsky]]

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

Comment 44

[Matthias Versen [:Matti]]

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

Comment 45

[Matthias Versen [:Matti]]

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

Comment 46

[Matthias Versen [:Matti]]

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

Comment 47

[Matthias Versen [:Matti]]

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

Comment 48

[Darryl Zurn]

Attachment: Still get garbled times with RC2 on Mac OS 9.2

GIF screenshot showing RC2 garbled download times, Mac OS 9.2

Comment 49

[Mike Kaply [:mkaply]]

As I pointed out before, someone needs to debug the long long xpt code on Mac

Comment 50

[Patrick C. Beard]

bryner, would you take a look at this?

Comment 51

[J Luh]

No activity in three years and I have not experienced this problem in a long
time. Please reopen this bug if you still see this problem in recent versions.