156698 - [META] Support for Windows XP SP1 Integration

Status: RESOLVED FIXED

(Core Graveyard :: Tracking, defect, P1)

Description

[Jaime Rodriguez, Jr.]

This is a meta tracking bug designed to track work for Windows XP Integration
targeted for the next release. Someone will nominate (or file) a bug. The ADT
will evaluate that bug, determine if it is needed for the next release, and will
add it as a dependency on this meta bug (if needed). Please don't
add bugs directly to this bug without going through the ADT first.

Comment 1

[Olav Vitters]

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

Comment 2

[Jaime Rodriguez, Jr.]

This bug is blocked by http://bugscape.mcom.com/show_bug.cgi?id=17582.

Comment 3

[Bill Law]

Attachment: Patch, v1.0

This adds support for a -defaultBrowser command line switch that will make us
the default browser.

The code jumps through some hoops to avoid the crash we see at shutdown time
(when releasing JS components).  It adds a new .xul file for a "dummy window"
that simply closes itself when it opens.  The command line handler returns the
chrome URL for that window.

Turbo mode is kind of handled.	The code turns off turbo mode (but not the
preference).  That way, we don't keep running in turbo mode after setting the
default browser.  Unfortunately, the flip side is that we end up exiting turbo
mode if it was running previously.

Another problem still lurking is the issue of multiple profiles.  This patch
will end up showing the profile manager window if the user has multiple
profiles (and does not already have the browser/app running).  That is a direct
consequence of the way we're opening the dummy window.	We need to find a fix
for this problem, as well as the turbo-mode quirk described in the preceding
paragraph.

Comment 4

[Bill Law]

Comment on attachment 91005 [details] [diff] [review]
Patch, v1.0

Path is a work-in-progress...

Comment 5

[Bill Law]

Attachment: Patch, v2.0

New cmdlinehandler JS component to implement -defaultBrowser switch.

Comment 6

[Bill Law]

Attachment: Additional patch to xpfe/bootstrap code

This patch modifies xpfe/bootstrap to support -silent command line switch to
facilitate -defaultBrowser (and other similar handlers).

-silent does 3 things:

a. It causes nsNativeAppSupportWin.cpp to ignore the "quick launch"
pseudo-preference so that we do not go into turbo mode.  That's because we
don't want to start up turbo mode when performing the -defaultBrowser setting.

b. It causes code in nsNativeAppSupportWin.cpp (EnsureProfile) to suppress the
display of the profile manager when we have to open a window.  That way, we can
open the "dummy window" from nsDefaultBrowser.js without it triggering the
profile manager display.  That window will open using the "default profile" but
since the window never shows and we immediately exit when it closes, this is
fine.

c. It modifies nsAppRunner.cpp so that it looks for -silent also, and
suppresses display of the profile manager when it calls DoProfileStartup.

Comment 7

[Curt Patrick (gone)]

Attachment: Rough draft for the uninstaller (mozilla)

This mostly works.  The one thing I know I need to clean up is that currently
I'm only hiding and showing shortcuts in the HKLM.  I need to add some smarts
to know when to do it in HKCU instead.

The browser and mail need to use the following syntax when setting the registry
key that tell the system the command-lines for hiding and showing:

"<path to netscape>\uninstall\NSUninstall.exe" /ua "7.0 (en)" /ss <component>

Where /ss means show shortcuts.  Use /hs for hiding shortcuts.
Where <component> can be "browser", "mail", or "aim"
The "7.0 (en)" (i.e., User Agent) will have to change for each release.

Comment 8

[Curt Patrick (gone)]

Attachment: Rough draft for Netscape installer scripts (and changes to build)

nim.jst still needs to set the registry settings that add aim to the IM list
for XP.  Dave is working on that yet tonight, I believe.

Otherwise, I think this is a pretty good start.

Comment 9

[Curt Patrick (gone)]

Attachment: Real rough draft for the uninstaller (mozilla)

Whoa daddy-o, what was that.

So the comments for attachment 91072 [details] [diff] [review] apply to this patch.

One other (big) thing the uninstaller does't yet do is launch the browser for
setting up mail and browser defaults.  Need to sync up with Bill and Sean for
that.

Comment 10

[Sean Su]

Comment on attachment 91039 [details] [diff] [review]
Patch, v2.0

don't we also want to set isHandlingCHROME and isHandlingXUL?

r=ssu regardless of change

Comment 11

[Sean Su]

Comment on attachment 91040 [details] [diff] [review]
Additional patch to xpfe/bootstrap code

r=ssu

Comment 12

[Jaime Rodriguez, Jr.]

The reviews, like the patch should go into the bug 156701. This is just a
tracking bug. thanks!

Comment 13

[Bill Law]

Attachment: Patch v3.0 (changed command line switch to -setDefaultBrowser)

Sorry, Jaime.  My patch isn't for installer or mail, so this bug is the only
place to attach it!

Comment 14

[Bill Law]

Comment on attachment 91147 [details] [diff] [review]
Patch v3.0 (changed command line switch to -setDefaultBrowser)

Propogating r=ssu from previous patch.

re: isHandlingCHROME/isHandlingXUL

I made this set the same things that the new "Set Default Browser" button on
the Navigator pref pane sets.  Most users don't care about these types and
anybody that does qualifies as "Advanced" so they can go to the Advanced/System
pref panel to tweak them.

Comment 15

[Bill Law]

Any comments on the choice of "-silent" for the command-line switch tht 
suppresses QuickLaunch and profile manager behaviors?  Does it matter?

Comment 16

[jag (Peter Annema)]

Comment on attachment 91147 [details] [diff] [review]
Patch v3.0 (changed command line switch to -setDefaultBrowser)

sr=jag

Change that license to mpl/gpl/lgpl.

Comment 17

[Bill Law]

Attachment: patch, v4.0 (updated license block in new .xul file)

Only difference is change to the license text in the new .xul file.

Comment 18

[Bill Law]

Attachment: Additional patch for packages-win

jag also pointed out that we need to package this new component; this
attachment shows the change to packages-win to accomplish that.

Comment 19

[Sean Su]

Attachment: The Whole Enchilada v1.0

This patch contains all of the patches from this bug _and_ the following bugs
(that are relative to the mozilla tree):
  http://bugzilla.mozilla.org/show_bug.cgi?id=156695
  http://bugzilla.mozilla.org/show_bug.cgi?id=156701

A similar patch for the ns tree has been attached to the ns equivalent bug:
  http://bugscape.mcom.com/show_bug.cgi?id=17893

Comment 20

[scottputterman]

adding adt1.0.1+.  Please get drivers approval before checking into the branch.

Comment 21

[chris hofmann]

Attachment: test plan

test plan for checking out the whole enchilada.  grace has put the patch though
this wringer..

Comment 22

[chris hofmann]

Comment on attachment 91701 [details] [diff] [review]
The Whole Enchilada v1.0

assuming all the r's and sr's get moved over to this complete patch, a=chofmann
for the 1.0.1 branch.

we should also open up a bug to get this on the trunk/1.1b (if there is time)
or final

Comment 23

[lchiang]

We should also make sure that these changes don't adversely affect Win XP (w/out
the SP1).  

Note that we are still testing with a pre-release version of SP1.

Comment 24

[Daniel Veditz [:dveditz]]

Comment on attachment 91701 [details] [diff] [review]
The Whole Enchilada v1.0

So no Mozilla config/install.js changes? Doing that would be a cheap way
to get a bunch of extra testing.

>+JS_UNSET_MAIL_COMPONENT = nsUnSetDefaultMail.js

I don't see nsUnsetDefaultMail.js or the -unsetDefaultMail command
line option used anywhere . Why do we need this?

>+      // Return URL for dummy window that will auto-close.  We do this rather
>+      // than throw NS_ERROR_NOT_AVAILABLE, which *should* work, because it
>+      // seems that if we don't open a window, we get a crash when trying to
>+      // release this (or some other) JS component during XPCOM shutdown.
>+      return "chrome://global/content/dummyWindow.xul";

If this is true what about nsKillAll.js and nsResetPref.js? The latter
is definitely used.

>+void SetDefault()
>+{
>+  char szBuf[MAX_BUF];
>+  char szRegKey[MAX_BUF];
>+
>+  GetPrivateProfileString(ugUninstall.szDefaultComponent, "ClientTypeName", "", szBuf, MAX_BUF, szFileIniDefaultsInfo);
>+  wsprintf(szRegKey, "SOFTWARE\\Clients\\%s", szBuf);

It would have been nice if the command-line arguments were the
terms used in the registry, and we then used that as the section
name. Could have avoided this first lookup, and the danger that
localizers might translate the ClientTypeName.

> void SetUninstallRunMode(LPSTR szMode)
> {
>   if(lstrcmpi(szMode, "NORMAL") == 0)
>     ugUninstall.dwMode = NORMAL;
>   if(lstrcmpi(szMode, "AUTO") == 0)
>     ugUninstall.dwMode = AUTO;
>   if(lstrcmpi(szMode, "SILENT") == 0)
>     ugUninstall.dwMode = SILENT;
>+  if(lstrcmpi(szMode, "SHOWICONS") == 0)
>+    ugUninstall.dwMode = SHOWICONS;
>+  if(lstrcmpi(szMode, "HIDEICONS") == 0)
>+    ugUninstall.dwMode = HIDEICONS;
>+  if(lstrcmpi(szMode, "SETDEFAULT") == 0)
>+    ugUninstall.dwMode = SETDEFAULT;
> }

Seems silly to parse these as strings instead of passing in the
values, not that efficiency matters all that much in this case.

Should these really be mutually-exclusive modes?

>Index: xpinstall/wizard/windows/uninstall/uninstall.h
>===================================================================
> #define NORMAL                          0
> #define SILENT                          1
> #define AUTO                            2
>+#define SHOWICONS                       3
>+#define HIDEICONS                       4
>+#define SETDEFAULT                      5

You may want to make these binary powers so you can combine
these options in the future... right now they are all mutually
exclusive.

Well, I guess that all adds up to nothing but grumping, nothing
to change for now but plenty that could be cleaned up later.

sr=dveditz

Comment 25

[Sean Su]

patches checked in to the BRANCH only.  I'm closing this bug because it was only
referring to the branch.  I'll open a new bug to have these patches checked into
the TRUNK.

Comment 26

[Michael Hendy (Hendikins)]

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

Comment 27

[Alex Bishop]

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

Comment 28

[R Malone]

#25 says:

"patches checked in to the BRANCH only.  I'm closing this bug because it was only
referring to the branch.  I'll open a new bug to have these patches checked into
the TRUNK."

But there is no reference to the new bug. 
No Link. Where is the bug for the TRUNK??
No success querying bugzilla either - only duplicates that have already been closed.

Comment 29

[Dawn Endico]

the bug for landing on the trunk is bug 158187