143267 - Tools > Download Manager menu doesn't focus already open Download Manager

Status: VERIFIED FIXED

(SeaMonkey :: Download & File Handling, defect)

Description

[:deanis74]

Steps to reproduce:

1. Open a navigator window
2. Tools > Download Manager
3. Download Manager window opens
4. Switch back to the window opened in step 1
5. Tools > Download Manager

Expected Results: Download Manager window that opened in step 3 is brought to
the front

Actual Results: nothing happens

Build: 2002050904 on WinNT

Comment 1

[:deanis74]

Forgot to mention that this was most likely caused by the check-in for bug 142766.

Comment 2

[sairuh (rarely reading bugmail)]

also see this on mac os 10.1.4 and linux rh7.2 ->all/regression.

Comment 3

[Peter Trudelle]

nsbeta1+/adt2 per Nav triage team.

Comment 4

[Christopher Aillon (sabbatical, not receiving bugmail)]

Attachment: Initial patch

This patch fixes the bug, although not necessarily in the best way.  We'll end
up checking for whether or not there is an active download manager window twice
if there is no active window and we click on the menu item.  We may want to add
a Focus() method to nsIDownloadManager and call that when we want to focus an
existing window (returning true if we focused the window and false if there is
no window to focus or something) or possibly something different... Failing
that, we should update all callers of nsIDownloadManager::Open() to first check
whether or not we want to physically open the window or merely focus it, or
neither...

Comment 5

[Blake Ross]

I do think this is the best way.  This is the patch I had in my tree. Thanks.

Comment 6

[Blake Ross]

-> caillon, do you want to drive this? if not, I will. We basically have the
same fix.

Comment 7

[Christopher Aillon (sabbatical, not receiving bugmail)]

sure blake.  wanna r/sr= it?  I'll find a second reviewer tomorrow.

Comment 8

[jag (Peter Annema)]

This is how the bookmarks code does it:
http://lxr.mozilla.org/seamonkey/source/xpfe/browser/resources/content/navigator.js#934

Comment 9

[Blake Ross]

Comment on attachment 85068 [details] [diff] [review]
Initial patch

sr=blake

Comment 10

[Samir Gehani]

Comment on attachment 85068 [details] [diff] [review]
Initial patch

r=sgehani

Comment 11

[Christopher Aillon (sabbatical, not receiving bugmail)]

Checked into the trunk.

Comment 12

[sairuh (rarely reading bugmail)]

hm, focus still doesn't go back to the dl mgr window for me, using 2002.06.10
commercial trunk bits (linux rh7.2, win2k, mac 10.1.5).

reopening...unless there's another bug that's covering a newer, similar issue?

Comment 13

[:deanis74]

Works properly for me using 2002061008.

Comment 14

[Christopher Aillon (sabbatical, not receiving bugmail)]

sairuh, I can reproduce that on Linux only.  However, that doesn't seem to be
our problem.  I call focus() on the window and it raises it... I'm not exactly
sure why the focus doesn't move to the Download Manager.  That is because of
some unrelated bug in the DOM somewhere (it does the same thing for popup
windows on Linux too).  I'll look for any existing bug and file a new one if
needed.  This bug though should be closed since we do our part.

Resolving fixed again.

Comment 15

[Christopher Aillon (sabbatical, not receiving bugmail)]

Blizzard: is this intentional behavior?  On linux, doing window.focus() will
raise the window, but not focus it.  Bryner says that he is pretty sure that
this is the way it should work... Should I file a bug on it?

Comment 16

[sairuh (rarely reading bugmail)]

caillon Clarified All for me. ignore comment 12, as this is indeed fine on all
platforms (ie, bringing the dl mgr window to the front) on the trunk.

nominating for the branch.

Comment 17

[Christopher Blizzard (:blizzard)]

That is correct.  window.focus() should never grab active focus on Linux.  It
should only raise the window.

Comment 18

[:deanis74]

Verifying based on my comment 13.

Comment 19

[Blake Ross]

This has had the adt1.0.1 keyword since 6/13, how is it missing ADT's radar? How
do I get it plussed?

Comment 20

[scottputterman]

adding adt1.0.1+.  Please get drivers approval and land it so it makes the 6/25
branch build.

Comment 21

[Asa Dotzler [:asa]]

Comment on attachment 85068 [details] [diff] [review]
Initial patch

a=asa (on behalf of drivers) for checkin to the branch.

Comment 22

[Asa Dotzler [:asa]]

please chance the mozilla1.0.1+ keyword to fixed1.0.1 when this lands. thanks.

Comment 23

[Blake Ross]

Fixed on branch.

Comment 24

[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,