104943 - Preference MAPI code needs to broadcast message that default mail client has changed.

Status: VERIFIED FIXED

(MailNews Core :: Simple MAPI, defect)

Description

[Sean Su]

Currently, when the WinXP's Mail Start menu item has been updated, it does not
get refreshed until a restart of the system.  This means that the Mail item in
WinXP's Start menu still reflects the old mail client until the system has been
restarted.

The following is currently done by the browser code to update WinXP's Start menu
when it has been updated by us:
  ::SendMessage( HWND_BROADCAST, WM_SETTINGCHANGE, 0,
(LPARAM)"Software\\Clients\\StartMenuInternet" );
This should also work for when the default mail client has been changed.

Comment 1

[Sean Su]

Actually, the function and parameters to use are:
   ::SendMessage( HWND_BROADCAST, WM_SETTINGCHANGE, 0,
(LPARAM)"Software\\Clients\\Mail" );

** Notice that the last parameter is (LPARAM)"Software\\Clients\\Mail".

Comment 2

[Srilatha Moturi]

Attachment: patch v1

Comment 3

[Srilatha Moturi]

In the patch SendMessage is getting called in SetDefaultMailClient() and 
UnsetDefaultMailClient(), after setting the HKLM and HKCU registry keys 
successfully.

Comment 4

[Srilatha Moturi]

Attachment: patch v2

Comment 5

[Srilatha Moturi]

Attachment: patch with more context

Comment 6

[Srilatha Moturi]

In the last patch I modified the SetRegistryKey because if you are trying to set 
a registry key with value "" and the key doesn't exist already, the key is not 
being created.

Comment 7

[Jaime Rodriguez, Jr.]

PDT-, this looks like a polish, cosmetic type bug (i.e. No gross loss of
functionality, crash, or dataloss). Sorry, but its a little too late for these
types of fixes. Let's get it next time.

Comment 8

[Sean Su]

The uninstaller is also not doing this.  I filed bug 104982.

Comment 9

[Jaime Rodriguez, Jr.]

tentative PDT+, pending positive reviews.

Comment 10

[Sean Su]

Comment on attachment 53683 [details] [diff] [review]
patch with more context

r=ssu
Patch has also been tested under my WinXP
system.

Comment 11

[Srilatha Moturi]

Attachment: patch with 20 lines of context

Comment 12

[lchiang]

esther - can you also help test this once fixed since Sean mentioned you saw
this problem on your XP machine?

Comment 13

[Srilatha Moturi]

Attachment: updated patch with new varibale names

Comment 14

[Srilatha Moturi]

Attachment: updated patch

Comment 15

[Daniel Veditz [:dveditz]]

Comment on attachment 53833 [details] [diff] [review]
updated patch

>@@ -572,54 +567,56 @@
>                 PRInt32 index = appPath.RFind("\\");
>                 if (index != kNotFound)
>                     appPath.Truncate(index + 1);
>                 appPath += "mozMapi32.dll";
>                 keyName.Assign("Software\\Clients\\Mail\\");
>                 keyName.Append(appName.get());
>                 result = SetRegistryKey(HKEY_LOCAL_MACHINE, 
>                                         (char *)keyName.get(), 
>                                         "DLLPath", (char *) appPath.get());
>+                mailKeySet = result;
>             }
>         }
>     }
>     if (!name.IsEmpty())
>         if (NS_SUCCEEDED(result))
>-            result = SetRegistryKey(HKEY_LOCAL_MACHINE, 
>+            mailKeySet = SetRegistryKey(HKEY_LOCAL_MACHINE, 
>                                 "Software\\Clients\\Mail", 
>                                 "", (char *)name.get());
>     else
>-        result = SetRegistryKey(HKEY_LOCAL_MACHINE, 
>+        mailKeySet = SetRegistryKey(HKEY_LOCAL_MACHINE, 
>                                 "Software\\Clients\\Mail", 
>                                 "", "");

Need some braces in this if-if-else, the indentation doesn't match what's
actually happening. Don't know if it's a bug or merely a whitespace problem.


Might be better to hold off on that lone mailkeySet = result until all the
mailkeysetting is done, that is, do it after or in the if(!name.IsEmpty())

>+    if (NS_SUCCEEDED(mailKeySet)) {
>+        ::SendMessage(HWND_BROADCAST, WM_SETTINGCHANGE, 0, 
>+                     (LPARAM)"Software\\Clients\\Mail");
>         UnregisterServer(CLSID_nsMapiImp, "mozMapi", "mozMapi.1");
>-        if (NS_SUCCEEDED(rv))
>-            return rv;
>+        return desktopKeySet;
>     }
>-    return NS_ERROR_FAILURE;
>+    return mailKeySet;

Why do we go ahead and set the desktop integration flag about the mailkey setting
independently of whether that setting in the windows registry actually worked?

Comment 16

[Srilatha Moturi]

Attachment: updated patch based on Dan's comments

Comment 17

[Daniel Veditz [:dveditz]]

Comment on attachment 53841 [details] [diff] [review]
updated patch based on Dan's comments

Thanks for all the work
sr=dveditz

Comment 18

[Sean Su]

Comment on attachment 53841 [details] [diff] [review]
updated patch based on Dan's comments

r=ssu

Comment 19

[Srilatha Moturi]

Fix checked into 0.9.4 branch

Comment 20

[Tiantian Kong]

Trix:

Please verify this bug on tomorrow's build ASAP. Many thanks.

Comment 21

[esther]

The Yes button hangs the app, still testing

Comment 22

[esther]

Reopening.  When selecting the Yes button, the app hangs, need to do a
ctrl+alt+del to get out to the app. Netscape 6 becomes the default and preferred
(shows up in Start Menu) is set. however Netscape 6 doesn't recognize it's the
default app when launched.  This regression is bad.

Comment 23

[Sean Su]

I tried today's branch build on both NT4 and XP.  Both worked as expected.  I 
did not hang after clicking on the "Yes" button.

Under XP, the Start Menu was updated correctly.

Comment 24

[Sean Su]

Can someone else try to verify this fix on another XP system other than Esther's 
or my machine?

Comment 25

[esther]

Actually I was using xp build 2600 and jason has that build too.  We tried this
scenario on his system and it didn't hang up. I need to find out if there is
something specific that causes this that I have overlooked because using
yesterdays build on this same system works, where today's build doesn't.  More
investigation going on...

Comment 26

[esther]

Other things were happening, like the Browser not knowing it was the default so
Sean had me deinstall all NS6, clean up my registry, make IE default Browser and
OL Express default mail.  Then reinstall today's build.  Now it works fine. I
then cleaned up my defaults and registry again and installed yesterday's build
to verify those were OK, then installed today's build in a different directory
just to make sure yesterday's build wasn't leaving me in a state that caused my
problems today.  All works fine now.  Can be resolved as fixed again and I will
verify.  

Comment 27

[esther]

resolving as fixed, since I reopened it.

Comment 28

[esther]

verified