103232 - Simple MAPI: Uninstaller needs to restore previous mapi32.dll if replaced

Status: VERIFIED FIXED

(SeaMonkey :: Installer, defect)

Description

[Sean Su]

Given the fix to bug 102308, the uninstaller needs to be able to detect if 
mapi32.dll was replaced by us, and if so, restore the backed up one.

Here are the comments from bug 102308 related to uninstall.

 ------- Additional Comments From Rajiv Dayal 2001-10-02 15:11 -------

 In case when we replace the existing mapi32.dll we need to take care of it
 during the uninstall. A user may select Mozilla as the default mail client and
 then without unsetting it from the Prefs, may uninstall Mozilla. In this case
 uninstall should replace the mapi32.dll with the backed up mapi32.dll
 (mapi32_moz_bak.dll) and delete the mapi32_moz_bak.dll.

 Also in case of a similiar scenario as below : 
 1) N4.7 set as default mail client, mapi32.dll is from N4.7
 2) Mozilla made the default mail client, mapi32.dll is from Mozilla,
 mapi32_moz_bak.dll is from N4.7
 3) Outlook made the default client, mapi32.dll is from Outlook

 Now during uninstall we need to take care we donot replace this mapi32.dll from
 Outlook with the backed up dll which is from N4.7. To take care of this,
 uninstall will check if the current dll is from Mozilla and replace it with the
 backed up one only if it is.

 The following are the changes for this :
 1) Modify uninstall to do the above.
 2) New method "GetMozMapiDllVersion" exported by mapi32.dll
 3) Store the backed up dll path in registry key




 ------- Additional Comments From Srilatha Moturi 2001-10-02 15:54 -------

 I checked in the code for setting the registry key when we switch the dll.
 This is the key that we set.
 [HKEY_LOCAL_MACHINE\SOFTWARE\Mozilla\Desktop]
 "Mapi_backup_dll"="D:\\WINNT\\SYSTEM32\\Mapi32_moz_bak.dll"



 ------- Additional Comments From Rajiv Dayal 2001-10-02 17:18 -------

 I have checked in the changes for the exported DllVersion method into the
 MAPI_SUPP_BRANCH. The name of the exported function is :
 'MAPIGetNetscapeVersion'. A similiar function also exists for the mapi dll for
 N4.7. Sean, u will need to call this function and if it returns 94 (Mozilla   
094) it means it is from us.

Comment 1

[Sean Su]

marking PDT+ per PDT meeting.

Comment 2

[Sean Su]

I talked to Rajiv, and he agreed that the function name should not have 
'Netscape' as part of its name.  The new name will be:
  MAPIGetVersion()

Rajiv, do you know if the future versions of our mapi32.dll will return numbers 
> 94 when MAPIGetVersion() is called?  If so, would it be a good idea to have 
the uninstaller check for versions >= 94?

If not, this means that with each new mapi version we release, we'll need to 
manually update the uninstaller code to check for the appropriate version.

Perhaps we want to only restore the version we've installed given a particular 
build?

Comment 3

[Sean Su]

Attachment: patch #1

Comment 4

[Sean Su]

In the patch, I've defined the mapi version to check for in uninstall.ini so 
that it's at least easier to udpate than the uninstaller code itself.

If Rajiv indicates that we can perform a wider range of checks, I'll update the 
patch then.

Comment 5

[Curt Patrick (gone)]

r=curt

Comment 6

[Sean Su]

Comment on attachment 52196 [details] [diff] [review]
patch #1

putting r=curt here

Comment 7

[Tiantian Kong]

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

Comment 8

[Daniel Veditz [:dveditz]]

Comment on attachment 52196 [details] [diff] [review]
patch #1

sr=dveditz

Comment 9

[Sean Su]

Patch checked into 0.9.4 branch only.

Awaiting trunk to open for general checkins before patching trunk.

Comment 10

[Sean Su]

Attachment: patch to update uninstall.it (ns tree)

Comment 11

[Sean Su]

Attachment: patch to update uninstall.it (moz tree)

Comment 12

[Sean Su]

the above patches to the uninstall.it files have also been applied to the branch 
only.

Comment 13

[Sean Su]

I was talking to Rajiv and had discovered a problem when undoing the mapi stuff 
in a certain situation:
 1) user installs 6.2.
 2) user sets 6.2 to be default mail handler.
 3) user then upgrades 6.2 (installs onto the same dir as previous version) to
    6.3.
 4) User then sets 6.3 as the default mail client.

The problem that just occurred is that when 6.2 was installed, we backed up the 
previous (say, outlook's mapi32.dll) maip32.dll into Mapi32_moz_bak.dll,  Then 
when 6.3 was installed and also set as the default mail client, the 6.2's 
mapi32.dll gets backed up as Mapi32_moz_bak.dll.  Since we only remember the 
previous mapi file, we just lost the outlook's mapi32.dll that was there before 
6.2 was installed.

So when we unsintall 6.3, 6.2 gets removed as well (because it was upgraded to 
6.3).  The uninstaller will restore the previous mapi32.dll saved, which turns 
out to be from 6.2.  Now we no longer have a Netscape 6.2 mail to go along with 
this mapi32.dll!

The solution is to have the installer perform mapi checking and if it turns out 
to be an "upgrade" (meaning user installs 6.3 ontop of 6.2), then have the 
installer automatically upgrade mapi32.dll to the new nscp's version *without* 
touching the backed up version.

This means that the installer also needs to know the version of 6.2's mapi32.dll 
so that it can perform an "intelligent" upgrade of this file.

Right now, the uninstaller has the mapi's version hard coded to be 94 so it can 
do the "intelligent" restore.  I rather not hard code this version in the 
installer as well because it can easily get stale (too many places to remember 
to update).

what I propose is for the mapi code to in addition of setting:

 [HKEY_LOCAL_MACHINE\SOFTWARE\Mozilla\Desktop]
 "Mapi_backup_dll"="D:\\WINNT\\SYSTEM32\\Mapi32_moz_bak.dll"

Also set it's current version:

 [HKEY_LOCAL_MACHINE\SOFTWARE\Mozilla\Desktop]
 "Mapi_version"="94"

This way, the installer and the uninstaller can use that value to make sure that 
the mapi32.dll file found belongs to us and is the correct version for that 
installed Netscape 6.

I would also recomment that if the user unsets our mail as the default client, 
that this new version key (Mapi_version) be removed as well (as it's currently 
does with Mapi_backup_dll).

Please let me know if we can do this.  It think this would save the installer a 
lot of headache.

Comment 14

[Daniel Veditz [:dveditz]]

How about much simpler: if mapi32_moz_bak.dll exists then don't overwrite it.

Comment 15

[Srilatha Moturi]

Attachment: Patch to set and unset Mapi_version key

Comment 16

[Srilatha Moturi]

Attachment: updated patch to set and unset mapi_version key.

Comment 17

[Sean Su]

We should try not to hard code the mapi version as "94" in the patch.  This 
will complicate things in the future when this version is updated.  This will 
introduce multiple places where the version will need to be changed.  Is there a 
way to get at the version that our mapi32.dll will set and just use that 
instead?

Comment 18

[Srilatha Moturi]

Attachment: patch for deleteregistrykey

Comment 19

[Srilatha Moturi]

Sean is looking into setting the Mapi_version key from the installer.
The new patch has DeleteRegistryKey method, and instead of setting 
Mapi_backup_dll to "", I am deleting it

Comment 20

[Sean Su]

Comment on attachment 52627 [details] [diff] [review]
patch for deleteregistrykey

r=ssu

Comment 21

[Rajiv Dayal]

Sean, as we discussed we should avoid setting the mapi version everytime mozilla
is made the default mail client since this requires loading the library
everytime. Instead we can do this in the install, in fact if there is a function
that can be called to get the current Mozilla version u can use (no need to even
load the mapi32 library) that and that can also be used in the GetMAPIDllVersion
function too so that when u check it in the uninstaller the comparision will
succeed. If u know of such a function that can be used to get the current
Mozilla version let me know.

Comment 22

[Sean Su]

Attachment: patch #1.1 (ns tree)

Comment 23

[Sean Su]

Attachment: patch #1.2 (moz tree)

Comment 24

[Sean Su]

The patches above (1.1 and 1.2) will:
 * fix mail.xpi to auto update Mapi32.dll if it was previously replaced by the 
mail client.
 * change the function name it used to call to figure out if the Mapi dll was 
ours and what version it was, from MAPIGetVersion() to GetMapiDllVersion().
 * added a few more logComment()s so that more registered Win Reg keys can be 
properly uninstalled.

Comment 25

[Daniel Veditz [:dveditz]]

Comment on attachment 52627 [details] [diff] [review]
patch for deleteregistrykey

A function called DeleteRegistryKey actually deletes registry values?
Am I confused or is it the code?

Comment 26

[Srilatha Moturi]

Attachment: changed the name to DeleteRegistryValue

Comment 27

[Srilatha Moturi]

Dan please review the attachment 52673 [details] [diff] [review].

Comment 28

[Daniel Veditz [:dveditz]]

Comment on attachment 52673 [details] [diff] [review]
changed the name to DeleteRegistryValue

Please use more contexts in your diffs next time

sr=dveditz for branch, carrying over r=ssu from previous version

Comment 29

[Daniel Veditz [:dveditz]]

Anyone going to review Sean's patches so I can sr= them?

Comment 30

[Curt Patrick (gone)]

Comment on attachment 52654 [details] [diff] [review]
patch #1.2 (moz tree)

r=curt for both attachments
(52653 & 52654)

Comment 31

[Jaime Rodriguez, Jr.]

pls check it into the branch - PDT+

Comment 32

[Sean Su]

Attachment: patch #2.0 (ns tree)

Comment 33

[Sean Su]

Attachment: patch #2.0 (moz tree)

Comment 34

[Sean Su]

Attachment: patch #3.0 (ns tree)

Comment 35

[Sean Su]

Attachment: patch #3.0 (moz tree)

Comment 36

[Sean Su]

Attachment: patch #3.1 (moz tree)

Comment 37

[Curt Patrick (gone)]

Comment on attachment 52824 [details] [diff] [review]
patch #3.1 (moz tree)

r=curt

Comment 38

[Curt Patrick (gone)]

Comment on attachment 52822 [details] [diff] [review]
patch #3.0 (ns tree)

r=curt

Comment 39

[Daniel Veditz [:dveditz]]

Comment on attachment 52822 [details] [diff] [review]
patch #3.0 (ns tree)

sr=dveditz

Why the short-form names?

Comment 40

[Daniel Veditz [:dveditz]]

Comment on attachment 52824 [details] [diff] [review]
patch #3.1 (moz tree)

sr=dveditz

Comment 41

[Sean Su]

because Srilatha's code sample to me showed she was setting short names.  I 
wanted to be consistent.

Comment 42

[Sean Su]

Srilatha, my patches have been checked into both trunk and branch.  I know that 
you can only check your patch into the branch.

Let me know when you have updated the branch with you patch.

Comment 43

[Srilatha Moturi]

patch 52673 checked into 0.9.4 branch

Comment 44

[Jaime Rodriguez, Jr.]

can we verify this fix on the 094 branch?

Comment 45

[lchiang]

trix - can you help to verify this?

Comment 46

[Trix Supremo]

FAIL: both 2001-10-15-05-0.9.4 & 2001-10-16-05-0.9.4 builds.

Uninstall removes the Mapi32_moz_bak.dll, but doesn't copy it over back to
Mapi32.dll.

EXAMPLE:
1. Set Comm 4.x to be default email client (MAPI32.DLL is approx ~68k)
2. Set Netscape 6.2 to be default email client (Mapi32.DLL is approx ~5k &
Mapi32_moz_bak.dll approx ~68k created.)
3. Uninstall Netscape 6.2.

ACTUAL RESULTS:
Mapi32.dll remains approx ~5k (which is the dll Netscape 6.2 copied over) while
the Mapi32_moz_bak.dll has been deleted.

Comment 47

[Jaime Rodriguez, Jr.]

trix - pls go back to the 10-10 build which is the day after this patch was
checked in and verify whether or not this patch had worked before.  Thanks, Lisa

Comment 48

[Sean Su]

Found the problem.  The problem was caused by a typo.  Patch coming up.

Comment 49

[Sean Su]

Attachment: patch to fix typo

Comment 50

[Srilatha Moturi]

Comment on attachment 53832 [details] [diff] [review]
patch to fix typo

r=srilatha

Comment 51

[Trix Supremo]

well just a little too late to make my comments, but yes the same problem
obviously occurred on 10-10 & 10-12 builds.

Comment 52

[Jaime Rodriguez, Jr.]

PDT+, pending positive review from Dan, and extra special, cross over review
from mscott.

Comment 53

[Daniel Veditz [:dveditz]]

Comment on attachment 53832 [details] [diff] [review]
patch to fix typo

sr=dveditz

Comment 54

[Scott MacGregor]

Comment on attachment 53832 [details] [diff] [review]
patch to fix typo

extra sr=mscott

Comment 55

[grega]

sean, are you checking this in tonight? Thanks!

Comment 56

[Sean Su]

patch checked in

Comment 57

[Sean Su]

marking this fixed (both trunk and branch).

Comment 58

[Tiantian Kong]

Trix:

Pls verify this bug on tomorrow's build ASAP. If you need some help, let me
know. I can ask Hong.

Comment 59

[Trix Supremo]

PASS: 2001-10-17-05-0.9.4 copies Mapi32_moz_bak.dll back to Mapi32.dll(if
exists) and removes the bak file during uninstall.

will leave to grace to mark verified since she is the contact.

Comment 60

[Jimmy Lee]

Pinch hitting for Grace while she's out.  Marking Verified!