When "Turbo mode" is activated Mozilla should be run from Registry, not from Start Menu. The normal way to launch background applications is from : (System Wide Setting) HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\Windows\CurrentVersion\Run Or from (Personal Setting) HKEY_CURRENT_USER\Software\Microsoft\Windows\CurrentVersion\Run Reproducible: Always Steps to Reproduce: Activate turbo mode in installer or advanced preferences Actual Results: A shortcut is created in Startup Menu Expected Results: Mozilla should be start from registry as a background process See comments on bug 81149 This is the way many applications start. ICQ, Yahoo Messenger, RealPlayer, Norton Antivirus, Babylon Translator, and many many more. If the installer is run by an administrator we should set the option for all users (or ask the administrator what he wants to do.) There are some ways to see if a user has admin privileges. But none of them are perfect, and most of them do not work in Windows 9x http://www.mvps.org/vcfaq/sdk/21.htm http://support.microsoft.com/support/kb/articles/q118/6/26.asp The easiest way (and perhaps safest) is to test for access denied errors when creating the registry entry.

I disagree. Using the autostart-approach every user can see what programs will start on system startup easily. Mozilla takes up 16 to 20 MB RAM in Turbomode, and hiding will only make user's life difficult. If you don't want Mozilla's Turbo Mode in autostart you can always hide it in at least 7 different locations in the registry.

->law. xpapps

ccing timeless

The "StartUp folder" is for end users to add or remove applications manually (e.g. Microsoft Office Shortcut Bar). Applications that need to run components at startup should use the Registry Interface. If this behaviour is optional, the application should have a preference option to enable or disable it (like the one in Mozilla Preferences). This is how 99% of Windows applications work More applications that use the registry : WinAmp Agent AOL Instant Messenger MSN Messenger

registry solution is better for me. In this case the bug 87035 should be change to invalid.

With all respect, I totally disagree with this bug. If the two methods for loading an app at Windows startup (Startup folder and registry) are identical regarding to the level of application priority (and I am sure they are), Startup folder method is more honest and much easier for handling if something goes wrong. E.g. uninstaller fails or someone deletes the Mozilla files manually. How do you expect from an average user (even not as ignorant as the "average Joe" example) to remove the registry key? For such a user, regedit (or other registry editors) isn't a toy to play with. Btw, I am very happy that e.g. MSOffice "fast start" feature (OSA.EXE) can be easily removed from the Startup folder, instead of hacking the registry files.

using registry seems to me like native way... to win32 or NT using start up is someting like to write in autoexec.bat in win3.xx... and is there any plans to do something about linux window managers to use something like "turbo mode" in windows?

Personally, I think it should be in the registry, just to avoid extra clutter in the Start menu (imagine if everything used the Startup folder). However it isn't really important. Let's focus on this stuff when the major bugs are fixed. Severity: trivial, anyone?

Personally, I think it should be in the registry, just to avoid extra clutter in the Start menu (imagine if everything used the Startup folder). However it isn't really important. Let's focus on this stuff when the major bugs are fixed. Severity: trivial, anyone?

You could also use HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\Windows\CurrentVersion\RunServices, which enable to load the app at startup without showing in the Close application dialog obtained by pressing CTRL-ALT-DEL. Another solution would be to use the Win.ini LOAD= or RUN= parameters, though this is a bit old-style but would make this setting available for Win 3.1, for what it's worth ;). Anyway, I agree that startup folder is more convenient, but if we go for the registry, I suggest HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\Windows\CurrentVersion\RunServices in order to mimic IE behaviour to be loaded with the OS and not as a separate app.

I'm not 100% sure, but I believe that "RunServices" does not work on NT/2000. I think it only works on Win95/98/ME

--> me

I'm handling the installer part of this, the patches above are for that. Blake has the rest of it.

r=ssu for patch id=48684.

Blake, the ns patch above is assuming that your code will use "Netscape" instead of "Mozilla" for the var name set in the Run key for the ns tree. If you're setting something other than "Netscape", please let Syd know. Other than the above comment, r=ssu for patch id=48685.

Some comments on attachment 48684 [details] [diff] [review] and attachment 48685 [details] [diff] [review] + fTemp = fileExe; + fileExe = getFolder("file:///", fTemp); + newKey = fileExe + ' -turbo'; + winreg.setValueString(subkey, valname, newKey); If fileExe contains any space, all the string must be enclosed between quotes ("). I don't know how "getFolder" works. Perhaps it's already quoted. In line 83 of browser.jst : http://lxr.mozilla.org/seamonkey/source/xpinstall/packager/windows/browser.jst#83 we are trying to see if the current user is admin (or something like that), so we set shortcuts for all users instead the current one. Should we do the same for turbo mode ? I mean, should we create the registry key on HKEY_LOCAL_MACHINE instead of HKEY_CURRENT_USER in that case ?

To respond to the above questions: Notice that the string *is* quoted. In the JavaScript language, single and double quotes can be used for this purpose. As to the other question, I believe that we decided to put this in HKEY_CURRENT_USER. I checked with Sean on this issue before creating the patch.

Daniel, you are correct. The path does not show up quoted when created under the Run registry key. However, I've tested under NT4 (sp6) and Win2k. Under both OS, they seem to be working fine. We'll need to test this under the other OS'es now: Win95, Win98, WinMe, and WinXP. We only want to set this under HKEY_CURRENT_USER. It was decided that we set only under this key for now due to the time constraint in codeing up an appropriate UI to let the user decide between the two (or do both).

I forgot to mention that the path I tested with had spaces in it: c:\program files\mozilla.org\mozilla\mozilla.exe -turbo

In my patch, disregard - appShell->UnregisterTopLevelWindow( 0 ); + appShell->Quit(); While testing my changes, I noticed that danm's change yesterday to ::UnregisterTopLevelWindow() had broken exiting from the turbo systray icon. But I see that bug 98792 is handling this (same patch). Sean, can you take a look at this patch, disregarding the hardcoded "Mozilla" (I still need to find out how to get the value from the app)?

Sean, you are correct. It works ok without the quotes. I think I am a big mouth. I tested this on NT and Win98 and worked fine. I wonder how Windows realize which part of the string is the command and which part are the parameters. If I try to execute the same string (without quotes) from "Start -> Run..." it fails. But doing the same from the registry it works fine. Sorry for the spam.

Overall, looks good. One little cosmetic beef in nsAppRunner.cpp: + PRBool serverMode = PR_FALSE; + nativeApps->GetIsServerMode(&serverMode); + if (!serverMode) { // okay, so it's not -turbo + HKEY key; + LONG result = ::RegOpenKeyEx(HKEY_CURRENT_USER, "Software\\Microsoft\\Windows\\CurrentVersion\\Run", 0, KEY_QUERY_VALUE, &key); Can't this be put into NS_CreateNativeAppSupport() or nsNativeAppSupportWin::Start()? Since we're no longer using the pref service, this can be done earlier, even before XPCOM is inited. It would make for less #ifdef XP_WIN32 cruft in apprunner.

Comment on attachment 48813 [details] [diff] [review] updated patch for app startup/prefs and exit dialog + attribute boolean shouldShowUI; - this could probably use the same amount of comments so graciously provided for the other params. + PRBool mShownTurboDialog; - does this need to be in the base class? Seems pretty specific to the Windows impl. Since those are pretty minor complaints, r=ccarlen

I agree with conrad wrt the mShownTurboDialog member, surely it wouldn't require much to move it into nsNativeAppSupportWin, unless we're planning to offer this same functionality on other platforms (?) Otherwise, sr=ben@netscape.com

Oh, forgot to mention. Use hbox/vbox instead of box with attrs, and autostretch is deprecated. Fix those too.

Comment on attachment 48813 [details] [diff] [review] updated patch for app startup/prefs and exit dialog sr=ben@netscape.com with the above conditions

Yeah I moved it. We'll want to show the dialog on other platforms if they support turbo, though. Also I'll update the xul, it's a little out of date because I just took it from the 0.9.2 branch. Thanks.

Comment on attachment 48813 [details] [diff] [review] updated patch for app startup/prefs and exit dialog a=asa for checkin to 0.9.4

Comment on attachment 48684 [details] [diff] [review] always tell the installer to remove the key sr=blake

Comment on attachment 48685 [details] [diff] [review] same as above but for ns tree sr=blake

Checked my part in for the installer, ns tree, mozilla tree, trunck and branch.

On attachment 48813 [details] [diff] [review] we are using a registry key value named "Mozilla QuickLaunch" but on attachment 48684 [details] [diff] [review] and attachment 48685 [details] [diff] [review] we are using a key value named "Mozilla". Is this correct ?

My last comment was not completely right. Attachment 48685 [details] [diff] uses a keyValue named "Netscape" . So we have 3 files, and each file uses a differen key. I understand that ns tree and mozilla tree shpould use different names. But I dont understand why we have 3 names for the same key.

r=ssu for syd's two patches: id=49173 and id=49174

r=ssu for Dp's patch too: id=49175

I just AIM'd with blake who asked if I could make the call to the http happen later after XPCOM was known to be inited. While looking at that (which was looking to be non-trivial), syd's and dp's patches showed up. +// The key that is used to write the quick launch appname in the windows registry +#define NS_QUICKLAUNCH_RUN_KEY "Mozilla Quick Launch" Is it possible to make this definition get setup by something in the makefiles so that it can be different between mozilla and commercial?

Just an aditional comment. If I understand the code correctly, then as a side effect of attachment 49173 [details] [diff] [review], attachment 49174 [details] [diff] [review] and attachment 49175 [details] [diff] [review], it is not possible to have both Mozilla and Netscape browsers installed on the same machine and using both the quick launch feature. Because they both use the same registry key value and so they overwrite each other. Also if we enable the quick launch feature in one browser, the "Quick Launch" preference option in the other one will be misleading.

Life's not perfect, we have a 0.9.4 to get out, NS won't be releasing for some time, so let's get this in so at least both, independent of each other, actually work when we wake up tomorrow. This turbo stuff has been down too long.

Ewww, hard tabs: + if ( result == ERROR_SUCCESS ) { + // XXX To make absolutely sure, we should check the value to see if this is + // XXX us or one of our predecessors - bascially distinguish betn mozilla and + // XXX mozilla based browsers + SetIsServerMode( PR_TRUE ); + } Fix these, and sr=brendan@mozilla.org. /be

a=asa for checkin to 0.9.4 branch

On the trunk (win2k cvs 2001091303) -turbo makes mozilla unusable now. It will only load 1 navigator window, any additional windows that are loaded exist in memory but not visually. They show under Tasks as emtpy lines.

sorry, spoke too soon, seeing it happen without quick launch too now

When syd checked in my patch last night, he enabled code that should have been #if 0 - This will cause xpcom to get double initialized, putting the app in a unstable state. This will happen only in the case where: - turbo mode was enabled - user quits turbo-moded mozilla by right clicking on the lizard icon in the systray - restarts mozilla The patch "Reenabling turbo on launch" fixes it.

Comment on attachment 49243 [details] [diff] [review] Reenabling turbo on launch I think having another method on nsINativeAppSupport which is called *after* xpcom is initialized is a good thing. The name setupServerMode(), since it's only done for one feature on one platform could be a bit more generic. Maybe what's currently called start() could be renamed initialize() and setupServerMode() becomes start()? Also, since what's happening in setupServerMode() is now happening after xpcom is initialized, can we go back to getting the app name in some non-static way? If the problem pointed out by Daniel is true, we should avoid that.

For why I think we need more generic methods on nsINativeAppSupport which are called at startup, see attachment=49227 from bug 98566.

We thought users wont want fast start for every mozilla and mozilla based browser. Hence the static name instead of different name. Whichever browser last enabled fast start should win. Since we are targetting this for 0.9.4, I would rather minimize the changes and make them post the branch. Marking TFV 0.9.4 to get in on the radar.

if we don't get this sorted out for the mozilla release we will want to get it sorted out on the branch afterwords...

Dp's latest patch (id=49243) does also fix a startup crash (in debug builds only) as well. There is also a comperable crash on exit which has been filed as bug 99553. I've noticed that the Mozilla Quick Launch variable in the Run key sometimes gets automatically (and mysteriouly) removed at the first startup immediately after the install is completed. I have not been able to reproduce it consistently. It might not have anything to do with this patch, but thought I just mention it. I'm still investigating it.

I know this is not the proper place to post such comments, but personally I'm very dissapointed in you people. Running apps from Registry has been always viewed as the "sneaky" way of auto loading apps, so that the common user can't remove the app easily. I think this is completly opposite the spirit of mozilla, and I'm very dissapointed that this bug didn't get more debate. :(

I think the patch (attachment 48813 [details] [diff] [review]) has not checked in properly into the 0.9.4 branch. The branch lost some of the code which is not part of this patch but part of bug 92447. Particularly, I am talking about xpfe/bootstrap/nsAppRunner.cpp The following changes are not yet landed on the branch but it seems that it's already applied to the branch as part of the fix 48813. This breaks the ReadConfig/AutoConfig functionality on the branch. Index: mozilla/xpfe/bootstrap/nsAppRunner.cpp =================================================================== RCS file: /cvsroot/mozilla/xpfe/bootstrap/nsAppRunner.cpp,v retrieving revision 1.305 diff -u -2 -r1.305 nsAppRunner.cpp --- nsAppRunner.cpp 2001/08/29 14:02:05 1.305 +++ nsAppRunner.cpp 2001/08/31 20:25:36 @@ -937,6 +937,4 @@ rv = prefs->ResetPrefs(); if (NS_FAILED(rv)) return rv; - rv = prefs->ReadConfigFile(); - if (NS_FAILED(rv)) return rv; rv = prefs->ReadUserPrefs(nsnull); if (NS_FAILED(rv)) return rv; @@ -944,7 +942,4 @@ else { - rv = prefs->ReadConfigFile(); - if (NS_FAILED(rv)) return rv; - nsCOMPtr<nsIAppShellService> appShellService(do_GetService(kAppShellServiceCID, &rv)); if (NS_FAILED(rv)) return rv;

Mitesh, I remember a conflict merging that diff onto the branch. I couldn't wrap my tired brain around what was going on and apparently erred in resolving the conflict the way I did. Can you work out how that code should now look on the branch? My recollection is that the 3 patches I had to work with (one from bug 88844, one from another bug, and the actual cvs diffs) had me sufficiently confused that I wasn't sure how to proceed. If need be, we should open a separate bug (if there isn't one already). Conrad Carlen might be able to help out; I think he likely understands the changes that were made here.

Actually, the fix from bug 92447 is also going to be checked in to the branch and it should be fine after that. In that case, we probably don't need to do a separate check in but need to modify the fix from bug 92447.

BTW, my patch on 9/14 16:13 is for the branch.

That latest patch looks good, dp. r=law I've applied it, built, and tested on my branch build and it fixes the problem.

Blake, can we reassign this bug to dp so he can track landing this latest patch? Or, dp, do you want to take it?

I will take this bug. Blake protest if you like to do it differently.

0.9.4 is out the door.

Comment on attachment 49396 [details] [diff] [review] [dp]Redoing relaunch enabling of turbomode and fixing XPCOM double initialization Bill reviewed this.

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

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

changing QA contact-

instead of re-using b, please use isServerMode and shouldShowUI. what ever nanoseconds you hope to gain by re-using the bool you loose in readability. fix that and sr=sspitzer

Comment on attachment 49396 [details] [diff] [review] [dp]Redoing relaunch enabling of turbomode and fixing XPCOM double initialization sr=alecf

PDT+. Check it in today.

Fix checked into trunk and branch.

verified on trunk and branch - builds 2001092505