122377 - Mozilla misbehave with Windows mail settings

Status: VERIFIED FIXED

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

Description

[Serge Skorokhodov]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:0.9.8) Gecko/20020124
BuildID:    2002012423

If the checkbox "Use Mozilla Mail as the default mail application" is set to
OFF, Mozilla "nullifies" the settings of default windows mail application.

Reproducible: Always
Steps to Reproduce:
1. Set the default mail application to say OE
2. Start Mozilla with flag "Use Mozilla Mail as the default mail application"
set to OFF.
3. Observe the result in Windows Internet settings:(

Actual Results:  If flag "Use Mozilla Mail as the default mail application" is
off, none of the applications is set as Windows default for mail.

Expected Results:  The default windows mail application should be unchanged.

Comment 1

[Sören 'Chucker' Kuklau (gone)]

Confirmed on 2002-01-28-09, Win32 (XP Pro), MathML/SVG-enabled.

Comment 2

[Matthias Versen [:Matti]]

Reporter:
Have you this line in your profile\prefs.js file ?

user_pref("mailnews.default_mail_client", true);

Can you delete the file windows\system32\mapi32_moz_bak.dll. between step 1 and 2 ?

Comment 3

[Serge Skorokhodov]

#1:

  My pref.js contains user_pref("mailnews.default_mail_client", true);.

#2:

  There is no windows\system32\mapi32_moz_bak.dll in winnt (I use w2k) directory.

Comment 4

[Serge Skorokhodov]

Sorry, I've just cut'n'paste:( Answer #1 should read
user_pref("mailnews.default_mail_client", false);

Sorry for that, just in a hurry:)

Comment 5

[Jeff Thieleke]

Attachment: Fixes RestoreBackedUpMapiDll()'s mishandling of mapi32.dll

**** Fix before 0.9.8! ****

I can confirm this bug, and I've attached the fix.

The problem is that nsMapiRegistryUtils::RestoreBackedUpMapiDll() (called by
nsMapiRegistryUtils::unsetDefaultMailClient() at startup) deletes mapi32.dll
before checking if the backup file, Mapi32_moz_bak.dll, exists.  So if
Mapi32_moz_bak.dll does not exist in the system directory, Mozilla deletes
mapi32.dll, and has nothing with which to replace it.  The result is that MAPI
is disabled until the user takes whatever action is necessary to re-enable
their mail application as the default MAPI client.


To reproduce:

1. Unset Mozilla as the default mail client, and exit Mozilla.
2. Delete Mapi32_moz_bak.dll from the Windows system directory, if present.
3. Start Netscape 4.x (or any MAPI application), and make it the default MAPI
client.  Leave Netscape 4.x running.
4. Start Mozilla.
5. Notice that Mapi32.dll has been deleted from the system directory, and
verify that Netscape 4.x confirms that it is no longer the default MAPI client.
 Try the Windows context menu item 'Send To->Mail Recipient' to further verify
that Mozilla has disabled MAPI.


As I mentioned in the first line of this comment, this bug really needs to be
fixed before 0.9.8.  Messing up people's prefs, especially when it disabling
the default mail client, is a Very Bad Thing.  RestoreBackedUpMapiDll() is
clearly doing the the wrong thing, and the fix is quite simple.

Comment 6

[Jeff Thieleke]

Attachment: Un-tab-ified version of previous patch

Comment 7

[Matthias Versen [:Matti]]

Jeff Thieleke: you need 2 reviews for your patch (r= and sr=)

Please read http://www.mozilla.org/hacking/.
BTW: It could be too late for 0.9.8...

AFAIK:
>1. Unset Mozilla as the default mail client, and exit Mozilla.
>2. Delete Mapi32_moz_bak.dll from the Windows system directory, if present.

If you unset Mozilla as default mail client, mozilla (should) delete it's own
Mapi file and rename the backup file to Mapi32.dll

>3. Start Netscape 4.x (or any MAPI application), and make it the default MAPI
client.  Leave Netscape 4.x running.
>4. Start Mozilla.

Mozilla should do nothing because you unset it as default mail application.
If you set Mozilla as default mail application again, mozilla should rename the
Mapi32.dll to Mapi32_moz_bak.dll and copy his own Mapi32.dll from the mozilla
directory into the system directory.
(we have already a bug that mozilla fails to rename the mapi32.dll if the there
is already a Mapi32_moz_bak.dll in the system directory.)

>5. Notice that Mapi32.dll has been deleted from the system directory, and
verify that Netscape 4.x confirms that it is no longer the default MAPI client.
 Try the Windows context menu item 'Send To->Mail Recipient' to further verify
that Mozilla has disabled MAPI.

the Mapi32_moz_bak.dll is the backup for the old Mapi32.dll that the old Mapi
client used (and not mozilla's Mapi32.dll)

and i think that this bug is a dupe of bug 103230

Comment 8

[Rajiv Dayal]

If we donot delete the Mozilla's version of mapi32.dll when you uncheck the
preference, irrespective of whether a previous version of mapi32.dll exists or
not, all MAPI calls will still be directed to Mozilla. So it DOES have to delete
the mapi32.dll even if mapi32_moz_bak.dll doesnot exist when you uncheck the
preference. Thus RestoreBackedUpMapiDll() should not be changed.

If no mapi32.dll exists, which can be the case if Mozilla deletes the mapi32.dll
and does not have a previous version to replace with, the other messaging apps
should still be able to put its own mapi32.dll and enable mapi. Also this could
be a case with a new Windows desktop which doesnot have any mail application
installed and hence no mapi32.dll.

Also at startup nsMapiRegistryUtils::unsetDefaultMailClient() should be called
only if the preference has changed, even from outside Mozilla (by editing
prefs.js or by the autoconfig). If you are seeing that it is called every time,
we need to figure out why.

Comment 9

[Jeff Thieleke]

> Jeff Thieleke: you need 2 reviews for your patch (r= and sr=)
>
> Please read http://www.mozilla.org/hacking/.
> BTW: It could be too late for 0.9.8...

What is your point in recommending http://www.mozilla.org/hacking?  I know what
sort of reviews are required, and realize that 0.9.8 has branched, which makes
it all more urgent that this simple, low risk fix gets in as soon as possible.  

As for your comments on the steps to reproduce, obviously that is what *should*
be happening, but judging from Serge's experience, my experience, and a lot of
grumbling that I hear on discussion forums like MozillaZine, this problem isn't
isolated.  Do you question that fact that RestoreBackedUpMapiDll() is doing the
wrong thing by deleting mapi32.dll without checking that the backed-up DLL is
even available?  Do you doubt that this patch would fix the problem described
and easily reproduced?  

Although fixing RestoreBackedUpMapiDll() is good enough to fix my problem of
Mozilla deleting mapi32.dll at every startup, the whole MAPI switching system
needs to be re-examined. I mean, calling functions like unsetDefaultMailClient()
 at every startup, especially when Mozilla isn't the default client to begin
with, is highly questionable and can easily lead to problems like are described
in this bug and several other MAPI bugs that sound suspiciously similar.  While
I don't believe that this bug is identical to Bug 103230 (I reviewed all of the
MAPI bugs before posting the patch to Bug 122377), the large number of similar
bugs tends to indicate that the MAPI switching code needs work.

Comment 10

[Jeff Thieleke]

> If no mapi32.dll exists, which can be the case if Mozilla deletes the 
> mapi32.dll and does not have a previous version to replace with, the 
> other messaging apps should still be able to put its own mapi32.dll 
> and enable mapi. Also this could be a case with a new Windows desktop 
> which doesnot have any mail application installed and hence no mapi32.dll.

But the whole point is that Mozilla is deleting a mapi32.dll that it didn't put
there, and doesn't have a backup to replace.  Saying that deleting another app's
mapi32.dll is ok because the app can restore it is just silly.  Why should a
user have to make the additional effort of re-enabling their MAPI client just
because they started up Mozilla?



> Also at startup nsMapiRegistryUtils::unsetDefaultMailClient() should be 
> called only if the preference has changed, even from outside Mozilla 
> (by editing prefs.js or by the autoconfig). If you are seeing that it 
> is called every time, we need to figure out why.

Agreed.  I will look into this more tonight, but it should be easy enough for
anyone to reproduce.

Comment 11

[Rajiv Dayal]

I never said that Mozilla should delete some other app's mapi32.dll, please read
my comments carefully again -- "If we donot delete the Mozilla's version of
mapi32.dll ""when you uncheck the preference"" ".

This case will occur only if unsetDefaultMailClient() is called at startup
everytime, it should be called only when the pref "mailnews.default_mail_client"
changes as I also mentioned in my comments above.

Comment 12

[Jeff Thieleke]

If you look into this, you'll see that SetIsDefaultMailClient() is *always*
called, as long as the user has "mailnews.default_mail_client" set, regardless
of the value, in prefs.js.  See pref_HashPref(), and in particular,
BOGUS_DEFAULT_BOOL_PREF_VALUE to see why this is so.  

So the progression goes: app startup -> mailnews.default_mail_client changed ->
SetIsDefaultMailClient(false) -> unsetDefaultMailClient() -> !isSmartDll() ->
RestoreBackedUpMapiDll() -> delete mapi32.dll with no backup.  

Something in that chain has to be altered - the prefs changed code and
RestoreBackedUpMapiDll() seem like a good candidates to me.  But without
changing prefs, modifying RestoreBackedUpMapiDll() would localize the change. 
If there is the need to delete mapi32.dll, create a new function or add a
parameter to RestoreBackedUpMapiDll() so that mapi32.dll is unconditionally
deleted *only* when Mozilla is actually the default MAPI client, not when the
preference "changed" from false to false...

Comment 13

[Rajiv Dayal]

the pref changed code should be the one that should change in this case. I will
look into this .. maybe there should not be a pref associated with this at all
since the settings for default Mail client (MAPI preferences) is stored in
Windows registry and that is the one that should be checked (and which is also
checked currently) rather than keeping another pref for it.

Rather than putting in any quick fixes i would rather go for the right solution.

Comment 14

[Jeff Thieleke]

Attachment: Updated patch to prevent non-Mozilla mapi32.dll files from being deleted

> the pref changed code should be the one that should change in this case. I
will
> look into this .. maybe there should not be a pref associated with this at
all
> since the settings for default Mail client (MAPI preferences) is stored in
> Windows registry and that is the one that should be checked (and which is
also
> checked currently) rather than keeping another pref for it.

The registry setting (HKLM\Software\Clients\Mail) is only effective if you are
using the MAPI stub DLL, the so-called smart DLL.  The first problem is not all
users will have the stub DLL, since it wasn't introduced until Windows 2000, IE
5, or Outlook 2000.  In addition, if the user is using an older mail client,
such as Netscape 4, it will typically remove the stub DLL and replace it with
its own version. 

So Mozilla will still need to be able to do the messy business of replacing
mapi32.dll with its own version, and switching back when it is no longer the
default MAPI client.  What it shouldn't do, and what my patch addresses, is
deleting some other client's mapi32.dll just because unsetDefaultMailClient()
is called and there is no Mapi32_moz_bak.dll available.

Rajiv, your point about having to delete mapi32.dll even when no backup is
available when Mozilla is unselected as the default MAPI client is taken. 
While the prefs issue should be addressed, modifying unsetDefaultMailClient()
to not delete some other client's mapi32.dll must also be done.  This function
already has a check to prevent the deletion of the stub DLL, so my latest patch
is just extending the check to also avoid deleting other non-Mozilla mapi32.dll
files.

Comment 15

[Rajiv Dayal]

nsMapiRegistryUtils::isSmartDll() is used to take care of all that you mentioned
about replacing the mapi dll if it is not the smart one, as you also know.

Anyway, the case that the unsetDefaultMailClient() will delete somebody's mapi
dll should never happen, the function unsetDefaultMailClient should be called
only when the preference changes thus making sure that somebody else's dll will
not be deleted by it. So the right fix here is to make sure that
unsetDefaultMailClient is called only in the case when the preference changes.

Jeff, the fix you have is a temporary solution, the right solution is to make
sure that unsetDefaultMailClient() or for that matter even
setDefaultMailClient() should not be called everytime on startup.

As I mentioned earlier another issue is whether we need to have this pref or
not. I am currently working on that (new bug # 123596). The fix for that will
fix this problem too.

Comment 16

[Jeff Thieleke]

> nsMapiRegistryUtils::isSmartDll() is used to take care of all that you
> mentioned about replacing the mapi dll if it is not the smart one, as you also
> know.

isSmartDll() is only used to detect the presence of the stub MAPI DLL.  It has
nothing to do with detecting the presence of a 3rd party MAPI DLL...


> Anyway, the case that the unsetDefaultMailClient() will delete somebody's mapi
> dll should never happen, the function unsetDefaultMailClient should be called
> only when the preference changes thus making sure that somebody else's dll will
> not be deleted by it. So the right fix here is to make sure that
> unsetDefaultMailClient is called only in the case when the preference changes.

You seem to be in denial about the current situation wrt
unsetDefaultMailClient().  The case that it deletes somebody's MAPI DLL exists
now.  It shouldn't do that, regardless on how or why it is called.  I agree that
the preference situation should be fixed, but adding an extra layer of security
by making unsetDefaultMailClient() smart enough to not to the wrong thing is not
a "quick fix" or hack.


> Jeff, the fix you have is a temporary solution, the right solution is to make
> sure that unsetDefaultMailClient() or for that matter even
> setDefaultMailClient() should not be called everytime on startup.

My fix is *the* solution to the problem of unsetDefaultMailClient() deleting a
3rd party MAPI DLL.  In *addition* to this, {un}setDefaultMailClient() should
not be called at every startup, but that is more of a performance issue.  Fixing
that , but not fixing unsetDefaultMailClient() is the real hack, since it does
nothing to prevent someone from incorrectly using unsetDefaultMailClient() in
the future.

Comment 17

[Matthias Versen [:Matti]]

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

Comment 18

[Rajiv Dayal]

In that case setDefaultMailClient should be fixed too so that if some other
app is made the default mail client, on startup Mozilla should not make itself
the default client and overwrite the other application settings without the
user's knowledge.

Also as per your patch (id=67889) :

+nsresult nsMapiRegistryUtils::unsetDefaultMailClient() {
+    // Just return if Mozilla isn't already the default MAPI client
+    if (!IsDefaultMailClient()) return NS_OK;
+

the IsDefaultMailClient() function checks if the mapi dll is neither a smart one
nor a Mozilla's version, which means its somebody's else's dll and thus the MAPI
settings in Windows Registry is for some other messaging app.

If the MAPI setting are not for Mozilla then what is the point in asking Mozilla
to unset its settings ?

Further in case for some "wrong reasons" unsetDefaultMailClient is called, as
per the patch (id=67889) should we return after just checking the mapi Dll
version or should we also check the registry settings too ? Also should we
return NS_OK in this case ?

Comment 19

[Jeff Thieleke]

> In that case setDefaultMailClient should be fixed too so that if some other
> app is made the default mail client, on startup Mozilla should not make itself
> the default client and overwrite the other application settings without the
> user's knowledge.

This is debatable, and I don't think there is perfect solution.  But if one were
to edit mailnews.default_mail_client to true offline, then start Mozilla, I
think that it would be expected that Mozilla would make itself the deafult MAPI
client at startup.  The same should be true if the user changes the setting via
the Preference menu.  So changing setDefaultMailClient() is probably not the
right thing, although it would prevent Mozilla and another program from fighting
it out.  Ultimately, the user needs to pick a single MAPI client, and Mozilla
can only try to do what it is configured to do.
   


> the IsDefaultMailClient() function checks if the mapi dll is neither a smart    
> one nor a Mozilla's version, which means its somebody's else's dll and thus 
> the MAPI settings in Windows Registry is for some other messaging app.
>
> If the MAPI setting are not for Mozilla then what is the point in asking
> Mozilla to unset its settings ?

Exactly.  If Mozilla's mapi32.dll or the stub/smart DLL are not installed, there
is no possibility that Mozilla is the default MAPI client.  Therefore,
unsetDefaultMailClient() is pointless (and currently dangerous), no matter why
it was called, and it should return without changing anything.


> Further in case for some "wrong reasons" unsetDefaultMailClient is called, as
> per the patch (id=67889) should we return after just checking the mapi Dll
> version or should we also check the registry settings too ? Also should we
> return NS_OK in this case ?

Unless I'm missing something, just verifying that neither the Mozilla nor stub
DLL are installed is good enough, since as I stated above, if a 3rd party
mapi32.dll is installed, there is no way that Mozilla could possibly be the
default MAPI client.  Therefore, there is no need to check the registry unless
the stub DLL is installed, which is how the patched unsetDefaultMailClient()
works in my patch.  

As for returning NS_OK, originally I had NS_ERROR_FAILURE, but the more I
thought about it, if Mozilla already isn't the default client,
unsetDefaultMailClient() is technically successful.  Also,
unsetDefaultMailClient() is currently only called one place in the code,
SetIsDefaultMailClient(), which will report an error if unsetDefaultMailClient()
returns an error.

Comment 20

[Ian Jefferies]

Bug confirmation for Windows 98Me. Currently using public release 0.9.8, this
behaviour not seen in 0.9.7 and earlier.

During Moz startup the registry key for identifying the mail client is cleared
to an empty string ("") when Moz is *not* the default mail client. i.e. this is
happening with user_pref("mailnews.default_mail_client", false);

Here's a registry access dump of the relevent change to the registry made by Moz
during every startup:

Mozilla
CreateKey
HKLM\Software\Clients\Mail
SUCCESS
hKey: 0xC19063C0	
Mozilla
SetValueEx
HKLM\Software\Clients\Mail
SUCCESS
""

Mozilla
CloseKey
HKLM\Software\Clients\Mail
SUCCESS
	

Moz has never been my default mail client, and fortunately my mail client
(Turnpike) recovers gracefully.

I've not hacked the Moz source, nor written for MAPI, so I HTH.

Comment 21

[Jeff Thieleke]

The behavior that Ian describes in Comment #20 occurs in
nsMapiRegistryUtils::unsetDefaultMailClient() when HKLM\Software\Clients\Mail is
empty or missing (name.IsEmpty()).  This registry problem, as well as the
non-Mozilla mapi32.dll deletion described in the comments above, are resolved
when the patch in attachment (id=67889) is applied.

Can I get a review on this patch?  I think it is pretty clear that the patch in
id=67889 does the right thing, since Mozilla should not be touching either the
registry or mapi32.dll when it isn't the default MAPI client.  If we want to fix
the problem of {un}setDefaultMailClient() being called at app startup, then
that's good, but unsetDefaultMailClient() should be fixed now.

Comment 22

[Ian Jefferies]

>The behavior that Ian describes in Comment #20 occurs in
>nsMapiRegistryUtils::unsetDefaultMailClient() when HKLM\Software\Clients\Mail is
>empty or missing (name.IsEmpty()).

Just to clarify: this HKLM\Software\Clients\Mail value is the one stored in the
branch HKLM\Software\Mozilla\Desktop (presumably as the backup of the setting
Moz originally found.)

The HKLM\Software\Clients\Mail registry branch as used by the MAPI smart stub
DLL is correctly set to my expected mail client (Turnpike), and is neither empty
nor missing when Moz overwrites it.


IMHO unsetDefaultMailClient() should be smart enough to leave the system
untouched if it determines that no change is required or possible. Given that
the purpose of the function is to make changes to the system that other
applications use, that function should not rely on the caller to protect it
(supporting Jeff's position).

Comment 23

[Rajiv Dayal]

The patch on bug # 123596 should solve all the problems mentioned on this bug,
both related to unsetDefaultMailClient and setDefaultMailClient and other MAPI
settings related problem seen on bugs 118616 and 120134.

With the bug # 123596 fix the case seen here with unsetDefaultMailClient will
never happen. Although after that fix unsetDefaultMailClient will never be
called for wrong reasons (and which should be the case) we can use Jeff's latest
patch to make  it extra safe. But we should wait to first land the big patch on
bug # 123596 first.

Comment 24

[Matthias Versen [:Matti]]

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

Comment 25

[[not reading bugmail]]

rdayal,

I use w2k, is there any code in place here or another bug filed for checking
whether Moz is not a netscape or other distribution release,  because I have
problems with this because I run Netscape 6.2.1 for my webmail and mozilla for
browsing/newsgroups.  They share registry keys, which there should be some
mechanism for distribution release using seperate registry keys.  You change a
setting in moz for this, it will change that setting in netscape.  if you change
netscape it will change moz, so they always have the same values and they will
go back and forth as soon as you change something.  A distribution release can
never be a different App than Moz to the OS with this in place.  We should get
that fixed, let me know if you wanna new bug filed, or point me to another bug.

btw, this also affects filehanding preferences.

-dman84

Comment 26

[Rajiv Dayal]

The fix for bug # 123596 has been checked in which should also solve the problem
as reported in the summary of this bug.

Trix, can you please verify if the preference for default mail application is
set from checked to unchecked, or if another messaging app is made as the
default and then Mozilla is started, the Windows MAPI registry settings should
show the settings for, as well as the mapi dll should be of, earlier application
or the new application. thanks.

Dennis, please see bug # 120359 for the problem you are seeing.

Comment 27

[[not reading bugmail]]

thanks,

I see I'd already posted in there.. umm. thanks for getting me back :)

Comment 28

[R.K.Aa.]

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

Comment 29

[R.K.Aa.]

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

Comment 30

[R.K.Aa.]

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

Comment 31

[Rajiv Dayal]

The problem(s) described in this bug has been fixed as part of the patch checked
in for bug # 123596. 

However the latest patch here is useful to improve the unsetDefaultMailClient.
Opening another bug for it, bug # 125830. Moving the patch there.

Marking this one as fixed. Also tested with build 2002021503.

Comment 32

[R.K.Aa.]

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

Comment 33

[Matthias Versen [:Matti]]

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

Comment 34

[Trix Supremo]

confirmed with Mozilla's 2002022103 build. The default setting is not mapi
enabled. The correct setting is made when mapi is enabled and returns the prior
setting when Mozilla is uninstalled.  

Comment 35

[Matthias Versen [:Matti]]

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

Comment 36

[Alfonso Martinez]

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

Comment 37

[Matthias Versen [:Matti]]

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

Comment 38

[Matthias Versen [:Matti]]

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

Comment 39

[Adrian Ulrich]

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

Comment 40

[Adrian Ulrich]

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

Comment 41

[Frederic Bezies]

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