131762 - Can open multiple occurrences of Download Manager

Status: VERIFIED FIXED

(SeaMonkey :: Download & File Handling, defect)

Description

[:deanis74]

1. Tasks > Tools > Download Manager
2. Tasks > Tools > Download Manager

Expected Results: Download Manager window is activated

Actual Results: a second DM window is opened

2002031803 on Win2K

Comment 1

[Boris Zbarsky [:bzbarsky]]

Marking dependency on the "Opening content in an existing window with
window.openDialog fails to send arguments"; if that bug is not fix we'll have to
reuse the hack we do for the preferences window or something...

Comment 2

[Blake Ross]

attaching a fix for this that also adds a preference panel with a pref:

When starting a download

() Open download manager
() Open progress dialog
() Don't open anything

Comment 3

[Blake Ross]

Attachment: patch

Comment 4

[Blake Ross]

Attachment: the whole fix

Comment 5

[Ben Goodger (use ben at mozilla dot org for email)]

Comment on attachment 74892 [details] [diff] [review]
patch

sr=[e-mail]

Comment 6

[Joe Hewitt (gone)]

Comment on attachment 74893 [details] [diff] [review]
the whole fix

r=el guapo

Comment 7

[Asa Dotzler [:asa]]

Comment on attachment 74893 [details] [diff] [review]
the whole fix

a=asa (on behalf of drivers) for checkin to the 1.0 trunk
and bringing ben's sr= forward

Comment 8

[Blake Ross]

fixed.

Comment 9

[Blake Ross]

cc'ing jatin because some new UI was added. let me know if you need help
understanding any of it.  (the wording I quoted earlier isn't exactly what's
used in the panel.)

Comment 10

[:deanis74]

verified using 2002032003 on win2k

Comment 11

[Bill Law]

I think some of this was a mistake.

There's no way to dismiss the progress dialog on the Mac now.  And it isn't 
obvious that the semantics of the progress dialog should change if the user 
chooses to "open progress dialog" in prefs.

I believe there needs to be more discussion before changing the fundamental 
behavior of closing the progress dialog.

Comment 12

[Blake Ross]

I discussed this with Marlon before checking in.  He thought we should do what I
did but present a question, "Do you want to cancel this download?" when closing
the progress dialog.  I am more or less in favor of that idea, and was going to
do it in a separate bug.  Perhaps he also meant for me to remove the Cancel
button if the user chooses to Open a Progress Window when downloading, I will
ask him. I don't think either of us realized that this would prevent closure on
mac. What do you think?

Comment 13

[:deanis74]

Hrm...  The original problem that I reported is fixed, though.  Shouldn't the
problem you mention be a new bug?

Comment 14

[Blake Ross]

> And it isn't obvious that the semantics of the progress dialog should change if 
> the user chooses to "open progress dialog" in prefs.

Wait, are you describing current behavior or just shooting down a possible
solution?  The behavior I see right now is that the dialog never has a Cancel
button. So there's no inconsistency, although there is, as you note, a problem
on Mac. I'm going to start a newsgroup discussion about this and ask Marlon if
he has any time to hash out a quick (formal) spec for this behavior.

Comment 15

[[not reading bugmail]]

I had an idea that you can hash on, we should just bring up the window.. and if
you wanna find out some more info on a download you hit properties.. like it
does, but you could make it so that you can close that individual dialog again
if you hit properties (does that work?, or no)  Or add a close dialog button
which doesn't cancel the download.. just closes down the dialog. and focuses the
window that is open.  if that is not possible (create some tab like context menu
for it)  I'd rather look at my Download Window feature than those dialogs.

My view here is that I'd like to see the Window first and only.. if I have a
problem with a download I hit properties.. 

but I should be able to close a properties dialog if the download keeps going in
the Window, right?  If so, there should be a context menu that have the same
items on them as there are buttons like in the download dialogs.

Other wise I just see this as just useless If I have to keep each download
dialog open.  point is like I said, I dont wanna look at them now that we have
the download manager.  Since I've not expressed my thoughs, A big thanks goes
out to blake and bill for this feature landing, its cool.

Comment 16

[Blake Ross]

Dennis: have you used today's nightly?  It should do just what you want by
default (and it's configurable via Navigator > Downloads).  But I've backed it
out for now until we discuss it fully.

Comment 17

[Bill Law]

re: comment 12

>What do I think?

* Closing a progress dialog opened to view download manager properties shouldn't
cancel the download (not by default, maybe not ever).
* Closing a progress dialog opened directly as the result of starting a download
should cancel the download (by default, maybe all the time), since that's the
way it has always worked.
* Closing a progress dialog should always do the same thing, regardless of how
it was opened, otherwise the user won't know what it will do and will curse us
whenever it does the wrong thing.

In other words, I think we're between a rock and a hard place.

Comment 18

[Bill Law]

re: comment 14

>Wait, are you describing current behavior or just shooting down a possible
>solution?

I'm not sure.  Are those my only two choices?  What I was trying to say was that
if a user chocses "open progress dialog," then they might expect that dialog to
offer some way of cancelling the download, and further, might well expect
closing the dialog via various means (pressing the close box, Alt-F4) to cancel
the download, since that's the way it worked yesterday (and the day before that,
as far back as anybody can remember).

>  The behavior I see right now is that the dialog never has a Cancel
>button. So there's no inconsistency

There's inconsistency with past behavior.  And there's the fact that there's no
way to cancel the download.  I didn't constrain myself to just the issue of
"consistency" (whatever that is).  I think changing the fundamental semantics of
closing this dialog is a big enough issue to warrant greater discussion.  As is
the issue of how we actually implement that new behavior.

Comment 19

[[not reading bugmail]]

I did try build 3-20.. and its just like you say.. its what I was talking
about.... but sad you pulled it.. :)  

For the Sake of shorthand:

*DM Download Manager
*PD Progress Dialog

I did like the implementation even better than what I was thinking, just maybe
need the [pause/resume] button on the DM toolbar, that should be enough instead
of a context menu, since there is already the other necessary ones.  Since
Launch never works in a PD, but does in the DM.


It sounds like the mac guys want you to use two dialogs while the DM guys want
1.  So change it was like it was in the 3-20 build, but if the user wants the PD
pref, Then use an older PD which seems to me that, that crowd doesn't wanna use
the DM.  

Bill, their functionality can overlap if you find a way to code it right so you
have a (case) clause dependent on pref settings.

..other wise it sounds like the solution is to use two seperate PD's.

So you get this result:


In the Case of the DM usage you need the PD patch that went into 3-20:

You then could have one [Close] (Close which then brings focus back to the DM to
show its still downloading, but no Cancel) button and a triple [PAUSE/RESUME or
blank area] button.

In the Case of the PD usage you need the old PD code that was before 3-20:

You then need to have the older PD dialog as before with no changes: although, I
believe Launch file seems aways greyed out/doesn't work.  This would allow the
user to have the DM open if then chosen from Tools > DM, but canceling or
closing the PD will get rid of the download for good.

I guess the only question I have is why do we need PD's if we can provide all
the same functionality with the DM?

Comment 20

[Blake Ross]

We have to fix this for machv, else new download managers will keep popping up
when you start a download (and, worse yet, only the newest instance will be
functional).  I already have a fix for this in hand, but since it was checked in
with an unrelated (problematic) patch, it got backed out. I'll be checking it
back in soon.

Comment 21

[Peter Trudelle]

Blake, where is the newsgroup discussion you referred to in comment #14?  Is
there an updated spec for any of this?

Comment 22

[Blake Ross]

Marlon is putting together a spec for this and then I will post it to the
newsgroups.

The bug for the pref panel is bug 132440.  What this bug reports isn't contentious.

Comment 23

[Peter Trudelle]

Ok, thanks.  nsbeta1+ ->1.0

Comment 24

[[not reading bugmail]]

blake are you and Marlon going off Ben's Download manager spec page?

http://mozilla.org/xpapps/MachVPlan/DownloadMgr.html

This has a [pause/resume] button on the pic there.. 

anyway build 3-29-16 win32 DM looks good with Jan's checkin.

Comment 25

[[not reading bugmail]]

a note about Launch File: I didn't know that launch file is disabled for .exe
files cause that is always what I download from mozilla's latest directory but I
hadn't downloaded anything else other than .exe files for some time.  So it
works for file types I guess that Mozilla handles.  Anyway this has nothing to
do with the bug problem here.. but just wanted to up to date this as I had
mentioned it here.

Comment 26

[Peter Trudelle]

ADT2

Comment 27

[Kai Lahmann (is there, where MNG is)]

OS = Windows 2000 ?
Well, under Linux you also can open multiple Download Managers

Comment 28

[Blake Ross]

Attachment: patch

Comment 29

[timeless]

that looks fine, but should failure to get the window-mediator be fatal? before 
this patch, we didn't mind if it wasn't there :)

Comment 30

[Ben Goodger (use ben at mozilla dot org for email)]

Comment on attachment 79221 [details] [diff] [review]
patch

SR=[e-mail]

Comment 31

[Blake Ross]

Comment on attachment 79221 [details] [diff] [review]
patch

r=hixie

Comment 32

[Blake Ross]

checked in on trunk.

Comment 33

[:deanis74]

Any reason this isn't marked as fixed?  Because I'll verify it once it is,
everything works well on 2002042209.

Comment 34

[Blake Ross]

I need adt1.0.0+ to checkin on the branch.

Comment 35

[sairuh (rarely reading bugmail)]

...and i'll verify this on the branch when/if it's checked in there.

Comment 36

[scottputterman]

Do you get a new download manager each time a download occurs or just if you
launch it through the tools menu more than once?  If it's just when you do the
tools menu, what bugs happen because you have two download managers open?

Comment 37

[Blake Ross]

Once bug 137440 is fixed, a new download manager will open for each download. 
But only the most recently opened download manager will work.  Thus, this is
critical to fix.

Comment 38

[Jaime Rodriguez, Jr.]

Nice work Blake. Let's get this on on the RTM train. adt1.0.0-/[adt2 RTM], since
bug 137440 was also adt1.0.0-.

Comment 39

[Blake Ross]

Marking fixed (it's checked in on the trunk). People say that's what I'm
supposed to do, and that adt's magical keywords will keep it on their radar...

Comment 40

[sairuh (rarely reading bugmail)]

vrfy'd fixed using 2002.04.29-trunk commercial bits. only get one instance of
the dl mgr. selecting Tools > Download Manager brings the window to front if
it's already up; initiating another download just adds that download to the
current dl mgr window (or brings it up if it's not open).

adding verified1.0.0 kw.

Comment 41

[sairuh (rarely reading bugmail)]

ugh, brain fart. trunk, not branch, trunk, not branch...

Comment 42

[Jaime Rodriguez, Jr.]

changing adt1.0.0 to adt1.0.1, to get on adt's radar.

Comment 43

[sairuh (rarely reading bugmail)]

really marking vrfy'd fixed [for the TRUNK].

Comment 44

[scottputterman]

adding adt1.0.1+ per ADT.  Please get drivers approval before checking in.

Comment 45

[Judson Valeski]

please checkin to the 1.0.1 branch. once there, remove the "mozilla1.0.1+"
keyword and add the "fixed1.0.1" keyword.

Comment 46

[Blake Ross]

Fixed on branch.

Comment 47

[sairuh (rarely reading bugmail)]

vrfy'd fixed on the branch using 2002.07.02.0x-1.0 comm bits on linux rh7.2,
win2k and mac 10.1.5.