Download Manager shows no progress unless download is cancelled.

RESOLVED FIXED

Status

()

Core
DOM
RESOLVED FIXED
12 years ago
6 years ago

People

(Reporter: Michael Graubart, Assigned: jst)

Tracking

({fixed1.8, regression})

Trunk
fixed1.8, regression
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.8b5 -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

12 years ago
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.

Comment 1

12 years ago
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
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

12 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
> Bug 281709 - Buffer view visibility changes like resizes. r/sr=roc

This was backed out.  Is this still a problem in today's builds?
More likely, this is a duplicate of bug 310417
(Reporter)

Comment 6

12 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

12 years ago
Sorry! Meant OS X 10.3.9!

Comment 8

12 years ago
*** Bug 310943 has been marked as a duplicate of this bug. ***
Blocks: 305032
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
Flags: blocking1.8b5?

Updated

12 years ago
Flags: blocking-seamonkey1.0b?

Comment 10

12 years ago
I'm unable to reproduce this with Firefox branch builds on Mac, Windows, or
Linux. Is this Seamonkey only?

Comment 11

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

12 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

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

12 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

12 years ago
Created attachment 198381 [details] [diff] [review]
Wrong file...

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

12 years ago
Created attachment 198382 [details] [diff] [review]
Create inner windows when necessery

Right diff this time.
Attachment #198382 - Flags: superreview?(brendan)
Attachment #198382 - Flags: review?(mrbkap)
(Assignee)

Updated

12 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 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 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)
Attachment #198382 - Flags: superreview?(brendan) → superreview+

Comment 22

12 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

12 years ago
Just downloaded Build 2005100410, and Download Manager works again.
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → WORKSFORME

Comment 24

12 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

12 years ago
marking fixed based on Reporter's comment 23
Status: REOPENED → RESOLVED
Last Resolved: 12 years ago12 years ago
Resolution: --- → FIXED
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

12 years ago
Depends on: 311479

Updated

12 years ago
Depends on: 327248
No longer depends on: 327248
You need to log in before you can comment on or make changes to this bug.