8002 - Tasks|Navigator opens a NEW Navigator window

Status: RESOLVED WORKSFORME

(Core :: XUL, defect, P3)

Description

[Ninoschka Baca]

Build 1999060909M7 Win32/NT4, Linux, Mac 8.5

Problem:
In Messenger select Tasks/Navigator and a Navigator window opens.
Each time this step is repeated a NEW Navigator window appears. I
would have expected the application to switch to Navigator, not
create a new window.

In Navigator select Tasks/Messenger and a Messenger window opens.
Repeat this step and a NEW Messenger window appears.

> > > Is Bug# 2652 associated with this? If not, then I should log a bug
> > > but what Component should be chosen since Navigator and Messenger
> > > appear to have a similar problem?

Comment 1

[don]

David, please triage this ...

Comment 2

[don]

*** Bug 8005 has been marked as a duplicate of this bug. ***

Comment 3

[don]

OK, that looks like a bug alright.  Can we fix this one for M7?

Comment 4

[davidm]

Until I am able to pass a nsIDOMWindow around there is no good solution to this
bug

Comment 5

[chris hofmann]

moving to m8 after discussion with don

Comment 6

[davidm]

move to m9 as I am no closer to fixing this bug

Comment 7

[davidm]

fix checked in

Comment 8

[rubydoo123]

Selecting Navigator from the menu toggles to the open Nav window. Verified on
mac and win95 -- it is still broken on Linux.

Comment 9

[davidm]

Linux doesn't appear to bring the window to the front after a call to
window.focus(). I am guessing that gtk is not bring the window to the front
after a call to Show. Reassigning

Comment 10

[Peter Trudelle]

reassigning to pavlov as p3 for m10, cc danm

Comment 11

[Dan M]

The JS function window.focus() is supposed to bring the affected window to the front
and make it visible if it wasn't.  Last time I checked, only Windows was doing this.
Personally, I think it's silly, but the spec insists, and all platforms should behave
the same way.

Comment 12

[Peter Trudelle]

mass-moving most m11 bugs to m12

Comment 13

[Peter Trudelle]

Mass-moving non-PDT+ bugs to M13

Comment 14

[Stuart Parmenter]

this is a bit strange.  if i make nsIWidget::Show() bring the window to the
front, the window gets brought to the front whenever you mouse in to the window

Comment 15

[Stuart Parmenter]

*** Bug 25740 has been marked as a duplicate of this bug. ***

Comment 16

[Claudius Gayle]

*** Bug 32219 has been marked as a duplicate of this bug. ***

Comment 17

[Claudius Gayle]

correcting the OS field. All the dupes are linux only and checking with 03/21 builds shows me that this is Linux only as well.

Comment 18

[Marko Macek]

Tasks|Navigator should open a new (or empty) navigator window.
Tasks|Mail should open new view of Inbox if there is no existing one.

My duplicate bug was complaining about the fact that the navigator window was
not raised at all.

The behaviour of netscape where it reuses browser windows from certain places
should be made optional (or completely removed).

Comment 19

[Peter Trudelle]

Mass-moving most M15 bugs to M16

Comment 20

[Peter "jag" Annema]

Bug topic drift? From "Tasks->Messenger opens a new window instead of the open 
one" to "JS window.focus() doesn't work on Linux" :-) And I'm going back.

What are the specs for the Tasks->* menu?
From Nav4.72's behaviour and what's implied in earlier comments, it seems to be 
this:
if (the selected task is a different type than what's on top right now) {
  if (there's no instance of the task) {
    create one;
  } else {
    switch to the most on top for that task;
  }
} else {
// the selected task is the same type as what's on top right now
  if (it's of type navigator or editor) {
    if (there's only one instance) { // inconsistent behaviour...
      create a new instance;
    } else {
      switch to next instance in list, wrap if needed; // list ordered by Z?
    }
  } else {
    // composer or addressbook, we can only have one instance; do nothing
  }
}

There's a bug preventing this behaviour. To reproduce:
Start Mozilla
Tasks -> Navigator (opens a second Navigator window)
Tasks -> Navigator (switches back to the first)
Tasks -> Navigator (switches back to the second)
Tasks -> Navigator : we're stuck on the second.

Manually switching between from the second to the first and back and then Tasks 
-> Navigator gives the same result.

Adding some tests in tasksOverlay.js:CycleWindow(...) revealed that ( 
firstWindow == topWindowOfType ) is true that last time when it shouldn't be.
This is because topWindowOfType is incorrect. The other thing that could be 
incorrect is the next window after firstWindow in the enumeration list. In the 
case of two windows this would mean the next window in that list and the first 
were the same, but testing showed this wasn't the case.
Conclusion: getMostRecentWindow returns incorrect values sometimes.

Another clear hint that getMostRecentWindow sometimes returns an incorrect value 
can be seen this way:
Start Mozilla
Tasks -> Mail
Tasks -> Navigator
Tasks -> Mail
Tasks -> Navigator : opens a new navigator window

This happens because (topWindowOfType != topWindow) is false that last time, 
which is clearly wrong since the topWindow was Mail and 
topWindowOfType(Navigator) was Navigator (duh). Testing shows topWindowOfType in 
that case is the Navigator window, so topWindow is a Navigator window while it 
should've been a Mail window.

P.S. is there some kind of window.getType() and window.getUniqueID() for JS? It 
would've made the above tests a bit easier :-)

Comment 21

[Stuart Parmenter]

*** Bug 32700 has been marked as a duplicate of this bug. ***

Comment 22

[Marko Macek]

The sensible behavior IMO:

Tasks|Navigator activates an empty navigator window (new or existing empty one)
Tasks|Mail activates the Inbox folder (opens a new Mail window if user is not 
viewing the Inbox in the existing one).

Reusing windows is a bad (annoying) idea.

Comment 23

[Peter "jag" Annema]

Attaching a patch which cleans up the CycleWindow() code to be a bit more 
transparant (but no change in functionality). It also cleans up 
toMessengerWindow(), hope you don't mind me smuggling that in with this ;-)

Comment 24

[Peter "jag" Annema]

Attachment: clean-up patch CycleWindow() and toMessengerWindow() in tasksOverlay.js

Comment 25

[Peter Trudelle]

moving to M18

Comment 26

[Peter Trudelle]

mass-moving all bugs to m21 that are not dofood+, or nsbeta2+

Comment 27

[Sebastian Späth]

Adding PATCH keyword for easier querying. If the clean up patch by Peter Annema 
is good, it should be checked in before it got rotten in M21.
Nothing is more annoying than improving stuff senselessly.

Comment 28

[Peter "jag" Annema]

I'll attach a new patch, these files have changed a bit since then.

Comment 29

[Sebastian Späth]

That's exactly why patches, when looking good, should be checked in as soon as 
possible. Sorry for the spam, just had to get rid of it...

Comment 30

[Peter "jag" Annema]

Attachment: [patch] Clean-up of CycleWindow()

Comment 31

[Peter "jag" Annema]

The first patch did some more cleaning up, but most of that's been done in 
one way or another in the mean time, so it's just for CycleWindow now.

Comment 32

[Peter "jag" Annema]

BenGoodger checked this patch in.

Marking fixed.

Comment 33

[Peter "jag" Annema]

Whoops, wrong bug... Reopening. Apologies for the spam.

BenGoodger did check in the clean-up patch though.

There still seems to be a problem somewhere with the Z-ordering of Moz's 
windows, making |getMostRecentWindow(windowType)| return incorrect values.

Comment 34

[Peter Trudelle]

Only switched to Nav window once out of many tries for me today, usually does
nothing.  

Comment 35

[Jesse Ruderman]

See bug 20306 for more discussion about whether reusing windows for Ctrl+1, 
Tasks|Navigator, and the Navigator "taskbar" button makes sense or not.

Comment 36

[Stuart Parmenter]

we need some kind of fix for this

Comment 37

[Bradley Hart]

*** Bug 65729 has been marked as a duplicate of this bug. ***

Comment 38

[Gervase Markham [:gerv]]

Removing patch keyword, as the attached patch has been checked in.

Gerv

Comment 39

[timeless]

*** Bug 71343 has been marked as a duplicate of this bug. ***

Comment 40

[Jesse Ruderman]

Does this bug cover Tasks|Navigator from any window or from any window other 
than a browser window?

Comment 41

[Claudius Gayle]

Jesse,
It would have to be from any window other than a navigator window b/c the expected behavior
from a navigator window is to produce a new navigator window, whereas from any other app the
expected behavior is to raise the existing navigator window and create one iff there isn't already
one.

Comment 42

[Stuart Parmenter]

over to xptoolkit.. maybe danm or dr can take a look at this?

Comment 43

[Peter Trudelle]

->danm in case this represents some window infrastructure problem, although IMO,
deciding whether switch tasks or create a new window should be XPApps or Apps.

Comment 44

[Dan M]

Sure seems like an Apps issue. Regardless, WFM today. (Messenger)Tasks->Navigator 
doesn't open a new Navigator window if one is already open; it just brings it to 
the front.

Comment 45

[John Morrison]

May God have mercy on us all. The 212 bug spam-o-rama is Now!