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".
Attached patch fix using brandname and version # (obsolete) (deleted) — Splinter Review
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.
Attached patch above patch updated with review comments. (obsolete) (deleted) — Splinter Review
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: