99526 - Installer for simple MAPI

Status: VERIFIED FIXED

(MailNews Core :: Simple MAPI, defect, P1)

Description

[Tiantian Kong]

This is to track the development for the installer for simple MAPI.

Comment 1

[Sean Su]

over to syd.  cc:ing myself.

Comment 2

[Sheela Ravindran]

changing qa contact as per bug 95122 since this a related bug. 

Comment 3

[Rajiv Dayal]

We will be able to do the MS COM registration part from preferences when the 
user checks/uncheck the checkbox for making mozilla as the default mail client.

Comment 4

[selmer (gone)]

Rajiv, could you put more description in the bug.  We talked about several
issues at the meeting.

Adding dveditz to the cc.

Comment 5

[Rajiv Dayal]

The installer needs to install/un-install MAPI support dlls and register / 
un-register the MAPI support MS COM components and Mozilla executable as an MS 
COM component into the Windows registry. 

However the registration of MS COM components can be registered / un-registered 
when the user selects / un-select mozilla as default mail client. This will be 
done at two places .. when the user does this from the prefs dialog and when the 
user does so from the mail integration dialog. 

Thus Install will just copy the mozilla support dlls which will be part of the 
other Mozilla/mailnews module.

During registration the preferences code will call the 
DllRegisterServer/DllUnregister function exposed by the mapiProxy Dll to 
register the MAPI support MS COM components as well as the 
RegisterServer/UnregisterServer function (needs to be exposed) of the 
msgMapi.dll to register Mozilla as MS COM component.

Besides this the prefs will modify the MAPI registry key in Windows registry to 
update it with Mozilla's mapi32.dll. It will also back up the already defined 
key value (which would be pointing to the mapi32.dll of any other messaging 
application) into HKLM/Software/Mozilla/Desktop registry key, similiar to what 
is done during the the HTTP preference setting as mention by Sean Su in the 
meeting.

This will be used by Un-Install to set the key back to the backed up value (the 
value before mozilla was selected as default mail client). 

Ideally Uninstall should also un-register the MAPI support MS COM component and 
Mozilla executable as an MS COM component (or call the Prefs code to do so) 
besides removing the Mapi support Dlls.

Comment 6

[Sean Su]

Please also add/create any appropriate windows registry keys that provide 
information on whether the mapi32.dll was replaced or the new method of 
referencing the correct mapi32.dlll using windows registry keys has been used.

Even if we only end up supporting the new method, it would still be a good idea 
to set some type of flag in case we decide to support the older method that 
requires the replacement of the system mapi32.dll.

As for unregistering the MS COM components, the installer just need to know what 
the names of these files are.  The uninstaller already has the capability to 
handle this situation.

Comment 7

[Tiantian Kong]

nsbranch+

Comment 8

[Syd Logan]

is anyone working on this?

Comment 9

[Sean Su]

I've been in the loop of this bug.  There's really nothing to do until we hear 
from Srilatha on what the registry keys are for the uninstaller to use in order 
to restore the previous default mail handler.

Comment 10

[Curt Patrick (gone)]

I'll take this.  I'll check with Sean and Srilatha to see what can be done to
get this in for emojo.

Comment 11

[Srilatha Moturi]

The following two strings will be saved in the registry
HKEY_LOCAL_MACHINE\\Software\\Clients\\Mail
HKEY_LOCAL_MACHINE\\Software\\Clients\\Mail\\"DefaultMailClient"\\protocols\\mai
lto\\shell\\open\\command

In the above string "DefaultMailClient" is the value of the first string.

and they are stored in the key
HKEY_LOCAL_MACHINE\\Software\\Mozilla\\Desktop

Comment 12

[Tiantian Kong]

HI, Curt: Please could you let me know about the status of this bug before 10am
on 9/18. I need the status for PDT meeting at 12 noon. Many thanks.

Comment 13

[Sean Su]

Actually, I'll be working on this.  It most likely will not be in tomorrow's 
build because I just got all the necessary info today.  This fix/feature does 
not impact UI code.  Is it really urgent to have this in by tonight?  Please let 
me know.

Comment 14

[Tiantian Kong]

Added PDT status, per PDT meeting today. Once we got all r and sr, will change
it into PDT+. 

All r ans sr, please try your best to give comments within 24 hours. If no
reply, then our assumption will be within 24 hours. If you cannot do it within
24 hours, please attend the PDT meeting at 12 noon on 9/18 at the pit of bldg 21
to explain why. Many thanks.
=======================
Subject: Urgent: your review and super review comments.
Date: Mon, 17 Sep 2001 13:18:36 -0700
From: Tiantian Kong <[e-mail]>
Reply-To: [e-mail]
To: [e-mail], [e-mail],	Doug Turner <[e-mail]>, [e-mail],	[e-mail],
Rajiv Dayal <[e-mail]>
CC: Elaine King <[e-mail]>

Hi:

You got this mail because you are the r or sr of features in simple MAPI.

Pathes for these has been up on the bug. 

On behalf on PDT, I'm requesting that you provide immediate feeback. If you
cannot be back to us today, please reply by 
email, and cc Elaine King, as to when you'll be able to get back to us.

This is an urgent request, please drop everything else that you are doing. 

Thx. Elaine will be calling each of you as well. 


Rajiv, r, 		Pref, 95122					

Seth, sr, 		Pref, 95122

Dan Veditz,	r	log on, log off, 95117/95121	

Doug Turner, sr, 	log on, log off, 95117/95121	
				
Bill Law,	r	xpfe/bootstrap for log on, off	
				
Alec, sr, 		xpfe/bootstrap for log on, off 


Tiantian
=========================

Comment 15

[selmer (gone)]

Sean, we're trying to reach feature complete on this so we can begin testing and
shake everything out.  The urgency is not about the UI deadline.  The sooner
we're done coding this beast, the more likely we can get all the bugs fixed
before it's too late.

Comment 16

[Sean Su]

Attachment: patch #1 (ns tree)

Comment 17

[Sean Su]

Attachment: patch #1 (moz tree)

Comment 18

[Sean Su]

Attachment: Complete rdi.c file. Might be easier to read than the rdi.c part of the dif above.

Comment 19

[selmer (gone)]

Marking PDT+ for the branch, provisional on getting the revies completed.

Comment 20

[Daniel Veditz [:dveditz]]

Comment on attachment 49861 [details]
Complete rdi.c file.  Might be easier to read than the rdi.c part of the dif above.

>  ZeroMemory(szOutSubKeyPath, dwOutSubKeyPathSize);
>  /* search for the first '\' char */
>  while((*ptr != 0)    &&
>        (*ptr != '\0') &&
>        (*ptr != '\\'))
>    ++ptr;

0 and '\0' are the same thing so that's a redundant check. Unless that first
test was supposed to be for 'ptr' rather than '*ptr', but if you're worried
about the input arg being null you should test that once, not in the loop.

More idiomatic would be 
    while ( *ptr && *ptr != '\\' )
rather than comparing against null directly.

But why not just use strchr() for this rather than roll your own?


>  if((*ptr == '\0') ||
>     (*ptr == '\\'))

How could it be anything else after the while-loop above? Maybe you just
want to assert the above and print "Hit by Gamma rays!" if it fails.

The code will work fine, so r=dveditz whether you fix the above or not.
I'm sure you've tested the above so safer/faster to just use that for
the branch, and consider the comments on the trunk in the future.

Comment 21

[Daniel Veditz [:dveditz]]

Comment on attachment 49860 [details] [diff] [review]
patch #1 (moz tree)

looks good, r=dveditz

Comment 22

[Daniel Veditz [:dveditz]]

Comment on attachment 49859 [details] [diff] [review]
patch #1 (ns tree)

r=dveditz

Comment 23

[Scott MacGregor]

Comment on attachment 49861 [details]
Complete rdi.c file.  Might be easier to read than the rdi.c part of the dif above.

sr=mscott

Comment 24

[Scott MacGregor]

Comment on attachment 49860 [details] [diff] [review]
patch #1 (moz tree)

sr=mscott

Comment 25

[Scott MacGregor]

Comment on attachment 49859 [details] [diff] [review]
patch #1 (ns tree)

sr=mscott

Comment 26

[Sean Su]

Due to a bug I found in the way the browser code restores the previous saved 
setting, a new moz tree patch is coming up.

The new patch tries to detect if garbage chars have been saved in any of the 
HKLM\Software\Mozilla\Desktop vars.  If so, it will not restore it.  If no 
garbage chars have been found, the it performs its normal purpose.

Comment 27

[Sean Su]

Attachment: patch #2 (moz tree) deals with garbage chars.

Comment 28

[Sean Su]

Attachment: New complete rdi.c file.

Comment 29

[Sean Su]

Attachment: Diff of rdi.c between patch #1 and patch #2

Comment 30

[Daniel Veditz [:dveditz]]

Could we try to fix the browser bug as well?

What "good" data is in these keys -- aren't they potentially path or file names?
If so I have concerns about your algorithm which might reject valid multibyte
pathnames. Does isprint() evenhandle single-byte chars > x7F ok?

Comment 31

[Sean Su]

Browser bug is being addressed by Bill Law.  I talked to Dan off line already.  
We're going to do deal with the garbage chars only if the key path contains 
ddeexec (since that's where the problem only happens with), in order to minimize 
any collateral damage that may arise.

New patch coming up.

Comment 32

[Sean Su]

Attachment: patch #3 (moz tree) deals with garbage chars.

Comment 33

[Sean Su]

Attachment: New complete rdi.c from patch #3

Comment 34

[Sean Su]

Attachment: Diff of rdi.c between patch #1 and patch #3

Comment 35

[Daniel Veditz [:dveditz]]

Comment on attachment 50142 [details] [diff] [review]
patch #3 (moz tree) deals with garbage chars.

>+            /* Due to a bug in the browser code that saves the previous HKEY
>+             * value it's trying to replace as garbage chars, we need to try
>+             * to detect it.  If found, do not restore it. This bug only
>+             * happens for the saved ddeexec keys. */
>+            if(DdeexecCheck(szSubKey, szValue))
>+            {
>+              /* Restore the previous saved setting here */

The name of this function and the way this is written makes it sound
like you're only restoring previous DDEExec key settings.

Not worth changing if there are no other problems, but if you're ever
in this code again reversing the sense of the function would make the
code more maintainable

        if (!CorruptDdeexecKey())
            // Restore setting

or something like that.

r=dveditz

Comment 36

[Scott MacGregor]

rs=mscott after Dan's good review.

Comment 37

[Sean Su]

patches checked in to both branch and trunk.

Since this code affects the uninstallation/restoration of the settings for 
mozilla/netscape 6 being the default browser, we need to still test to make sure 
we didn't break restoring the settings to the previous default browser.

This new code should now also deal with restoring the previous default mail app 
at uninstall time.

Comment 38

[Trix Supremo]

verified on 2001-10-12-05-0.9.4