XPInstall support for Windows XP SP1

VERIFIED FIXED in mozilla1.0.1

Status

SeaMonkey
Installer
P1
major
VERIFIED FIXED
16 years ago
14 years ago

People

(Reporter: Jaime Rodriguez, Jr., Assigned: Curt Patrick (gone))

Tracking

Trunk
mozilla1.0.1
x86
Windows XP

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [adt1 RTM] [ETA 07/18] [needs sr=])

Attachments

(4 attachments, 12 obsolete attachments)

446 bytes, text/plain
Details
21.87 KB, patch
Sean Su
: review+
Details | Diff | Splinter Review
26.50 KB, patch
Sean Su
: review+
Details | Diff | Splinter Review
5.27 KB, text/plain
Details
(Reporter)

Description

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

Comment 1

16 years ago
XPInstall team has provided a verbal estimate of 4 days to the ADT. QA is
awaiting minimum design specificaiton, before committing to an estimate.
Blocks: 156698
Severity: normal → major
Keywords: mozilla1.0.1, nsbeta1+
Priority: -- → P1
Whiteboard: [adt1 RTM] [ETA 07/15]
Target Milestone: --- → mozilla1.0.1
(Reporter)

Updated

16 years ago
No longer blocks: 156698
(Reporter)

Updated

16 years ago
Blocks: 156698
(Assignee)

Comment 2

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

Comment 3

16 years ago
yeah, i noticed that, too. :-) 

Updated

16 years ago
QA Contact: bugzilla → gbush
(Assignee)

Comment 4

16 years ago
Created attachment 91190 [details] [diff] [review]
uninstaller Patch 1

Comment 5

16 years ago
Created attachment 91205 [details] [diff] [review]
changes to /ns/xpinstall/packager/windows

This should probably be in a bug on the netscape side

Comment 6

16 years ago
Created attachment 91207 [details]
defaults_info.ini

defaults_info.ini file
(Assignee)

Comment 7

16 years ago
Created attachment 91208 [details] [diff] [review]
uninstaller Patch 2

With changes Sean suggested.
Attachment #91190 - Attachment is obsolete: true
(Assignee)

Updated

16 years ago
Attachment #91205 - Flags: review+
(Assignee)

Comment 8

16 years ago
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
(Assignee)

Comment 9

16 years ago
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.
(Assignee)

Comment 10

16 years ago
Created attachment 91305 [details]
defaults_info.ini take 2

Had to add a keyword to support setting the IconsVisible Key/Value in the
registry.
Attachment #91207 - Attachment is obsolete: true
(Assignee)

Comment 11

16 years ago
Created attachment 91307 [details] [diff] [review]
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.)
Attachment #91208 - Attachment is obsolete: true

Comment 12

16 years ago
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.
Attachment #91205 - Flags: review+ → needs-work+
(Assignee)

Comment 13

16 years ago
Created attachment 91393 [details] [diff] [review]
uninstaller Patch 4

Cleanups which Sean identified.
Attachment #91307 - Attachment is obsolete: true

Comment 14

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

Comment 15

16 years ago
Created attachment 91411 [details] [diff] [review]
patch 2 - /ns/xpinstall/packager/windows changes
Attachment #91205 - Attachment is obsolete: true

Comment 16

16 years ago
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.
Attachment #91411 - Flags: needs-work+
(Assignee)

Comment 17

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

Comment 18

16 years ago
Created attachment 91455 [details]
defaults_info.ini take 3

To support the configurablility in the last uninstaller patch (91454) above
Attachment #91305 - Attachment is obsolete: true
Attachment #91393 - Attachment is obsolete: true

Comment 19

16 years ago
Created attachment 91457 [details] [diff] [review]
xpinstall/packager/windows diffs
Attachment #91411 - Attachment is obsolete: true
(Assignee)

Comment 20

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

Comment 21

16 years ago
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.
Attachment #91606 - Flags: review+

Comment 22

16 years ago
Created attachment 91621 [details] [diff] [review]
xpinstall/packager/windows diffs - today's changes
Attachment #91457 - Attachment is obsolete: true
(Reporter)

Updated

16 years ago
Whiteboard: [adt1 RTM] [ETA 07/15] → [adt1 RTM] [ETA 07/18] [needs sr=]

Comment 23

16 years ago
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.
Attachment #91621 - Flags: needs-work+
(Assignee)

Comment 24

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

Comment 25

16 years ago
Created attachment 91669 [details] [diff] [review]
ns/xpinstall/packacger/windows/ changes
Attachment #91621 - Attachment is obsolete: true

Comment 26

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

r=ssu
Attachment #91669 - Flags: review+

Comment 27

16 years ago
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.
Attachment #91606 - Flags: review+ → needs-work+
(Assignee)

Comment 28

16 years ago
Created attachment 91686 [details] [diff] [review]
uninstaller Patch 5
Attachment #91606 - Attachment is obsolete: true

Comment 29

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

r=ssu
Attachment #91686 - Flags: review+

Comment 30

16 years ago
testing on test build passes my test cases.  Will attach test matrix

Comment 31

16 years ago
Created attachment 91715 [details]
test matrix

Comment 32

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

Comment 33

16 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: 16 years ago
Keywords: fixed1.0.1
Resolution: --- → FIXED

Comment 34

16 years ago
branch build 20020071808
(install/uninstall/registry testing)
Status: RESOLVED → VERIFIED
Keywords: fixed1.0.1 → verified1.0.1
*** Bug 168569 has been marked as a duplicate of this bug. ***
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.