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

VERIFIED FIXED in mozilla1.0

Status

SeaMonkey
Download & File Handling
VERIFIED FIXED
16 years ago
14 years ago

People

(Reporter: Dean Tessman, Assigned: Christopher Aillon (sabbatical, not receiving bugmail))

Tracking

({regression})

Trunk
mozilla1.0
regression

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [adt2 rtm] custrtm- [verified on trunk])

Attachments

(1 attachment)

(Reporter)

Description

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

Updated

16 years ago
Keywords: nsbeta1
(Reporter)

Comment 1

16 years ago
Forgot to mention that this was most likely caused by the check-in for bug 142766.
also see this on mac os 10.1.4 and linux rh7.2 ->all/regression.
Keywords: regression
OS: Windows 2000 → All
Hardware: PC → All

Comment 3

16 years ago
nsbeta1+/adt2 per Nav triage team.
Keywords: nsbeta1 → nsbeta1+
Whiteboard: [adt2]
Target Milestone: --- → mozilla1.0

Updated

16 years ago
Whiteboard: [adt2] → [adt2 rtm]
Created attachment 85068 [details] [diff] [review]
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

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

Comment 6

16 years ago
-> caillon, do you want to drive this? if not, I will. We basically have the
same fix.
Assignee: blaker → caillon
sure blake.  wanna r/sr= it?  I'll find a second reviewer tomorrow.
Status: NEW → ASSIGNED

Updated

16 years ago
Whiteboard: [adt2 rtm] → [adt2 rtm] custrtm-

Comment 9

16 years ago
Comment on attachment 85068 [details] [diff] [review]
Initial patch

sr=blake
Attachment #85068 - Flags: superreview+

Comment 10

16 years ago
Comment on attachment 85068 [details] [diff] [review]
Initial patch

r=sgehani
Attachment #85068 - Flags: review+
Checked into the trunk.
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED
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?
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Reporter)

Comment 13

16 years ago
Works properly for me using 2002061008.
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.
Status: REOPENED → RESOLVED
Last Resolved: 16 years ago16 years ago
Resolution: --- → FIXED
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?
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.
Keywords: adt1.0.1
Whiteboard: [adt2 rtm] custrtm- → [adt2 rtm] custrtm- [verified on trunk]
That is correct.  window.focus() should never grab active focus on Linux.  It
should only raise the window.
(Reporter)

Comment 18

16 years ago
Verifying based on my comment 13.
Status: RESOLVED → VERIFIED

Comment 19

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

16 years ago
adding adt1.0.1+.  Please get drivers approval and land it so it makes the 6/25
branch build.
Keywords: adt1.0.1 → adt1.0.1+, mozilla1.0.1

Comment 21

16 years ago
Comment on attachment 85068 [details] [diff] [review]
Initial patch

a=asa (on behalf of drivers) for checkin to the branch.
Attachment #85068 - Flags: approval+

Comment 22

16 years ago
please chance the mozilla1.0.1+ keyword to fixed1.0.1 when this lands. thanks.
Keywords: mozilla1.0.1 → mozilla1.0.1+

Comment 23

16 years ago
Fixed on branch.
Keywords: mozilla1.0.1+ → fixed1.0.1
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,
Keywords: fixed1.0.1 → verified1.0.1
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.