Closed Bug 109101 Opened 23 years ago Closed 23 years ago

Windows XP Menu item "Netscape 6.2 Mail" should not be hardcoded

Categories

(MailNews Core :: Simple MAPI, defect)

x86
Windows NT
defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED
mozilla0.9.8

People

(Reporter: srilatha, Assigned: rdayal)

References

Details

Attachments

(1 file, 4 obsolete files)

In the 0.9.4 branch, the string "netscape6.2 mail" that shows up in windowsXP 
start menu is hard coded. need to add a new resource so that this string is 
generic. To do this I need to something similar to what Bill did in bug # 102017
Need to fix this before we land it on the trunk.
Blocks: 104672
In order to display Mozilla / Netscape Mail in the Windows XP Start Menu it is
required to either set it as the system default which is done by the
defaultMailClient pref, or by setting "LocalizedString" attribute of the
Registry key for [HKLM\Software\Clients\Mail\Mozilla] /
[HKLM\Software\Clients\Mail\Netscape] as per the Microsoft support documentation.

Since we register the Mail as the system default mail client we need not
separately register it as the StartMenu mail client. Thus it is not required to
set the LocalizedString attribute mentioned above and as done in bug # 102017
which requires a resource id.

Thus fix for this could be just putting this string in the mapi.properties and
using it without bothering to load the Windows resource separately.
The string resource for the Mail display string would be different for both
Mozilla and Netscape which means we would need it in commercial tree too.

Hi Sean, can u please take a look at this patch. thanks - rajiv.
Comment on attachment 62789 [details] [diff] [review]
removes hard coding for making default mail client

r=ssu

is there a bug already filed for the ns tree?
Attachment #62789 - Flags: review+
No, I will file another bug in bugscape for this, thanks for the r= Sean.

Seth, can u please sr this one. - Rajiv.
Assignee: srilatha → rdayal
On second thoughts we can put this string in the brand.properties, it is a brand
name for Mail, what do u think Sean and Seth ? 
well, it's a product name just like the browser has a name too.  I would say
that it it a brand name, but I don't really know much about where it should go.
1) 

why does MakeMapiStringBundle() return NS_OK if it fails to get a bundle?  that 
should be fixed.

see my comments in #116993 about other style nits.

2) instead of doing a C style cast, use NS_STATIC_CAST()

3)  don't add "Mozilla Mail" or "Netscape Mail".  Use something 
like "brandShortName" entity from chrome://global/locale/brand.properties and 
add "%S Mail" to mapi.properties and build up the string from that.  that way, 
you won't have to do anything for the ns tree.
 

1) MakeMapiStringBundle never returns NS_OK if it cannot get the bundle, will
remove the check for (!bundle) from :
if (NS_FAILED(rv) || !bundle) return NS_ERROR_FAILURE;

other style nits : 

using :
if (NS_FAILED(rv))
   return NS_ERROR_FAILURE
instead of :
if (NS_FAILED(rv)) return NS_ERROR_FAILURE

will make it inconsistent with all the rest of MAPI code.

2) done.

3) Sean suggested that using a separate string that can be used for the Mail
display string will provide more flexibility for Marketing to define the string
rather than using "<brandName> Mail".
This patch removes the hardcoding using the <brandname> <version#> Mail rather
than using a separate string defined in mapi.properties file as suggested by
Seth.

I am using the useragent pref "misc" which is used by the useragent for
populating the useragent string for version #.
Attachment #62789 - Attachment is obsolete: true
Attachment #62908 - Attachment is obsolete: true
use this patch rather than above.. this one gets the string "Mail" from
mapi.properties besides getting brandname from brand.properties and version #
from  the useragent pref. This pref is used for creating the useragent string
in nsIHttpProtocolHandler which is also used in nsAppRunner.

Hi Seth, can u please take a look at this patch. thanks, - rajiv.
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla0.9.8
I'm not sure that your patch will do what you want.

"general.useragent.misc" is defined in all.js as

pref("general.useragent.misc", "rv:0.9.7+");

in all-ns.js, it is not defined, which means that for a commercial ns build, 
your final string will be "Netscape 0.9.7+ Mail" which is not correct.

is defined in all-ns.js as "6.2.1+" but that isn't defined in all.js

I'm not sure the right way to get the version, or the app and the version yet.
maybe shaver or dveditz knows.

some more comments:  

+        keyValuePrefix.AppendWithConversion (" ") ;
+        keyValuePrefix.AppendWithConversion (versionNo()) ;


1)  don't hard code the space and the order.  you should do something like this:

# localization note, $1%S is the app name,  $2%S is the version 
defaultMailDisplayTitle=$1%S $2%S Mail

that assumes there isn't a way to get "Netscape 6.x" or "Mozilla 0.9.7" 
already, and we have to get the version seperately.

2)

versionNo() should return a const PRUnichar *, and m_versionNo should be a 
nsString.  (m_brand should be a nsString, I'm not sure how m_thisApp and 
thisApplication() is used but I bet they should be converted as well.)

the reason for nsString instead of nsCString is so you do the conversion from 
cstring to string once, instead of every time you use it.

maybe I missed something.

were you planning to define pref("general.useragent.misc", "rv:6.2.1+"); [or 
whatever] in all-ns.js?

if so, that might work, but we need to check and see what the side effects of 
that would be.
Yes, but i found out that there is another pref in all-ns.js called
"general.useragent.vendorSub" showing the version # which is not defined in
all.js. I will go ahead use this one .. the way i would do this is check if i
get value for "general.useragent.misc" i use that else use
"general.useragent.vendorSub". Thus first case will work for Mozilla and second
for NS. 

However if somebody else defines "general.useragent.misc" later in all-ns.js it
would cause problems. But looking at the all.js and all-ns.js files it surely
looks like the useragent prefs are called differently for Mozilla and NS.

As per the other comments above :

1) I will change it to put the space delimiter in the properties file. I think
we should decide this once I the method for getting the version # is decided.

2) m_brand is Unicode, m_thisApp is retrieved and used as ASCII and not Unicode
and it and m_thisApplication is defined accordingly.


Dan or Mike, what's the correct way to handle this? Mozilla doesn't seem to
define vendor or vendorSub in prefs...is there a reason for this?
Hi David, Can u please sr this patch. thanks - Rajiv.
has r=ducarroz as per his comments above.

Hi David, can u please sr this patch. thanks, - Rajiv.
Attachment #62945 - Attachment is obsolete: true
Attachment #62969 - Attachment is obsolete: true
Attachment #62969 - Attachment is obsolete: false
Comment on attachment 62975 [details] [diff] [review]
above patch updated with review comments. 

Oops posted this patch in the wrong bug !!!!!

Please review the above the patch. thanks.
Attachment #62975 - Attachment is obsolete: true
Once again i posted the patch (id=62975) here by mistake please ignore it and
comments for it.

Please see patch : "updated fix using useragent.vendorSub pref for ns"
(id=62969), that is the one to be reviewed.
Comment on attachment 62969 [details] [diff] [review]
updated fix using useragent.vendorSub pref for ns

sr=bienvenu, but we should make sure this is the correct way to deal with these
prefs.
Attachment #62969 - Flags: superreview+
this is in the trunk.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
verified on trunk 2002042510
Status: RESOLVED → VERIFIED
Product: MailNews → Core
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: