156701 - XPInstall support for Windows XP SP1

Status: VERIFIED FIXED

(SeaMonkey :: Installer, defect, P1)

Description

[Jaime Rodriguez, Jr.]

This is track the XPinstall work needed to support Windows XP SP1.

Pls supply the following:
1. An estimate of engineering and QA person days required to implement this
request in the Status Whiteboard
2. Minimum Design Specificaiton (as an attachement)
3. Test Case (as an attachement)

Comment 1

[Jaime Rodriguez, Jr.]

XPInstall team has provided a verbal estimate of 4 days to the ADT. QA is
awaiting minimum design specificaiton, before committing to an estimate.

Comment 2

[Curt Patrick (gone)]

Oops, I put the first drafts of the patches in the meta bug.  I'll put the final
drafts in this bug now that I've got it straight.

Comment 3

[Jaime Rodriguez, Jr.]

yeah, i noticed that, too. :-) 

Comment 4

[Curt Patrick (gone)]

Attachment: uninstaller Patch 1

Comment 5

[dprice (gone)]

Attachment: changes to /ns/xpinstall/packager/windows

This should probably be in a bug on the netscape side

Comment 6

[dprice (gone)]

Attachment: defaults_info.ini

defaults_info.ini file

Comment 7

[Curt Patrick (gone)]

Attachment: uninstaller Patch 2

With changes Sean suggested.

Comment 8

[Curt Patrick (gone)]

Comment on attachment 91205 [details] [diff] [review]
changes to /ns/xpinstall/packager/windows

I think that we need to wrap the path to the app in quotes. So, instead of

    data = fProgram + "uninstall\\NSUninst.exe /ua \"$UserAgent$\" /ss
browser";

I believe it should be
    data = "\"" + fProgram + "uninstall\\NSUninst.exe\" /ua \"$UserAgent$\" /ss
brower;

With that change it looks good.
r=curt

Comment 9

[Curt Patrick (gone)]

Darn.  While I was reviewing Dave's patch it came to me that the uninstaller
isn't setting the IconsVisible registry key.  Guess I've got some more work to do.

Comment 10

[Curt Patrick (gone)]

Attachment: defaults_info.ini take 2

Had to add a keyword to support setting the IconsVisible Key/Value in the
registry.

Comment 11

[Curt Patrick (gone)]

Attachment: uninstaller Patch 3

This is the good stuff.  Really.  Including:

- The warning doesn't get popped by uninstaller in the /hs or /ss mode when the
browser is running.  (Does still get popped if neither of these modes has been
selected.

- Using wsprintf instead of lstrcat and such.

- Sets the IconsVisible key/value in the windows registry.

(Grace, while working on getting the warning dialog to happen at the right
times it occurred to me that we need to make sure and do regression testing to
make sure we have't broken any of the original installer behavior.  I know that
I had some difficulty getting the warning to desist for the new options without
having it fail for the old options, too.  I'm just concerned that there might
be some other dependency like that that I haven't noticed.)

Comment 12

[Sean Su]

Comment on attachment 91205 [details] [diff] [review]
changes to /ns/xpinstall/packager/windows

sorry, mail.jst won't work.  you can't use:
  ...\Clients\Mail\$MainExeFile$\...

mail is different from the browser.  You need to instead use:
  ...\Clients\Mail\$ProductName$\...

$ProductName$ is what it sets under Mail.

The IM path is okay because we've never set it before.

Please don't forget to also apply the patch to both mozilla and ns trees when
you do.

Comment 13

[Curt Patrick (gone)]

Attachment: uninstaller Patch 4

Cleanups which Sean identified.

Comment 14

[Sean Su]

Comment on attachment 91393 [details] [diff] [review]
uninstaller Patch 4

there are redundancies in the code, but given the time constraint, this works
for now.

I'm glad you commented it.  Hopefully it will be redone.

r=ssu

Comment 15

[dprice (gone)]

Attachment: patch 2 - /ns/xpinstall/packager/windows changes

Comment 16

[Sean Su]

Comment on attachment 91411 [details] [diff] [review]
patch 2 - /ns/xpinstall/packager/windows changes

my bad.  it shouldn't be $ProductName$.  I should be $ProductNameNoVersion$.

$ProductName$ tacks on a version string at the end of the product name which we
can't use.

r=ssu once that's changed.

Comment 17

[Curt Patrick (gone)]

Attachment: uninstaller Patch 4

Cannot assume the name of the key (not to mention that I had hardcoded it,
which was a bad thing) where IconsVisible lives.  The browser and mail actually
use a slightly different name.	Made this configurable.

Comment 18

[Curt Patrick (gone)]

Attachment: defaults_info.ini take 3

To support the configurablility in the last uninstaller patch (91454) above

Comment 19

[dprice (gone)]

Attachment: xpinstall/packager/windows diffs

Comment 20

[Curt Patrick (gone)]

Attachment: uninstaller Patch 5

Added function SetDefaults() to support the following command-line:

   NSUninstall.exe /ms /ua "7.0 (en)" /sd aim

where /sd stands for "set default".  It turns out all the information we need
to generalize the command-line (instead of something specific to Aim like
setDefaultAim) already exists in defaults_info.ini so we got that for free.

Comment 21

[Sean Su]

Comment on attachment 91606 [details] [diff] [review]
uninstaller Patch 5

r=ssu 

great job curt!
Now we just need to update the .jst patches (from dprice) to take advantage of
this.

Comment 22

[dprice (gone)]

Attachment: xpinstall/packager/windows diffs - today's changes

Comment 23

[Sean Su]

Comment on attachment 91621 [details] [diff] [review]
xpinstall/packager/windows diffs - today's changes

for:
  data = "\"" + fProgram + "uninstall\\NSUninst.exe\" /ms /sd IM";

don't we need to also pass:
  /ua \"$UserAgent$\"

Actually, instead of updating config.it to register the default aim value,
update nim.jst instead.  This is so that it'll get registered to get
uninstalled.

You also do not need all those logComment() calls.  The registry functions will
appropriatelly log what it does.

Come talk with me about this.  I can show you what I mean.

Comment 24

[Curt Patrick (gone)]

Sean is right about the   /ua \"$UserAgent$\"  with /sd.  Uninstaller cannot
find the defaults_info.ini with that parameter.

Comment 25

[dprice (gone)]

Attachment: ns/xpinstall/packacger/windows/ changes

Comment 26

[Sean Su]

Comment on attachment 91669 [details] [diff] [review]
ns/xpinstall/packacger/windows/ changes

r=ssu

Comment 27

[Sean Su]

Comment on attachment 91606 [details] [diff] [review]
uninstaller Patch 5

sorry Curt.  I found one problem.  In SetDefaut(), you need to pass the length
of szBuf, not it's size, like so:
  SetWinReg(HKEY_LOCAL_MACHINE, szRegKey, "", REG_SZ, szBuf, lstrlen(szBuf));

I've already updated this change in my local build.  Just need a new patch from
you to attach to this bug.

Comment 28

[Curt Patrick (gone)]

Attachment: uninstaller Patch 5

Comment 29

[Sean Su]

Comment on attachment 91686 [details] [diff] [review]
uninstaller Patch 5

r=ssu

Comment 30

[Grace Bush]

testing on test build passes my test cases.  Will attach test matrix

Comment 31

[Grace Bush]

Attachment: test matrix

Comment 32

[scottputterman]

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

Comment 33

[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 34

[Grace Bush]

branch build 20020071808
(install/uninstall/registry testing)

Comment 35

[Matthias Versen [:Matti]]

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