should we have a pref "default_mail_client"

VERIFIED FIXED in mozilla0.9.9

Status

MailNews Core
Simple MAPI
P2
critical
VERIFIED FIXED
17 years ago
10 years ago

People

(Reporter: Rajiv Dayal, Assigned: Rajiv Dayal)

Tracking

Trunk
mozilla0.9.9
x86
Windows NT
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

17 years ago
Since the MAPI setting for default mail application is stored in Windows
registry, do we need a separate pref "mailnews.default_mail_client" for
represting whether Mozilla is the default mail app ? A problem this poses is
that if some other messaging app is made the default mail client the MAPI
settings will be changed in the Windows registry but the Mozilla's pref
"mailnews.default_mail_client" will still be unchanged.
(Assignee)

Comment 1

17 years ago
The fix below removes the unnecessary and problematic MAPI pref
"mailnews.default_mail_client" and rather just uses the Windows regsitry
settings for setting / un-setting itself as the default mail client. 

This solves the stated problem in bug summary above and also related problems like: 
a) different settings seen for different profiles although there is only one
system wide setting for MAPI, b) mozilla setting itself as default mail
application without asking the user which may overwrite another messaging app's
settings and mapi Dll.
Severity: major → critical
Target Milestone: --- → mozilla0.9.9
(Assignee)

Comment 2

17 years ago
Created attachment 68685 [details] [diff] [review]
patch to fix problems related to MAPI settings

Hi JF and Scott,
Can you please r and sr the patch respectively.
thanks, 
- Rajiv.
1)
+                if (prefBranch.getBoolPref("defaultMailClient"))
+                    mapiRegistry.isDefaultMailClient = true ;
+                else
+                    mapiRegistry.isDefaultMailClient = false ;


could be:
   mapiRegistry.isDefaultMailClient = prefBranch.getBoolPref("defaultMailClient");

Also, you could define "defaultMailClient" as a constante instead of harcoding
it twice in the same file!


2)
+        if (parent.mapiPref.isDefaultMailClient)
+            mapiRegistry.isDefaultMailClient = true;
         else
-            mapiRegistry.unsetDefaultMailClient();
+            mapiRegistry.isDefaultMailClient = false;

again, could be:
   mapiRegistry.isDefaultMailClient = parent.mapiPref.isDefaultMailClient;

Appart that, looks good. R=ducarroz
(Assignee)

Comment 4

17 years ago
Created attachment 68912 [details] [diff] [review]
updated patch with changes as per comments

changed pref-mailnewsOverlay.js as per comment # 3. Also changed
accountUtils.js for the same.

thanks for your review, JF.
Attachment #68685 - Attachment is obsolete: true
(Assignee)

Updated

17 years ago
Attachment #68912 - Flags: review+

Comment 5

17 years ago
Comment on attachment 68912 [details] [diff] [review]
updated patch with changes as per comments

I couldn't tell who calls the method Startup that you added to pref-mailnews.js
.

That method looks like it contains only mapi related behavior anyway. Can't we
put it in the mapi prefs panel overlay and then when that overlay gets loaded
it calls Startup? That way, the base pref JS doesn't depend on MAPI and you
don't need to include all the special try/catch clauses checking for the
existence of a mapi function.
(Assignee)

Comment 6

17 years ago
If i define the onload property for pref-mailnewsOverlay it does not seem to 
call the function refered by it. Can I have an onload property for an overlay ?
I will need to look more into XUL here.

Comment 7

17 years ago
Sorry I wasn't clear about what I meant.

In your mapi overlay code, add a line of JS that looks like the following:

addEventListener("load", YourMapiOnLoadMethod, true);

now when the overlay gets loaded and it fires an on load event, your method will
get called. 
(Assignee)

Comment 8

17 years ago
Created attachment 69081 [details] [diff] [review]
new local load event handler for the mapi pref overlay, no need for Startup in base

Cool! I didnt know about this, thanks Scott for enlightening me.
Please find the update patch here.
Attachment #68912 - Attachment is obsolete: true

Comment 9

17 years ago
Comment on attachment 69081 [details] [diff] [review]
new local load event handler for the mapi pref overlay, no need for Startup in base

sr=mscott
Attachment #69081 - Flags: superreview+
(Assignee)

Comment 10

17 years ago
Thank you Scott for the review.
Fix checked in the trunk.

Trix can you please verfiy all the bugs blocked by this bug. Most of them 
should be fixed by this fix. thanks, - Rajiv.
Status: NEW → RESOLVED
Last Resolved: 17 years ago
Priority: -- → P2
Resolution: --- → FIXED

Updated

17 years ago
No longer blocks: 122377
(Assignee)

Updated

17 years ago
Blocks: 122377

Comment 11

17 years ago
I just filed a few bugs on 0.9.8 that could be affected by this change.
- bug 127762
- bug 127764
- bug 127766
- bug 127767

I'll have to download 0.9.9 when it's out and see how these are affected.

Have the user-visible (GUI-configurable) prefs changed due to these changes?

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

Comment 14

16 years ago
verified on mozilla & netscape trunk builds 2002022703
Status: RESOLVED → VERIFIED

Comment 15

16 years ago
*** Bug 133905 has been marked as a duplicate of this bug. ***
Product: MailNews → Core
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.