348925 - Transferred data column isn't updated when download finishes

Status: RESOLVED FIXED

(SeaMonkey :: Download & File Handling, defect)

Description

[Rob Hulswit]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a1) Gecko/20060816 SeaMonkey/1.5a
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a1) Gecko/20060816 SeaMonkey/1.5a

When a download finishes, the column with the transferred data amounts misses its final update on completion. In the URL, it currently shows 454KB of 472KB on a finished download. It is different from bug 149237 in that the size of the download is actually known (and shown during downloading)

Reproducible: Always

Steps to Reproduce:
1. Download the file http://ftp.mozilla.org/pub/mozilla.org/seamonkey/nightly/2006-08-16-11-trunk/windows-xpi/langenus.xpi
2. Open the download manager
3. After completion of the download, look at the transferred column.


Actual Results:  
It shows 454KB of 472KB

Expected Results:  
It shows 472GB of 472KB

This is really confusing; the user is given contradictionary information; on the one hand the progress column says that the download is finished, but the transferred column says that it hasn't

Comment 1

[Gérard Talbot]

I confirm your findings with Seamonkey 1.5a rv:1.9a1 build 2006081609 under XP Pro SP2.

CONFIRMING

Comment 2

[Gérard Talbot]

Attachment: Screenshot of download manager after completing steps to reproduce

Progress says Finished 
while
Transferred says 470KB of 472KB

Comment 3

[Steve Wendt]

Problem also present on Gecko/20060821 SeaMonkey/1.1a, as well as the OS/2 build of 1.1a.  It's only cosmetic, but it is a regression from 1.0.x.

Comment 4

[Bruno 'Aqualon' Escherl]

This is a regression from bug 273971, which was checked in into 1.8.1 partly with bug 245725.

Comment 5

[Bruno 'Aqualon' Escherl]

WFM with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20051218 SeaMonkey/1.5a, regressed with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20051220 SeaMonkey/1.5a. Checkin of bug 273971 was 20051219.

Comment 6

[Son Le]

I've got a fix for this. It was meant to go in as part of the patch for bug 237623, but never made it. I'll clean it up tonight and attach it here.

Comment 7

[Son Le]

Attachment: patch v0 - for toolkit

Comment 8

[Son Le]

Attachment: patch v0 - for xpfe

Comment 9

[[e-mail]]

Comment on attachment 238888 [details] [diff] [review]
patch v0 - for xpfe

I guess this explains why unknown sized downloads used to constantly update because it was impossible to know which was the last... biesi, should ExecuteDesiredAction use mProgress for all values, to force an update?

>+  if (aMaxTotalProgress != -1 && LL_EQ(aCurTotalProgress, aMaxTotalProgress))
I'd hope that LL_EQ(aCurTotalProgress, aMaxTotalProgress) guarantees aMaxTotalProgress != -1 ;-) In which case, you could just change the test below to delta < gInterval && LL_NE(aCurTotalProgress, aMaxTotalProgress)

Comment 10

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

Comment on attachment 238888 [details] [diff] [review]
patch v0 - for xpfe

yeah, please remove he "aMaxTotalProgress != -1 && " part. Also, you can now use == instead of LL_EQ, we gave up on supporting platforms without native PRInt64. Finally, please use PRBool instead of bool (and PR_FALSE instead of false)

Please attach a new patch with those changes, thanks. Sorry for the delay.

Comment 11

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

(In reply to comment #9)
> biesi, should
> ExecuteDesiredAction use mProgress for all values, to force an update?

I agree. Didn't I review a patch somewhere that did that?

Comment 12

[[e-mail]]

(In reply to comment #11)
> I agree. Didn't I review a patch somewhere that did that?

Any of these? (I queried on attachment data contains OnProgressChange64)

https://bugzilla.mozilla.org/buglist.cgi?bug_id=83265,187483,200448,228968,237623,241972,245725,273971,288585,290648,291406,312370,326840,343921,348925

Comment 13

[Son Le]

Attachment: patch v1 - for toolkit

Comment 14

[Son Le]

Attachment: patch v1 - for xpfe (Checked into trunk & 1.8.1 branch)

Updated.

Comment 15

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

neil: ah. see bug 149237 comment 48, at the end. seems to have been before the 64-bit version of onProgressChange, so your query doesn't find it.

Comment 16

[Andrew Schultz]

Comment on attachment 239994 [details] [diff] [review]
patch v1 - for xpfe (Checked into trunk & 1.8.1 branch)

I landed this on trunk

Comment 17

[Robert Kaiser]

Should this go into SeaMonkey 1.1 (on 1.8 branch)? If so, please nominate it for that by requesting approval-seamonkey1.1b?

This is no really major problem though, it's just annoying, so we won't push back a beta for it, we can still take such trivial polishing fixes (without L10n changes) after Beta and still have it in final.

Comment 18

[Robert Kaiser]

Comment on attachment 239994 [details] [diff] [review]
patch v1 - for xpfe (Checked into trunk & 1.8.1 branch)

Looks like an easy change :)
a=me for SeaMonkey 1.1b

Comment 19

[Ian Neal]

Comment on attachment 239994 [details] [diff] [review]
patch v1 - for xpfe (Checked into trunk & 1.8.1 branch)

Checking in (1.8.1 branch)
nsDownloadManager.cpp;
new revision: 1.111.4.4; previous revision: 1.111.4.3
done

Comment 20

[Mike Connor [:mconnor]]

Comment on attachment 239993 [details] [diff] [review]
patch v1 - for toolkit

if this still applies, please re-request review.  I'm pretty sure this is no longer needed though.

Comment 21

[Robert Kaiser]

What needed to be fixed has been fixed for SeaMonkey 1.1 Beta. Let's mark this correctly.