159038 - Using -turbo with other cmd line options disables secondary processes

Status: VERIFIED FIXED

(Core Graveyard :: QuickLaunch (AKA turbo mode), defect)

Description

[Bill Law]

This is the Bugzilla equivalent of bugscape bug 17912.  The bug is in Mozilla
code so I'm opening a bug here.  The bug only occurs on the branch (not sure why
that is) so the fix won't be checked in on the trunk.  The fix already has
approval (granted in the bugscape bug).

Here's the comments from the bugscape bug:

Every time I turn on my computer, our IM pops up.  Most of the time I don't
notice this and try to click on the NS 7.0 desktop icon to launch a browser
window.  When I do this, the standalone IM window pops up instead of a browser.
 The only way for me to get a browser is to go through the IM UI.  The desktop
icon should work.



------- Additional Comment #1 From Jaime Rodriguez, Jr. 2002-07-18 08:21 -------

I've been able to reproduce this, and its a pretty bad user experience, if the
person doesn't now an other way to launch the browser (i.e. "N" Logo or Window
menu). We should fix this before we ship. nsbeta1+/ADT1



------- Additional Comment #2 From [e-mail] 2002-07-18 08:42 -------

This may be complicated by the fact that you already do have the browser
running; it is just in IM mode. Perhaps the OS is unable to make that
distinction. Does IM raise to the front when you click on the icon?



------- Additional Comment #3 From Scott Putterman 2002-07-18 08:50 -------

Yes, IM comes to the front.



------- Additional Comment #4 From Lori Kaplan 2002-07-18 09:44 -------

I have this sitation too. But I always forget and close the IM window which then
closes quicklaunch and then I have to launch fm the the desktop icon. We should
figure out how to handle the interaction between IM launch on OS reboot and
QuickLaunch and desktop icon interoperablity. Scott-would you be happy if when
you clicked the desktop icon the browser window launched?



------- Additional Comment #5 From Scott Putterman 2002-07-18 09:50 -------

Yeah.  That's what I want.  It's pretty easy to ignore the IM window that's
opened.  And when I click on the NS 7.0 icon on the desktop when it brings the
IM window to the front I always suffer a moment of frustration when I realize
that I have to use the IM UI to access the browser.  I have it set so that the
browsers opens when I start up (I'm glad it doesn't do that when IM launches)
but since I have it set, I'd expect it to work when clicking on the desktop
icon.  I assume the same happens when using the Start Menu item.



------- Additional Comment #6 From Paul Wyskoczka 2002-07-18 09:52 -------

I'm unable to use any shortcuts like Ctrl 1 to start the browser, mail,
composer, etc.  You have to use the menu item to start any of the apps.



------- Additional Comment #7 From [e-mail] 2002-07-18 10:14 -------

This sounds like bug 17850 which Lisa found last week.



------- Additional Comment #8 From Joseph Elwell 2002-07-18 10:15 -------

I'm investigating.



------- Additional Comment #9 From [e-mail] 2002-07-18 10:15 -------

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



------- Additional Comment #10 From Todd Pringle 2002-07-18 10:46 -------

Yep, I agree this is something we need to do for rtm.  I didn't notice it
because I always use the IM UI to launch other components, but I suspect that
your behavior Scott is the more common one by far.



------- Additional Comment #11 From Joseph Elwell 2002-07-18 10:51 -------

This isn't specific to AIM. If I launch "netscp -mail -turbo" I see the same 
problem.

cc'ing morse and law who made a lot of changes to the nsNativeAppSupportWin.cpp 
in the last month or so.



------- Additional Comment #12 From Joseph Elwell 2002-07-18 10:53 -------

Found it. We need to back out the fix to bugzilla bug 147223, "URL bar doesn't 
function if launched too quickly in turbo mode". Or work on a better solution.



------- Additional Comment #13 From Lisa Chiang 2002-07-18 10:55 -------

What is "netscp -mail -turbo" equivalent to in the UI?   Is there a way to get
this combination besides typing it in at runtime?



------- Additional Comment #14 From Joseph Elwell 2002-07-18 11:13 -------

There is no ui equivalent. If you type in "mozilla -mail -turbo" you will see
this bug as well in Mozilla.



------- Additional Comment #15 From Joseph Elwell 2002-07-18 11:25 -------

If we launch with -mail -turbo, this line should succeed,
http://lxr.mozilla.org/seamonkey/source/xpfe/bootstrap/nsNativeAppSupportWin.cpp#2431

Which would prevent this bug. But that's not happening. We need to ensure
mShouldShowUI is set when we launch other applications.



------- Additional Comment #16 From Stephen P. Morse 2002-07-18 12:35 -------

I'm looking into this.  Should have a patch shortly.



------- Additional Comment #17 From Stephen P. Morse 2002-07-18 13:15 -------

I'm unable to reproduce this with a trunk build from this morning.  Is this a 
branch problem only.

I did mozilla -mail -turbo and I indeed got into the mail program.



------- Additional Comment #18 From Bill Law 2002-07-18 13:31 -------

Steve, I was able to reproduce it on a branch mozilla build.  I suspect the bug
is on the trunk also.  The problem isn't that mail (aim, etc.) don't start.  It
is that using other program icons fail to open browser windows (or mail, if you
start with -aim).

The problem is that mInitialWindow is getting set when we open the special Nav
window when we start turbo mode.  But it is never getting reset because we never
get a call to OnLastWindowClosing (for that window, or any other).  The fix is
to  not open the special Nav window when we've already opened another window
(that will also improve the performance of opening the window the user asked for).

Here's the patch I came up with to fix this:

Index: nsNativeAppSupportWin.cpp
===================================================================
RCS file: /cvsroot/mozilla/xpfe/bootstrap/nsNativeAppSupportWin.cpp,v
retrieving revision 1.63.6.9
diff -u -5 -r1.63.6.9 nsNativeAppSupportWin.cpp
--- nsNativeAppSupportWin.cpp   16 Jun 2002 01:24:27 -0000      1.63.6.9
+++ nsNativeAppSupportWin.cpp   18 Jul 2002 20:17:20 -0000
@@ -2250,10 +2250,20 @@

     if (mShouldShowUI) {
         // We dont have to anything anymore. The native UI
         // will create the window
         return NS_OK;
+    } else {
+        // Sometimes a window will have been opened even though mShouldShowUI i
s false
+        // (e.g., mozilla -mail -turbo).  Detect that by testing whether there'
s a
+        // window already open.
+        nsCOMPtr<nsIDOMWindowInternal> win;
+        GetMostRecentWindow( 0, getter_AddRefs( win ) );
+        if ( win ) {
+            // Window already opened, don't need this special Nav window.
+            return NS_OK;
+        }
     }

     // Since native UI wont create any window, we create a hidden window
     // so thing work alright.



------- Additional Comment #19 From Stephen P. Morse 2002-07-18 13:57 -------

Bill, if I'm understanding you correctly, the problem occurs if you do

   mozilla -mail -turbo

and then attempt to launch mozilla again using an icon.  Well I can't even get 
that far with my trunk build because I crash after the mail program comes up.  
I'm pulling a branch build to see what that does.



------- Additional Comment #20 From Joseph Elwell 2002-07-18 16:06 -------

r=jelwell for that patch if you need it.



------- Additional Comment #21 From Stephen P. Morse 2002-07-18 18:09 -------

I finally got my branch build finished and now I can reproduce the problem.  
Applying law's patch does fix it.  And I agree that this is the correct patch.



------- Additional Comment #22 From Jaime Rodriguez, Jr. 2002-07-18 18:18 -------

morse: does this mean you are giving it a r =, too? who can sr= this one, so we
can get it landed on the branch tonight?



------- Additional Comment #23 From Stephen P. Morse 2002-07-18 19:16 -------

My r= isn't needed since it already has one from jelwell.  But for the record 
r=morse



------- Additional Comment #24 From Jaime Rodriguez, Jr. 2002-07-19 02:35 -------

if we get an sr= tonight, we might be able to get adt approval. who can sr= this
one?



------- Additional Comment #25 From [e-mail] 2002-07-19 03:15 -------

sr=syd



------- Additional Comment #26 From Jaime Rodriguez, Jr. 2002-07-19 12:02 -------

adt1.0.1+ (on ADT's behalf) approval for checkin to the 1.0 branch. pls check
this into the 1.0 branch and the trunk asap. thanks!



------- Additional Comment #27 From Joseph Elwell 2002-07-19 16:11 -------

Checked into the branch. Need drivers approval for trunk.



------- Additional Comment #28 From [e-mail] 2002-07-23 14:06 -------

I am still seeing this on WinXP (7/22 branch build) and WinNT (7/23 branch build)



------- Additional Comment #29 From [e-mail] 2002-07-23 14:09 -------

'this' - I mean I am still seeing the problem.
launching with -aim -turbo does not allow me to use context menu to open other
components
with IM set to launch on reboot, I am unable to launch other components from icon



------- Additional Comment #30 From Lisa Chiang 2002-07-23 14:17 -------

reopen (clear fixed1.0.1) per grace's comments.



------- Additional Comment #31 From Bill Law 2002-07-23 20:51 -------

It doesn't look like Joe actually checked this in (branch or trunk).  That would
explain why it isn't fixed, I suppose.

I'll check it in myself, ASAP, on the branch; it doesn't look like Joe got
drivers OK for the trunk yet.



------- Additional Comment #32 From Bill Law 2002-07-23 20:54 -------

Created an attachment (id=7822)
patch (for the record)



------- Additional Comment #33 From Bill Law 2002-07-23 20:55 -------

(From update of attachment 7822 [details])
r=jelwell,morse
sr=syd



------- Additional Comment #34 From Jaime Rodriguez, Jr. 2002-07-23 21:04 -------

adding back the + on ADT's behalf. approval for checkin on the 1.0 branch. pls
check this in tonight, then add the fixed1.0.1" keyword. thanks!

Comment 1

[Bill Law]

Attachment: patch (for the branch)

Comment 2

[Bill Law]

Comment on attachment 92522 [details] [diff] [review]
patch (for the branch)

r=jelwell, morse
sr=syd
a=jaimer

(from corresponding bugscape bug)

Comment 3

[Bill Law]

fix checked in on branch

the problem doesn't seem to exist on the trunk so I'm marking this one fixed.

Comment 4

[timeless]

thanks for publishing, but technically syd isn't an sr in our world.

Comment 5

[Grace Bush]

branch 20070724
no adt keywords, marking verified