Closed Bug 414214 Opened 14 years ago Closed 14 years ago

Download Manager window doesn't stay open when browser.download.manager.closeWhenDone is true

Categories

(Toolkit :: Downloads API, defect, P1)

defect

Tracking

()

VERIFIED FIXED
mozilla1.9beta3

People

(Reporter: elmar.ludwig, Assigned: sdwilsh)

References

Details

(Keywords: regression)

Attachments

(3 files, 3 obsolete files)

Tested with a new profile, today's nightly build:

1. Open Preferences and enable the option:
   "Main > Downloads > Close it when all downloads are finished"
2. Open Download Manager window

The download manager appears for a fraction of a second and immediately closes itself again. This makes it impossible to work with the download manager when browser.download.manager.closeWhenDone is enabled, so requesting blocking.

This is a very recent regression, I'm investigating which checkin caused it.

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b3pre) Gecko/2008012704 Minefield/3.0b3pre ID:2008012704
Flags: blocking-firefox3?
Regression range:

2008-01-26_11-27 works
2008-01-26_13-05 fails

Checkins between 2008-01-26 11:27 and 2008-01-26 13:04:

http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&date=explicit&mindate=1201375620&maxdate=1201381499

The Bonsai query shows several changes to the download manager code here, but from looking at the patches, bug 411947 seems the most likely one to have caused this regression.

Note: This is very similar to the problem reported in bug 397935 and it may well be that bug 411947 just made this older bug more visible.
Blocks: 411947
And this happens with Windows also.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b3pre) Gecko/2008012704 Minefield/3.0b3pre ID:2008012704
OS: Linux → All
Hardware: PC → All
Duplicate of this bug: 414463
Duplicate of this bug: 414589
Awesome.
Flags: blocking-firefox3? → blocking-firefox3+
Assignee: nobody → sdwilsh
Priority: -- → P1
Whiteboard: [needs patch]
Target Milestone: --- → Firefox 3 M11
Attached patch v1.0 (obsolete) — Splinter Review
ug...stupid mistake on my part.

Now with more tests!
Attachment #300072 - Flags: review?(mano)
Whiteboard: [needs patch] → [has patch][needs review Mano]
Comment on attachment 300072 [details] [diff] [review]
v1.0

That's what you get for copy and paste.
Attachment #300072 - Flags: review?(mano) → review-
Attached patch v1.1 (obsolete) — Splinter Review
fixed the comment change...
Attachment #300072 - Attachment is obsolete: true
Attachment #300076 - Flags: review?(mano)
Attached patch v1.2 (obsolete) — Splinter Review
one additional comment fix
Attachment #300076 - Attachment is obsolete: true
Attachment #300091 - Flags: review?(mano)
Attachment #300076 - Flags: review?(mano)
Comment on attachment 300091 [details] [diff] [review]
v1.2

please document this behavior in the downloadmanangerui interface.

r=mano.
Attachment #300091 - Flags: review?(mano) → review+
Whiteboard: [has patch][needs review Mano] → [needs new patch][has review]
Attached patch v1.3Splinter Review
Address review comment.
Attachment #300091 - Attachment is obsolete: true
Whiteboard: [needs new patch][has review] → [has patch][has review][can land]
Checking in toolkit/components/downloads/src/nsDownloadManagerUI.js;
new revision: 1.5; previous revision: 1.4
Checking in toolkit/components/downloads/test/browser/Makefile.in;
new revision: 1.2; previous revision: 1.1
Checking in toolkit/components/downloads/test/browser/browser_bug414214.js;
initial revision: 1.1
Status: NEW → RESOLVED
Closed: 14 years ago
Flags: in-testsuite+
Flags: in-litmus-
Resolution: --- → FIXED
Whiteboard: [has patch][has review][can land]
Test isn't liking tinderboxen - disabled for debugging some other time.

Checking in toolkit/components/downloads/test/browser/Makefile.in;
new revision: 1.3; previous revision: 1.2
Flags: in-testsuite+ → in-testsuite?
Duplicate of this bug: 414737
(In reply to comment #13)
> Test isn't liking tinderboxen - disabled for debugging some other time.
> 
> Checking in toolkit/components/downloads/test/browser/Makefile.in;
> new revision: 1.3; previous revision: 1.2
> 

Status: RESOLVED FIXED ?
The DL window still does not open for me if no downloads are in progress with the latest nigtly. Not fixed yet?
(In reply to comment #15)
> Status: RESOLVED FIXED ?
Yes - I *only* backed out the test.  It passes locally (tinderboxen are slow, I didn't want to delay the tree going green on a freeze day...)

(In reply to comment #16)
> The DL window still does not open for me if no downloads are in progress with
> the latest nigtly. Not fixed yet?
build id?
Shawn, I'm currently using Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b3pre) Gecko/2008013004 Minefield/3.0b3pre. Just tested again, still the same.
Me neither, sadly, with Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9b3pre) Gecko/2008013004 Minefield/3.0b3pre; reopening.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
ug - I missed the browser.js file...

attaching a pass with the test fixed shortly
Attached patch v2.0Splinter Review
Tests fixed to not use window.timeout too!
Attachment #300401 - Flags: review?(mano)
Attachment #300401 - Flags: review?(mano) → approval1.9b3?
Attachment #300401 - Flags: approval1.9b3? → approval1.9b3+
Got the file this time.  Test landed as well.

Checking in browser/base/content/browser.js;
new revision: 1.952; previous revision: 1.951
Checking in toolkit/components/downloads/test/browser/Makefile.in;
new revision: 1.4; previous revision: 1.3
Checking in toolkit/components/downloads/test/browser/browser_bug414214.js;
new revision: 1.2; previous revision: 1.1
Checking in toolkit/components/downloads/test/browser/browser_nsIDownloadManagerUI.js;
new revision: 1.2; previous revision: 1.1
Status: REOPENED → RESOLVED
Closed: 14 years ago14 years ago
Flags: in-testsuite? → in-testsuite+
Resolution: --- → FIXED
Test passes on mac only, so backed out again.

The actual fix is checked in this time though...

Checking in toolkit/components/downloads/test/browser/Makefile.in;
new revision: 1.5; previous revision: 1.4
Flags: in-testsuite+ → in-testsuite?
Attached patch v1.0 test fixSplinter Review
OK, follow-up to fix the test.

metaKey isn't used on windows or linux...
Duplicate of this bug: 414973
Verified FIXED using:

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9b3pre) Gecko/2008020104 Minefield/3.0b3pre

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b3pre) Gecko/2008020104 Minefield/3.0b3pre

-and-

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b3pre) Gecko/2008020104 Minefield/3.0b3pre
Status: RESOLVED → VERIFIED
Looks like we should update the tests for this, if indeed it caused bug 413093.
Product: Firefox → Toolkit
You need to log in before you can comment on or make changes to this bug.