413093 - With "Remember what I've download" disabled/unchecked (browser.download.manager.retention set to 0), the Download Manager window remains open until clicked/focused

Status: VERIFIED FIXED

(Toolkit :: Downloads API, defect, P3)

Description

[pal-moz]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b3pre) Gecko/2008011815 Minefield/3.0b3pre ID:2008011815

maybe regression between 20080115 - 20080116 Nightly.
http://forums.mozillazine.org/viewtopic.php?p=3222823#3222823

Comment 1

[Reed Loden [:reed]]

Anything in the Error Console?

Comment 2

[pal-moz]

(In reply to comment #1)
> Anything in the Error Console?
> 

nothing.

Comment 3

[pal-moz]

work : 20080115_0848_firefox-3.0b3pre.en-US.win32.zip
not : 20080115_1045_firefox-3.0b3pre.en-US.win32.zip

range : http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&date=explicit&mindate=1200415680&maxdate=1200422699

Comment 4

[Reed Loden [:reed]]

(In reply to comment #3)
> work : 20080115_0848_firefox-3.0b3pre.en-US.win32.zip
> not : 20080115_1045_firefox-3.0b3pre.en-US.win32.zip

Does the patch in bug 412598 fix this?

Comment 5

[Bill Gianopoulos [:WG9s]]

(In reply to comment #4)
> (In reply to comment #3)
> > work : 20080115_0848_firefox-3.0b3pre.en-US.win32.zip
> > not : 20080115_1045_firefox-3.0b3pre.en-US.win32.zip
> 
> Does the patch in bug 412598 fix this?
> 

It does for me.

Comment 6

[pal-moz]

(In reply to comment #4)
> (In reply to comment #3)
> > work : 20080115_0848_firefox-3.0b3pre.en-US.win32.zip
> > not : 20080115_1045_firefox-3.0b3pre.en-US.win32.zip
> 
> Does the patch in bug 412598 fix this?
> 

20080121_1635_firefox-3.0b3pre.en-US.win32.zip

not fixed.

Comment 7

[Bill Gianopoulos [:WG9s]]

I guess this one is not a dupe then.

Comment 8

[Bill Gianopoulos [:WG9s]]

Or perhaps that is not a complete fix for all of the regressions based on https://bugzilla.mozilla.org/show_bug.cgi?id=413200#c17

Comment 10

[Stephen Donner [:stephend] Not actively reading bugmail]

Confirming, based on dupes; although I can't yet reproduce this, I'm working on it, and we should block until we're sure it's fixed.

Comment 11

[Bill Gianopoulos [:WG9s]]

It is possible the patched attached to Bug 413200 will fix this as well.

Comment 12

[Bill Gianopoulos [:WG9s]]

(In reply to comment #6)
> (In reply to comment #4)
> > (In reply to comment #3)
> > > work : 20080115_0848_firefox-3.0b3pre.en-US.win32.zip
> > > not : 20080115_1045_firefox-3.0b3pre.en-US.win32.zip
> > 
> > Does the patch in bug 412598 fix this?
> > 
> 
> 20080121_1635_firefox-3.0b3pre.en-US.win32.zip
> 

I have builds on my server that include the fix from bug 413200 which is the new and improved version of the bug 412598 fix.  Could you Try a build from http://www.wg9s.com/mozilla/firefox/ to determine if the patch for bug 413200 will fix this?
> not fixed.
> 

Comment 13

[pal-moz]

(In reply to comment #12)
> I have builds on my server that include the fix from bug 413200 which is the
> new and improved version of the bug 412598 fix.  Could you Try a build from
> http://www.wg9s.com/mozilla/firefox/ to determine if the patch for bug 413200
> will fix this?

not fixed.

Comment 15

[pal-moz]

sometimes DM window does not appear while downloading.

Comment 16

[Shawn Wilsher :sdwilsh]

Gavin - you were seeing this (I have yet to reproduce it).  Do you think you might be able to take this?

Comment 17

[paranoid654321]

I'm running XP Pro x64 SP2 and have this bug whereby the Download Manager window will not close automatically after all downloads have finished, even though "Options->Main->Close it when downloads are finished" is enabled. This happens every time.

When all downloads have finished and the DM window is open, then starting another download will cause the DM window to close immediately. The "Downloads Complete" tray pop-up notification will still be displayed when the download finishes. Starting another download will cause the DM window to open, but it will not automatically close when the download(s) have finished. 

If the mouse cursor is moved over the client area (ie. not the title bar) of the DM window when all downloads have completed, the DM window will automatically close as soon as the cursor moves back onto the window border or title bar.

Comment 18

[Shawn Wilsher :sdwilsh]

(In reply to comment #17)
I don't think that's the issue reported by the original reporter.  Could someone please clarify what exactly is filed in this bug report?

Comment 19

[paranoid654321]

In addition to my post, the issue I describe also happens with the print progress dialog (and probably all progress dialogs, although I haven't tested others)

Comment 20

[pal-moz]

(In reply to comment #18)
> (In reply to comment #17)
> I don't think that's the issue reported by the original reporter.  Could
> someone please clarify what exactly is filed in this bug report?
> 

(1) DM should be close soon after finish downloading, if check ON "Close it when downloads are finished"
(2) DM should be always displayed when downloading.
(3) "download complete" dialog should always be displayed when finish downloading.

Comment 21

[paranoid654321]

I've found the cause (at least for my system) - If browser.download.manager.retention is set to 0 (ie. Clear Download History at Shutdown), then the download manager will not close automatically. I also have it set to clear the private data at shutdown, without confirmation.

Can someone confirm ?

Comment 22

[Stephen Donner [:stephend] Not actively reading bugmail]

(In reply to comment #21)
> I've found the cause (at least for my system) - If
> browser.download.manager.retention is set to 0 (ie. Clear Download History at
> Shutdown), then the download manager will not close automatically. I also have
> it set to clear the private data at shutdown, without confirmation.
> 
> Can someone confirm ?

Here's what I've seen:

1) The "Downloads" window can get 'stuck' in the Windows system taskbar if I set browser.download.manager.retention to 0, but that once I click (i.e. set focus to) the "Downloads" window, it disappears
2) It also remains if that same pref is set but the DM window is left open, and it has focus; again, an explicit click makes it vanish

Pal-moz: what's the value of your browser.download.manager.retention pref?  If "0", does changing it back to its default of "2" fix this bug?

Comment 23

[Stephen Donner [:stephend] Not actively reading bugmail]

(For "1)", I meant to add that "if the 'Downloads' window is minimized..")

Short story: explicit focus clicks make it go away for me, using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b4pre) Gecko/2008021504 Minefield/3.0b4pre (but of course that's a bug that it requires doing so).

Comment 24

[pal-moz]

(In reply to comment #22)
> Pal-moz: what's the value of your browser.download.manager.retention pref?  If
> "0", does changing it back to its default of "2" fix this bug?
> 

now set 0 (check OFF "Remember what I've downloaded").

and set 2 (check ON "Remember what I've downloaded") seems to be fine.

Comment 26

[Paweł Smoliński]

I've the same problem on my Linux machine. Maybe this helps: the download manager window closes when it gets some event from window manager (cursors moved in/over the window area, mouse click, minimize, focus on).

Comment 27

[Paweł Smoliński]

I think the bug was created when fixed this: https://bugzilla.mozilla.org/show_bug.cgi?id=414214

Comment 28

[pal-moz]

(In reply to comment #27)
> I think the bug was created when fixed this:
> https://bugzilla.mozilla.org/show_bug.cgi?id=414214
> 

I think no.
see comment #3

Comment 29

[Stephen Donner [:stephend] Not actively reading bugmail]

Changing summary from "DM partially broken (sometimes not close soon after finish downloading, "download complete" dialog not appears)" to "With browser.download.manager.retention set to 0, the Download Manager window remains until clicked/focused" so I and others can more easily find it.

Comment 30

[pal-moz]

(In reply to comment #29)
> Changing summary from "DM partially broken (sometimes not close soon after
> finish downloading, "download complete" dialog not appears)" to "With
> browser.download.manager.retention set to 0, the Download Manager window
> remains until clicked/focused" so I and others can more easily find it.
> 

if change summary, "browser.download.manager.retention set to 0" should be "check OFF "Remember what I've downloaded"" ?

Comment 31

[Stephen Donner [:stephend] Not actively reading bugmail]

(In reply to comment #30)
 
> if change summary, "browser.download.manager.retention set to 0" should be
> "check OFF "Remember what I've downloaded"" ?

Done; it might be too long now (or it's already been, in which case it's even longer now :-), but this should allow everyone to find it.

Comment 32

[Shawn Wilsher :sdwilsh]

Attachment: v0.1

So, window.close() is now being called, but the window isn't actually being closed.  I'm not really sure what's up with that sadly :/

I'm not sure who to bug about that either - guidance will be appreciated here

Comment 33

[Shawn Wilsher :sdwilsh]

Attachment: v1.0

This works - just doesn't pass anything after the first test run.  window.close() is being called, it's just not closing the window.  Weird.

Comment 34

[Shawn Wilsher :sdwilsh]

note: jst thinks the closing issue may be related to bug 413200.

Comment 35

[pal-moz]

(In reply to comment #34)
> note: jst thinks the closing issue may be related to bug 413200.
> 

from https://bugzilla.mozilla.org/show_bug.cgi?id=413200#c11 ,

maybe bug 413200 have same regression range as this bug.
(both bug have been regressed from same checkin, bug 352791 )
see this bug's comment#3

Comment 36

[Ed Lee :Mardak]

Comment on attachment 306557 [details] [diff] [review]
v1.0

>       autoClose = pref.getBoolPref(PREF_BDM_CLOSEWHENDONE);
>+      if (pref.getIntPref(PREF_BDM_RETENTION) != 2)
>+        autoClose = true;
Why should autoClose be true if the retention is 1 (remove when the browser exits)?

Why not just check == 0?

Also, you can just do this instead of an extra if:

autoClose = pref.getBoolPref(..) ||
  pref.getIntPref == 0;

Comment 37

[Shawn Wilsher :sdwilsh]

Attachment: v1.1

Comment 38

[Ed Lee :Mardak]

Comment on attachment 306588 [details] [diff] [review]
v1.1

r=Mardak

>-      autoClose = pref.getBoolPref(PREF_BDM_CLOSEWHENDONE);
>+      autoClose = pref.getBoolPref(PREF_BDM_CLOSEWHENDONE) ||
>+                  pref.getIntPref(PREF_BDM_RETENTION) == 0)
s/)$/; for the last line

Comment 40

[pal-moz]

seems to be fine after 20080303_1836_firefox-3.0b5pre.en-US.win32.zip

fixed by/with bug 413200 ?

would someone can confirm ?

Comment 41

[Stephen Donner [:stephend] Not actively reading bugmail]

Fixed by bug 413200, indeed (I just tested using Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9b5pre) Gecko/2008030405 Minefield/3.0b5pre).

Comment 42

[Shawn Wilsher :sdwilsh]

Edward - do you know *how* this is now fixed?  I don't see code anywhere that actually closes us...

Comment 43

[Ed Lee :Mardak]

Hrmm... fyi, the mochi test doesn't pass even with bug 413200 fixed.

Comment 44

[Shawn Wilsher :sdwilsh]

reopening per comment 43.

Comment 45

[Stephen Donner [:stephend] Not actively reading bugmail]

/me shrugs

It's working for me with trunk nightlies on Mac, Windows, and Linux.

https://bugzilla.mozilla.org/show_bug.cgi?id=413200#c45

Do our tests need to be updated?

Comment 46

[pal-moz]

(In reply to comment #20) 
> (1) DM should be close soon after finish downloading, if check ON "Close it
> when downloads are finished"
> (2) DM should be always displayed when downloading.
> (3) "download complete" dialog should always be displayed when finish
> downloading.
> 

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b5pre) Gecko/2008030607 Minefield/3.0b5pre ID:2008030607

no problem with (1) and (2).

problem with (3).
sometimes/often dialog is not displayed.

Comment 48

[Daniel Goetz]

I have this problem as well on 3b3/3b4 on the following:
XP SP2 (x2)
Vista x86 (x1)
Vista x64 (x1)

Comment 49

[Shawn Wilsher :sdwilsh]

Right because the fix for it didn't make it in for b4...

Comment 50

[Ed Lee :Mardak]

Did you still want to land this shawn?

Comment 51

[Shawn Wilsher :sdwilsh]

(In reply to comment #50)
> Did you still want to land this shawn?
I want to land the test, but you indicated that it still wasn't working.  We *need* a working test on this, and as far as I'm concerned, that's blocking.  I won't have cycles until at least Sunday, so if you want to take what I've got and fix it the rest of the way, that'd be great.

I still don't know how it works either...

Comment 52

[Ed Lee :Mardak]

What's wrong with just landing attachment v1.1?

Comment 53

[Ed Lee :Mardak]

Wait, this bug is assuming browser.download.manager.closeWhenDone is true?

Comment 54

[Ed Lee :Mardak]

Yeah, probably can just land the testcase and not the code changes. The testcase needs to set closeWhenDone to true instead of the default false.

Comment 55

[Shawn Wilsher :sdwilsh]

Would you be willing to fix that and land (if it's just the test)?  tests are NPOTB, and you have module owner approval to land the test ;)

I *really* won't have the cycles until at least Sunday - even for something as simple as that.

Comment 56

[Ed Lee :Mardak]

Attachment: testcase v2

Updated Shawn's testcase to work.

Comment 57

[Ed Lee :Mardak]

Checking in toolkit/mozapps/downloads/tests/browser/Makefile.in;
/cvsroot/mozilla/toolkit/mozapps/downloads/tests/browser/Makefile.in,v  <--  Makefile.in
new revision: 1.19; previous revision: 1.18
done
RCS file: /cvsroot/mozilla/toolkit/mozapps/downloads/tests/browser/browser_bug_413093.js,v
done
Checking in toolkit/mozapps/downloads/tests/browser/browser_bug_413093.js;
/cvsroot/mozilla/toolkit/mozapps/downloads/tests/browser/browser_bug_413093.js,v  <--  browser_bug_413093.js
initial revision: 1.1
done

Comment 58

[Shawn Wilsher :sdwilsh]

Stephen - if we have a litmus test for this, feel free to remove it now! :)

Comment 59

[Stephen Donner [:stephend] Not actively reading bugmail]

(In reply to comment #58)
> Stephen - if we have a litmus test for this, feel free to remove it now! :)

Sadly, this is one test area I missed, but I'm happy to find it has a unit test.

Pal-moz, would you do the honors of verification?

Comment 60

[pal-moz]

Attachment: screenshot

20080311_1535_firefox-3.0b5pre.en-US.win32.zip

[Bonsai Query]
http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&date=explicit&mindate=1205269500&maxdate=1205274899

file icon change to folder icon, intended or regression ?

and ow test to some downloads.

Comment 61

[Stephen Donner [:stephend] Not actively reading bugmail]

There's a separate bug on file for that--see bug 422077--I was only asking for verification of the original bug being fixed (I can confirm, but as the reporter, it means more if you'd do that honors).

Comment 62

[pal-moz]

not fixed. rather terrible.

case (3) "download complete" dialog should always be displayed when finish downloading.

dialog is not displayed, if/when downloading jpg (image) file(s).


(In reply to comment #61)
> There's a separate bug on file for that--see bug 422077--I was only asking for
> verification of the original bug being fixed (I can confirm, but as the
> reporter, it means more if you'd do that honors).
> 

sorry, but WFM with build before 20080311_1535_firefox-3.0b5pre.en-US.win32.zip.
while/after downloading, file icon is folder icon, not change while/after.
so I think a different bug.

Comment 63

[Shawn Wilsher :sdwilsh]

(In reply to comment #62)
> not fixed. rather terrible.
> 
> case (3) "download complete" dialog should always be displayed when finish
that's not *this* bug.  You added it in comment 20, but it's a completely separate issue.

Comment 64

[pal-moz]

OK.
for me, this was fixed on 20080303_1836_firefox-3.0b5pre.en-US.win32.zip
(Comment #40)

Comment 65

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED per comment 45, comment 46, comment 64...

Comment 66

[Sebastian Vendel]

Had no problems with 3.0b5pre.
But the recent released beta 4 has still the bug...

Comment 67

[Shawn Wilsher :sdwilsh]

(In reply to comment #66)
> Had no problems with 3.0b5pre.
> But the recent released beta 4 has still the bug...
Right, like I said in comment 49, and as the target milestone indicates.  This bug is fixed for beta 5.