[META] Support for Windows XP SP1 Integration

RESOLVED FIXED in mozilla1.0.1

Status

P1
major
RESOLVED FIXED
17 years ago
2 years ago

People

(Reporter: jaimejr, Assigned: law)

Tracking

Trunk
mozilla1.0.1
x86
Windows XP
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [ETA 07/16])

Attachments

(7 attachments, 4 obsolete attachments)

(Reporter)

Description

17 years ago
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.
(Reporter)

Updated

17 years ago
Depends on: 156695
(Reporter)

Updated

17 years ago
Depends on: 156701
(Reporter)

Updated

17 years ago
Blocks: 143047
No longer depends on: 156695, 156701

Comment 1

17 years ago
*** Bug 156694 has been marked as a duplicate of this bug. ***
(Reporter)

Comment 2

17 years ago
This bug is blocked by http://bugscape.mcom.com/show_bug.cgi?id=17582.
(Reporter)

Updated

17 years ago
Depends on: 156695, 156701
(Reporter)

Updated

17 years ago
Severity: normal → major
Priority: -- → P1
QA Contact: chofmann → paw
Whiteboard: [ETA 07/16]
Target Milestone: --- → mozilla1.0.1

Updated

17 years ago
Summary: [META] Support for Windows XP SP1 Inetegration → [META] Support for Windows XP SP1 Integration
(Assignee)

Comment 3

17 years ago
Created attachment 91005 [details] [diff] [review]
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.
(Assignee)

Comment 4

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

Path is a work-in-progress...
Attachment #91005 - Flags: needs-work+
(Assignee)

Comment 5

17 years ago
Created attachment 91039 [details] [diff] [review]
Patch, v2.0

New cmdlinehandler JS component to implement -defaultBrowser switch.
Attachment #91005 - Attachment is obsolete: true
(Assignee)

Comment 6

17 years ago
Created attachment 91040 [details] [diff] [review]
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

17 years ago
Created attachment 91072 [details] [diff] [review]
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

17 years ago
Created attachment 91073 [details] [diff] [review]
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

17 years ago
Created attachment 91075 [details] [diff] [review]
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.
Attachment #91072 - Attachment is obsolete: true

Comment 10

17 years ago
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
Attachment #91039 - Flags: review+

Comment 11

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

r=ssu
Attachment #91040 - Flags: review+

Updated

17 years ago
Blocks: 156695
No longer depends on: 156695
(Reporter)

Comment 12

17 years ago
The reviews, like the patch should go into the bug 156701. This is just a
tracking bug. thanks!
No longer blocks: 156695
Depends on: 156695
(Assignee)

Comment 13

17 years ago
Created attachment 91147 [details] [diff] [review]
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!
Attachment #91039 - Attachment is obsolete: true
(Assignee)

Comment 14

17 years ago
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.
Attachment #91147 - Flags: review+
(Assignee)

Comment 15

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

Comment 16

17 years ago
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.
Attachment #91147 - Flags: superreview+
(Assignee)

Comment 17

17 years ago
Created attachment 91565 [details] [diff] [review]
patch, v4.0 (updated license block in new .xul file)

Only difference is change to the license text in the new .xul file.
Attachment #91147 - Attachment is obsolete: true
(Assignee)

Comment 18

17 years ago
Created attachment 91567 [details] [diff] [review]
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

17 years ago
Created attachment 91701 [details] [diff] [review]
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

17 years ago
adding adt1.0.1+.  Please get drivers approval before checking into the branch.
Keywords: adt1.0.1+, mozilla1.0.1

Comment 21

17 years ago
Created attachment 91721 [details]
test plan

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

Comment 22

17 years ago
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
Attachment #91701 - Flags: approval+

Updated

17 years ago
Keywords: mozilla1.0.1 → mozilla1.0.1+

Comment 23

17 years ago
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 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
Attachment #91701 - Flags: superreview+

Comment 25

17 years ago
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.
Status: NEW → RESOLVED
Last Resolved: 17 years ago
Keywords: fixed1.0.1
Resolution: --- → FIXED
*** Bug 162510 has been marked as a duplicate of this bug. ***

Comment 27

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

Comment 28

16 years ago
#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

16 years ago
the bug for landing on the trunk is bug 158187
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.