Closed
Bug 289724
Opened 19 years ago
Closed 19 years ago
Missing progress bar animation in download window
Categories
(Core :: XPConnect, defect)
Tracking
()
VERIFIED
FIXED
People
(Reporter: krmathis, Assigned: mark)
References
Details
(Keywords: regression)
Attachments
(2 files, 1 obsolete file)
12.23 KB,
image/jpeg
|
Details | |
22.74 KB,
patch
|
bryner
:
review+
brendan
:
superreview+
asa
:
approval-aviary1.1a1+
brendan
:
approval1.8b2+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b2) Gecko/20050405 Firefox/1.0+ (PowerBook) Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b2) Gecko/20050409 Firefox/1.0 The latest Trunk build no longer display a progress bar animation when you save a file to disk. The progress bar is there, but its static. Its not animating... Reproducible: Always Steps to Reproduce: 1. Click Save Link As.. 2. Open the Download window 3. Watch the progress bar Actual Results: No progress bar animation Expected Results: Display progress bar animation
*** Bug 289754 has been marked as a duplicate of this bug. ***
Reporter | ||
Comment 2•19 years ago
|
||
I'll add that it dont display the text for the remaining time, download speed or amount of data either. The window title say "Download" instead of "20% of 1 file - Downloads" as well.
Reporter | ||
Comment 3•19 years ago
|
||
Reporter | ||
Comment 4•19 years ago
|
||
Comment on attachment 180252 [details]
Downloads
Screenshot to show what it looks like.
Attachment #180252 -
Attachment description: Screenshot to show what it looks like. → Downloads
Comment 5•19 years ago
|
||
Confirmed on Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b2) Gecko/20050412 Firefox/1.0+
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment 7•19 years ago
|
||
As noted in duped bug 289754, I noticed the problem first with the 20050407 official nightly build.
Comment 8•19 years ago
|
||
I am seeing it too. Pretty bad, because you don't know how much you've downloaded :( Could it be fixed for gecko 1.8b2, please ?
Flags: blocking-aviary1.1?
Reporter | ||
Comment 9•19 years ago
|
||
It started with the 20050406 nightly build (working fine in the 20050405 build).
Comment 10•19 years ago
|
||
*** Bug 290138 has been marked as a duplicate of this bug. ***
Updated•19 years ago
|
Flags: blocking-aviary1.1? → blocking-aviary1.1+
Comment 11•19 years ago
|
||
regression from bug 283489 ?
Comment 12•19 years ago
|
||
hmm. any messages in JS Console? also, how are you saving, "save link target as" or by just left clicking the file?
Comment 13•19 years ago
|
||
Erh ;o) JS console is empty when clicking on a download link - and I have to open it first (15 hours old homemade build). Also, I cannot get download window in front if JS console window is opened. Save link as ? Just name of file and a "starting" line under it. Cancel line is available in both cases. JS console ? Empty. Should I have to do a debug build ?!
Comment 14•19 years ago
|
||
(In reply to comment #12) > hmm. any messages in JS Console? also, how are you saving, "save link target as" > or by just left clicking the file? Downloading the latest nightly from http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-trunk/ Click on the link, download manager comes up, ask what to do with the file. click OK, 'downloads' window appears. Same happens with 'Save Page as' JS Console is empty in all cases. javascript.options.showInConsole pref to true, or false, doesn't matter.
Comment 15•19 years ago
|
||
It might be relevant that (at any rate in Build 2005041309), if the 'Progress Dialog' is selected in Preferences, this does work, even though the 'Download Manager' does not.
Comment 16•19 years ago
|
||
One more observation that might conceivably help someone to fix this: if you have the Download Manager open while downloading something, and you click 'Cancel', the percentage and number of KB downloaded appear.
Comment 17•19 years ago
|
||
We need to find out exactly what checkin caused this. If I have time I will look into the regression. (Find out what regressed)
Comment 18•19 years ago
|
||
Just one more comment :) According to the info Kai gave, the problem checkin should be one of these: <http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD &branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date &hours=2&date=explicit&mindate=2004-04-05+10%3A51%3A00&maxdate=2004-04-06+15%3A17%3A00 &cvsroot=%2Fcvsroot>
Comment 19•19 years ago
|
||
(as unwrapped url: http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2004-04-05+10%3A51%3A00&maxdate=2004-04-06+15%3A17%3A00&cvsroot=%2Fcvsroot )
Assignee | ||
Comment 20•19 years ago
|
||
That Bonsai query is from last year. Try this: http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2004-04-05+10%3A51%3A00&maxdate=2004-04-06+15%3A17%3A00&cvsroot=%2Fcvsroot Looks like bug 228968. That brought 64-bitness to the download progress dispalay, to handle proper size reporting for files larger than 2GB. (click click click) Yup. There's your problem. 228968. Once things went 64-bit, some corruption was introduced into the data path. The numbers are fine in toolkit/components/downloads/src/nsDownloadManager.cpp:OnProgressChange64. Something's getting mucked up by the time it's on the far side of XPIDL, though. Once it hits toolkit/mozapps/downloads/content/DownloadProgressListener.js:onProgressChange, aCurTotalProgress and aMaxTotalProgress are screwed up, and aDownload is unusable. aCurTotalProgress and aMaxTotalProgress show up shifted 32 bits left (or low/high32 swapped, same thing for |values| under 2G). Curiously, aCurSelfProgress and aMaxSelfProgress both are fine all the way through. With aDownload unusable, onProgressChange kicks out at the first unchecked referenece, right below the comment that says "Update download rate." The rest of that method is responsible for making changes to the download manager window, and it never gets a chance to run. Restoring toolkit/components/downloads/public/nsIDownloadProgressListener.idl so that the relevant fields are 32 bits covers up the problem. Obviously, this isn't a solution, but it points to something not being 64-bit-clean. JS? XPIDL-to-JS marshalling? Are they not 64-bit-clean on the Mac? Does this bug not affect other platforms? I'll investigate more when I get another chance.
Assignee | ||
Comment 21•19 years ago
|
||
Pasted the wrong Bonsai query (again) - one more try. http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2005-04-05+10%3A51%3A00&maxdate=2005-04-06+15%3A17%3A00&cvsroot=%2Fcvsroot
Comment 22•19 years ago
|
||
Oops, no wonder I couldn't find anything using that query :) Well this is just a guess but the fix for bug 228968 may have caused this. If I have time I'll try it.
Assignee | ||
Comment 23•19 years ago
|
||
Diagnosed and have a fix. It works, but I want to test it a little bit longer. Feel free to reassign to me. Stay tuned.
Assignee | ||
Comment 24•19 years ago
|
||
When I picked this one up, I didn't expect that I'd have to get so close to the metal to make it happen. I guess that's why this it's so satisfying, and why I'm particularly proud of this one. Here's what happened. The fix for bug 228968 turned the numbers used for download progress tracking into 64-bit integers. See comment 20. The xptcall port to Mac OS X/PowerPC was not friendly with 64 bits, though. It contained an implementation that didn't match the Mac OS X/PowerPC ABI. Specifically, it enforced natural alignment of 64-bit integers, where the actual ABI has no such requirement and allows (requires) 64-bit types to be word-aligned in the parameter section of a stack. Mozilla was using an implementation of SOME ABI, but it wasn't the RIGHT ABI. Here's a good guide to the ABI: http://developer.apple.com/documentation/DeveloperTools/Conceptual/MachORuntime/PowerPCConventions/chapter_3_section_1.html As for the strange behavior I saw in comment 20 with some 64-bit numbers making it through xptcall without problems, that's because the first pair of 64-bit ints were passed in registers and saved separately; the old code didn't have trouble pulling them but did have trouble pulling the other numbers from the stack. (An interesting bug in the implementation is that for any array of 64-bit ints, depending on the alignment, EITHER the values would be pulled from registers properly OR they'd be pulled from the stack properly, but not both.) Nobody noticed this bug up until now, most likely, because 64-bit ints are used so sparingly, and because if the alignment was right, everything would pass muster anyway. (That's alignment of ints, not planets, although I'm sure that those were a crucial factor too.) The attached patch contains a revised (fixed, largely rewritten) implementation of the Mac OS X/PPC ABI for xptcall. In addition to doing a better job of tracking the ABI, it fixes a couple of latent bugs. For example, the 11th, 12th, and 13th floating-point arguments would be trash if anyone ever did anything as foolish as passing more than 11. I hope it doesn't introduce any new ones. It tests well in Firefox/trunk. The patch also significantly cleans up the old code, both in appearance and in function.
Assignee | ||
Updated•19 years ago
|
Attachment #181103 -
Flags: review?(shaver)
Attachment #181103 -
Flags: approval-aviary1.1a?
Assignee | ||
Comment 25•19 years ago
|
||
Corrects a problem with float (not double) arguments not in FPRs (after the 13th). @@ -519 +519 @@ -+ dp->val.f = (float) theParam; ++ dp->val.f = *(float *) &argsStack[argIndex]; That's better.
Attachment #181103 -
Attachment is obsolete: true
Attachment #181112 -
Flags: review?(shaver)
Attachment #181112 -
Flags: approval-aviary1.1a?
Comment 26•19 years ago
|
||
great, glad to hear this regression isn't my fault ;-)
Assignee: bugs → mark
Component: Download Manager → XPConnect
Flags: review?(shaver)
Product: Firefox → Core
QA Contact: aebrahim-bmo → pschwartau
Version: unspecified → Trunk
Assignee | ||
Updated•19 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Updated•19 years ago
|
Attachment #181112 -
Flags: review?(shaver) → review?(bryner)
Assignee | ||
Updated•19 years ago
|
Attachment #181103 -
Flags: approval-aviary1.1a?
Assignee | ||
Updated•19 years ago
|
Flags: blocking1.8b2?
Comment 27•19 years ago
|
||
Comment on attachment 181112 [details] [diff] [review] Updated patch Looks good to me. Assuming you've tested it thoroughly (it sounds like you have), r=bryner.
Attachment #181112 -
Flags: review?(bryner) → review+
Assignee | ||
Comment 28•19 years ago
|
||
Comment on attachment 181112 [details] [diff] [review] Updated patch Requesting sr=brendan. (And someone with cvs access to check in.) bryner: It's well-tested. It fixes this bug for Firefox and bug 290797 for Seamonkey. Camino works with it, but never exhibited a bug (it's still truncating to 32 bit ints and it doesn't push download stats through xptcall).
Attachment #181112 -
Flags: superreview?(brendan)
Attachment #181112 -
Flags: approval1.8b2?
Comment 29•19 years ago
|
||
Comment on attachment 181112 [details] [diff] [review] Updated patch rs=me, and a= for 1.8b2. /be
Attachment #181112 -
Flags: superreview?(brendan)
Attachment #181112 -
Flags: superreview+
Attachment #181112 -
Flags: approval1.8b2?
Attachment #181112 -
Flags: approval1.8b2+
Comment 30•19 years ago
|
||
checked in 2005-04-19 18:01
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Comment 31•19 years ago
|
||
Comment on attachment 181112 [details] [diff] [review] Updated patch a=asa
Attachment #181112 -
Flags: approval-aviary1.1a? → approval-aviary1.1a+
Updated•19 years ago
|
Flags: blocking1.8b2?
Comment 32•19 years ago
|
||
*** Bug 290797 has been marked as a duplicate of this bug. ***
Reporter | ||
Updated•18 years ago
|
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•