65974 - first window after start is below all other open windows

Status: VERIFIED FIXED

(Core :: XUL, defect, P2)

Description

[Robert Pollak]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; m18) Gecko/20010117
BuildID:    2001011704

Reproducible: Always
Steps to Reproduce:
1) Open some application windows
2) Start Mozilla from the start menu
3) Wait until Mozilla has started - do not click anywhere else

Actual Results:  The mozilla window is concealed by the others.

Expected Results:  The mozilla window is in front of other windows.

I know this since I know bugzilla, so there should already be a bug filed - but
I couldn't find it (and I tried!). If you find it, tell me how you did it.

Comment 1

[Blake Ross]

-> danm

Comment 2

[Dan M]

Odd. I can't find an older duplicate bug, either. I guess it's just been one of 
those "well known problems." We've taken a couple of stabs at fixing this one and 
never quite figured it out. And like before, though annoying, I still don't have 
time to work on it. Futuring, marking helpwanted.

Comment 3

[Robert Pollak]

I'd like to try fixing this - but since I never helped with mozilla development
before, I have to get the code and a local build running first, so please be
patient.

Comment 4

[Peter Trudelle]

I don't think this should be futured so quickly.  This is a failure of 
fundamental expected behavior, people affected by this are very likely to not 
know where Netscape went.  Really.  Also, it is an accessiblity issue.

Comment 5

[Robert Pollak]

I have now built mozilla on my machine from 2001-02-10-08-Mtrunk/mozilla-
source.tar.gz, and I have problems reproducing the bug with this.

When I start %MOZ_SRC%\mozilla\dist\WIN32_D.OBJ\bin\mozilla.exe from a cmd 
shell, I see a different behaviour to the installed mozilla:
The splash screen is displayed correctly in front of all windows, and the 
browser window is opened just behind the shell and in front of all other app 
and explorer windows. The shell window gets the focus again in the moment the 
spash screen is closed/the browser window is opened.

Can I build mozilla-win32-installer.exe? If yes: how?

Comment 6

[Robert Pollak]

diary of a newbie:

Today I tried to find the code that opens the browser window.

1) Stepping through mozilla.exe didn't help me - I don't get deeper than the 
event loop in |nsAppShell::Run|.
2) I searched for usages of the Microsoft Foundation Class |CWnd| - but I didn't 
find any relevant ones. Are there other ways to create windows windows?
3) I looked at http://www.mozilla.org/docs/ (RTFM if nothing else helps:), and I 
got the impression that window creation is probably somewhere in xpfe. So I 
browsed the mozilla/xpfe/ source directory, but again couldn't find anything 
useful.

Can anyone give me a hint how to proceed?
(I'm really willing to learn, so I hope the hint is not: Keep away from this, 
you wannabe coder ...)

Comment 7

[Robert Pollak]

Should the keyword 'mozilla1.0' be added to put this bug on the radar?
I think for many people mozilla1.0 will be the first mozilla they see (due to 
the expected press coverage). This bug is very visible immediately at startup. 
And, you know: the first impression counts.
(I increased the severity to 'Minor', since this bug annoys me several times a 
day.)

Comment 8

[John Morrison]

I agree that we need to get this fixed, so adding the mozilla1.0 as suggested,
but I'm not sure if this will make danm's top 10 list of fenestral fixes.

As for where to look, I am naive about this area (window creation), but I 
throw out a couple of starters (well, actually, these are more like personal
guesses than solid advice :-). 

First, no, you won't find any MFC in this code base (aside from perhaps some 
specialized case, and the MFC example of embedding the layout/networking
engine). 

Second, as the saying goes, LXR is your friend. <http://lxr.mozilla.org>. You 
can browse and search through the code this way and figure out where you want
to set a breakpoint. I would start with the native implementation of the window 
class and figure out how it is called and from where.
  http://lxr.mozilla.org/seamonkey/source/widget/src/windows/nsWindow.cpp

You also might look at http://www.mozilla.org/xpfe/xptoolkit/windows.html, if
you haven't already, although some of that information is likely out of date.

Comment 9

[John Morrison]

Well, I see that danm is reshuffling some window opening code at the moment.
You might want to look at the big, shiny new comment at the top of 
  http://lxr.mozilla.org/mozilla/source/xpfe/bootstrap/nsWindowCreator.cpp
for some info on window opening.

Comment 10

[Peter Trudelle]

->moz1.0/p2

Comment 11

[Peter Trudelle]

clearing target for re-triage, bumping severity to normal.  I think we may have
to fix this for 0.9.2 at least.

Comment 12

[Peter Trudelle]

->jag/moz0.9.2

Comment 13

[Asa Dotzler [:asa]]

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

Comment 14

[Mark Hammond [:markh] [:mhammond]]

Attachment: Patch that corrects the foreground window behaviour.

Comment 15

[Mark Hammond [:markh] [:mhammond]]

Adding Bill Law - hoping he can review the attached patch.

Comment 16

[jag (Peter Annema)]

could you, instead of using the WindowMediatorCID, use
NS_RDF_DATASOURCE_CONTRACTID_PREFIX "window-mediator" ?

http://lxr.mozilla.org/seamonkey/source/xpfe/appshell/src/nsAppShellFactory.cpp#72

It's the ContractID, which should generally be preferred over the ClassID (CID).

Otherwise this looks okay to me, comments make sense, and code looks like it
should work, but get some Windows jock to scrutinize this :-)

Patch tested, works as it was said it would, r=jag with the above changes made
(I'll post a patch for that, even).

Comment 17

[jag (Peter Annema)]

I see now you moved that NS_DEFINE_CID line up from lower in the file. Still,
since we're touching it, might as well fix it.

Comment 18

[John Morrison]

I just want to say '*kiss*' :-]

Patch works for me on win2k and on win98, for splash, profilemanager, windows 
integration alert dialogs and browser window, during startup. It even does
a nice 'flashing' in the icon tray, if you intentionally raise another window
while it starts (for prof and browser on 2k, but just for prof on 98; the 
browser window always forces to the top on 98).

Comment 19

[jag (Peter Annema)]

Attachment: [patch] markh's fix with my comments applied

Comment 20

[jag (Peter Annema)]

All yours now :-)

Comment 21

[John Morrison]

(er, oops. minor correction: the windows integration alert does not come to the 
front on 2k, but that's a relatively minor defect, since that dialog will not
typically show for an end user [treat it as a separate bug]).

Comment 22

[saari (gone)]

sr=hyatt

Comment 23

[Markus Hübner]

this might be related to bug 66866 ?

Comment 24

[Mark Hammond [:markh] [:mhammond]]

FYI - there is still a problem when starting a second "instance" of mozilla - 
the newly opened window is not in the foreground.  I've opened bug 85860 for 
that.

Comment 25

[Bill Law]

Excellent work.  Thanks.

Should we try to combine the (now) three sets of code in this module that get
recent windows from the window mediator?  It would have to take a PRUnichar* and
nsIDOMWindowInternal** and could be either a static function or a static member
of nsNativeAppSupportWin.  That's the only improvement I would suggest.  I
should have done that when I wrote the *second* instance of that code, but
better late than never.  BTW, I think we'll have need for that function to fix
some other bugs.

Comment 26

[jag (Peter Annema)]

Attachment: [patch] With Bill's comments applied

Comment 27

[Mark Hammond [:markh] [:mhammond]]

Jag's new patch still works for me :)  So now all we need are one or 2 r=, and 
we are off and running!  Bill - you willing to add your stamp of approval?

Comment 28

[Bill Law]

Certainly; r=law.

Comment 29

[Mark Hammond [:markh] [:mhammond]]

Mail sent to drivers seeking a=

Comment 30

[Asa Dotzler [:asa]]

a= [e-mail] for checkin to the trunk.
(on behalf of drivers)

Comment 31

[Mark Hammond [:markh] [:mhammond]]

Checked in:
/cvsroot/mozilla/xpfe/bootstrap/nsNativeAppSupportWin.cpp,v  <--  
nsNativeAppSupportWin.cpp
new revision: 1.26; previous revision: 1.25

->FIXED :)

Comment 32

[John Morrison]

verified fixed with win32 6/20 build. Thanks Mark!

Comment 33

[timeless]

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