Closed
Bug 310552
Opened 19 years ago
Closed 19 years ago
Download Manager shows no progress unless download is cancelled.
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
People
(Reporter: michael.graubart7, Assigned: jst)
References
Details
(Keywords: fixed1.8, regression)
Attachments
(1 file, 1 obsolete file)
6.89 KB,
patch
|
bzbarsky
:
review+
bzbarsky
:
superreview+
asa
:
approval1.8b5+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1) Gecko/20050915 SeaMonkey/1.1a Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1) Gecko/20050915 SeaMonkey/1.1a If a download is started from any site that has downloadable files (e.g. the SeaMonkey nightly builds), the Download Manager opens but shows no progress. In fact, however, the download is proceeding normally. If 'Cancel' is clicked, the Download Manager shows the progress that had been achieved before cancellation. I have observed this misbehaviour in SeaMonkey builds 20050928 and 20050930 (trunk, Mac), but build 20050915 does not show this malfunction. This seems to be a regression. I have found bug reports from 2002 and 2003 showing similar behaviour, e.g. Bugs nos. 215894 and 295244 (to which I added comments before noticing how old they were). I apologize if this is therefore a duplicate, but I have not found a recent report of this bug. Reproducible: Always Steps to Reproduce: 1. In any site providing downloadable files, start the download. 2. Either wait for a considerable time, 3. or click "Cancel'. Actual Results: Under 2., the Download Manager shows no progress, but the download will have been satisfactorily completed. Under 3., the download may have been completed or cancelled; either way, the Download Manager now displays the progress up to completion or the moment of cancellation, as the case may be. Expected Results: In every case, the Download Manager should show progress as soon as downloading begins, and at the conclusion should show that it has been completed. eMac G4, OS X 10.9.3, Classic theme.
Confirming, WFM with SeaMonkey/20050927 but reproducible with SeaMonkey/20050928, -29 and -30 (-> All/All). This seems to be a very recent regression so I assume this bug isn't a duplicate of those older bugs. The progress dialog that opens when you click the properties button still works.
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: MacOS X → All
Hardware: Macintosh → All
Comment 2•19 years ago
|
||
It's also in the branch builds, confirming with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b5) Gecko/20050929 SeaMonkey/1.0b
Comment 3•19 years ago
|
||
(In reply to comment #1) > Confirming, WFM with SeaMonkey/20050927 but reproducible with > SeaMonkey/20050928, -29 and -30 (-> All/All). http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=SeaMonkeyAll&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2005-09-27+00%3A00&maxdate=2005-09-28+07%3A00&cvsroot=%2Fcvsroot Maybe regressed from checkin for Bug 281709 - Buffer view visibility changes like resizes. r/sr=roc Bug 281709 Under at least Windows XP, shadows are not drawn under menus on first appearance
Keywords: regression
Comment 4•19 years ago
|
||
> Bug 281709 - Buffer view visibility changes like resizes. r/sr=roc
This was backed out. Is this still a problem in today's builds?
Comment 5•19 years ago
|
||
More likely, this is a duplicate of bug 310417
Reporter | ||
Comment 6•19 years ago
|
||
Re Comment #4: In Build 2005100209 (trunk, Mac), I find that this bug is still present. (eMac G4, OS X 10.9.3, Classic theme)
Reporter | ||
Comment 7•19 years ago
|
||
Sorry! Meant OS X 10.3.9!
Comment 8•19 years ago
|
||
*** Bug 310943 has been marked as a duplicate of this bug. ***
Comment 9•19 years ago
|
||
OK, the freeze-thaw we do in bug 305032 for some reason causes this (tested via local backout). I don't know why yet, and I'm not going to be able to look until late tomorrow afternoon. But we probably need to figure out exactly what's broken here for 1.8b5...
Assignee: download-manager → general
Component: Download Manager → DOM
Product: Mozilla Application Suite → Core
QA Contact: ian
Version: unspecified → Trunk
Updated•19 years ago
|
Flags: blocking1.8b5?
Updated•19 years ago
|
Flags: blocking-seamonkey1.0b?
Comment 10•19 years ago
|
||
I'm unable to reproduce this with Firefox branch builds on Mac, Windows, or Linux. Is this Seamonkey only?
Comment 11•19 years ago
|
||
This appears to be seamonkey only and we're locking things down tonight so not going to block on this.
Flags: blocking1.8b5? → blocking1.8b5-
Comment 12•19 years ago
|
||
Renominating -- this is a core DOM bug that'll bite any C++ caller who wants to use windows. The problem is that resolving a prop on a window in JS instantiates an inner, but QI from C++ doesn't do anything like that. The C++ caller in question does: win = window.open(); win.addEventListener(); This throws because there is no inner. It should probably force inner creation instead (and we should think about other FORWARD_TO_INNER methods to see whether they should be doing the same). Asa, just because the steps to reproduce only happen in one app doesn't mean the bug is app-specific, dammit.
Flags: blocking1.8b5- → blocking1.8b5?
Comment 13•19 years ago
|
||
So to make it clear, there is a simple and I believe safe fix here. I won't be able to post it until tomorrow afternoon, however.
Comment 14•19 years ago
|
||
jst, bz isn't going to be back until tomorrow when it's too late. Any chance you can help out here?
Assignee: general → jst
Comment 15•19 years ago
|
||
bz, we have tens of thousands of bugs. unless you can show me how this impacts our Firefox and Thunderbird users, it's not going to block this release.
Flags: blocking1.8b5? → blocking1.8b5-
Comment 16•19 years ago
|
||
This is a major platform regression -- various DOM methods that should be available on windows suddenly are not if you access the window after opening it from C++. It affects any user who happens to want to install any extension that has a C++ part to it and that tries to do this to windows. The only reason we didn't catch it right away after splitwindow landed was that it was being hidden by the fact that any window that had arguments worked, for bogus reasons. But changing the arguments behavior makes this a problem. So what it comes down to is that while you apparently only care about Firefox and Thunderbird, I care about Gecko and our platform story. Since we're talking about a Gecko milestone (1.8b5 is that, last I checked), and since after this milestone I don't expect drivers to take changes to this code based on past communications, if we're going to fix this platform regression at all, it needs to be fixed for 1.8b5. I'm not going to bother rerequesting approval on this, because I'm rapidly approaching just writing 1.8 off completely and moving on with life.
Comment 17•19 years ago
|
||
If jst doesn't have cycles, maybe you can help look at this too blake (I see you are already cc'ed)
Assignee | ||
Comment 18•19 years ago
|
||
This makes us create a new inner window if AddEventListener(), AddGroupedEventListener(), or GetListenerManager() is called. Those are the only ones of the methods that forward to the inner window where it makes sense to create a new inner window if one doesn't already exists. The other methods that forward to the inner are HandleDOMEvent(), SetScriptsEnabled() (which doesn't actually enable or disable scripts, just runs timeouts if necessary), DispatchEvent(), RemoveGroupedEventListener(), RemoveEventListenerByIID(), GetObjectProperty(), SetTimeoutOrInterval(), ClearTimeoutOrInterval(), SuspendTimeouts(), and ResumeTimeouts(), none of which should IMO create an inner. I also made HandleDOMEvent() and DispatchEvent() return NS_OK in stead of an error if there is no inner window.
Attachment #198381 -
Flags: superreview?(brendan)
Attachment #198381 -
Flags: review?(mrbkap)
Assignee | ||
Comment 19•19 years ago
|
||
Right diff this time.
Attachment #198382 -
Flags: superreview?(brendan)
Attachment #198382 -
Flags: review?(mrbkap)
Assignee | ||
Updated•19 years ago
|
Attachment #198381 -
Attachment description: Create inner windows when necessery → Wrong file...
Attachment #198381 -
Attachment is obsolete: true
Attachment #198381 -
Flags: superreview?(brendan)
Attachment #198381 -
Flags: review?(mrbkap)
Comment 20•19 years ago
|
||
Comment on attachment 198382 [details] [diff] [review] Create inner windows when necessery r+sr=bzbarsky. Thanks for dealing with this, Johnny. I owe you one. Requesting approval; this should be reasonably safe; only changes the behavior of methods that mess with event listeners on newly-opened windows, and fixes them to work instead of throwing.
Attachment #198382 -
Flags: superreview?(brendan)
Attachment #198382 -
Flags: superreview+
Attachment #198382 -
Flags: review?(mrbkap)
Attachment #198382 -
Flags: review+
Attachment #198382 -
Flags: approval1.8b5?
Comment 21•19 years ago
|
||
Comment on attachment 198382 [details] [diff] [review] Create inner windows when necessery >Index: dom/src/base/nsGlobalWindow.cpp >+ nsresult fwdic_nr = GetDocument(getter_AddRefs(doc)); \ This could really just be |nsresult rv| since you're in a new scope and don't have to worry about conflicting with the rest of the function. Please remember to test to make sure that this doesn't break focus on 2nd and 3rd windows as well. r=mrbkap
Attachment #198382 -
Flags: superreview+ → superreview?(brendan)
Updated•19 years ago
|
Attachment #198382 -
Flags: superreview?(brendan) → superreview+
Comment 22•19 years ago
|
||
Comment on attachment 198382 [details] [diff] [review] Create inner windows when necessery Thanks for the quick turnaound on this Johnny.
Attachment #198382 -
Flags: approval1.8b5? → approval1.8b5+
Reporter | ||
Comment 23•19 years ago
|
||
Just downloaded Build 2005100410, and Download Manager works again.
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → WORKSFORME
Comment 24•19 years ago
|
||
Bug was fixed: 2005-10-03 18:08 jst%mozilla.jstenback.com mozilla/ dom/ src/ base/ nsGlobalWindow.cpp 1.790 26/8 Fixing bug 310552. Fix window.addEventListener() Worksforme is the resolution for Bugs where it is not known how they got fixed. Reopening
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
Comment 25•19 years ago
|
||
marking fixed based on Reporter's comment 23
Status: REOPENED → RESOLVED
Closed: 19 years ago → 19 years ago
Resolution: --- → FIXED
Comment 26•19 years ago
|
||
jst landed this on branch too. In general, for future reference, it's up to the person fixing the bug to decide when to resolve it...
Keywords: fixed1.8
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•