89504 - enabling/disabling quicklaunch should not require a restart

Status: VERIFIED FIXED

(SeaMonkey :: UI Design, defect, P2)

Description

[Jesse Ruderman]

Enabling or disabling Quick Launch should not require a restart of the operating
system or the browser.

Steps to reproduce:
1. Enable Quick Launch in Edit->Prefs->Advanced.
2. Close and restart Mozilla.
Result: Mozilla takes a long time to start up.
Expected: Mozilla should restart quickly.

3. Close and restart Mozilla again.

Result: Mozilla takes a long time to start up.
Expected: Mozilla should restart quickly.

4. Close Mozilla.
5. Restart Windows, or run mozilla.exe -turbo.
6. Restart Mozilla.

Result: Mozilla restarts quickly (good).

(Similar steps for trying to disable Quick Launch)

Comment 1

[sairuh (rarely reading bugmail)]

turbo/quicklaunch ->vishy, qa: paw/jrgm. bill is on sabbatical...

Comment 2

[Paul Chen]

nav triage team:

marking nsbeta1+, p2, and mozilla1.0

Comment 3

[Blake Ross]

--> me

Comment 4

[Blake Ross]

Attachment: prototype

Comment 5

[Conrad Carlen (not reading bugmail)]

r=ccarlen

Comment 6

[Viswanath Ramachandran]

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

Comment 7

[Viswanath Ramachandran]

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

Comment 8

[Viswanath Ramachandran]

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

Comment 9

[Viswanath Ramachandran]

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

Comment 10

[Viswanath Ramachandran]

Blake - have we tested all the cases with this bug, if so we shd get it sr'ed 
and speed it into the tree... 

Comment 11

[Blake Ross]

Conrad is working out an issue with his multiple profiles patch; plan is to land
this after he lands to help avoid conflicts.

Comment 12

[Ben Goodger (use ben at mozilla dot org for email)]

In your code you have (long exprs replaced):

       rv = GetNativeAppSupport(getter_AddRefs(nativeApp));
-      if (NS_SUCCEEDED(rv) && NS_SUCCEEDED(goats) && moreGoats) {
-        // Create special Nav window.
-        nativeApp->StartServerMode();
-        return NS_OK;
-      }
+      // Create special Nav window.
+      if (NS_SUCCEEDED(goats) && moreGoats) {


What is the relevance now of obtaining the return value in:
rv = GetNativeAppSupport(getter_AddRefs(nativeApp));

if the return value is now never used? I'd say it'd allow you to return the
return value with |return rv| at the end of the function, however subsequent
functions promptly clobber whatever value GetNativeAppSupport returned so it's
not really achieving anything ;) 

Comment 13

[Blake Ross]

Fixed.

Comment 14

[Blake Ross]

Paul: please, like, test the crap out of this :-)  You may want to wait until
multiple profile support lands.  We need to see how changing this pref in one
profile affects the other profiles (we think it's going to be global, and are
okay with that).  It's also critical to see how backwards compatible this is,
i.e. what happens if you used turbo in 6.1 and then upgrade.  I believe we still
need an installer-side fix to make all this work properly.

Comment 15

[Blake Ross]

reopening.

Comment 16

[Blake Ross]

Attachment: I can sleep at night with this patch.

Comment 17

[Ben Goodger (use ben at mozilla dot org for email)]

alright. sr=[e-mail]

Comment 18

[Conrad Carlen (not reading bugmail)]

Back to having only one attribute for determining whether we're in server mode
is much better. Some of my code (multiple profile patch) was already tripped up
by the difference. r=ccarlen

Comment 19

[Blake Ross]

Fixed.

Comment 20

[Paul Wyskoczka]

Terri, can you verify this bug for me. I'm up to my ears in reviews. Thanks.

Comment 21

[Terri Preston]

Verified W2k build 2001082203