If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Text is not diplayed for the simple MAPI preference checkbox

VERIFIED FIXED in mozilla0.9.8

Status

MailNews Core
Simple MAPI
P1
normal
VERIFIED FIXED
16 years ago
9 years ago

People

(Reporter: Srilatha Moturi, Assigned: Srilatha Moturi)

Tracking

({regression})

Trunk
mozilla0.9.8
x86
Windows NT
regression

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 4 obsolete attachments)

(Assignee)

Description

16 years ago
For the preference "Use Netscape Mail as the default mail application" 
only the checkbox is displayed and there is no text.
(Assignee)

Updated

16 years ago
Keywords: regression
Priority: -- → P1
Target Milestone: --- → mozilla0.9.8
(Assignee)

Comment 1

16 years ago
I believe the text is not being displayed because the pref-mailnewsOverlay.dtd 
file which was part of messenger-mapi package, was moved from en-US.jar to 
en-win.jar.
(Assignee)

Comment 2

16 years ago
Created attachment 65349 [details] [diff] [review]
patch v1

register chrome messenger-mapi from en-win.jar

Comment 3

16 years ago
Comment on attachment 65349 [details] [diff] [review]
patch v1

the patch works for me.  r=ssu
Attachment #65349 - Flags: review+
(Assignee)

Comment 4

16 years ago
ccing dveditz for sr
Attachment #65349 - Flags: superreview+
I am giving sr=dveditz for this patch to move the messenger-mapi chrome, but I
note that registering this component is *only* in the windows version of this
script when tao and the i18n team worked very hard to make the locale installers
XP (note the code to figure out which platform they're on, and to always ship
all three platform files).

Tao: suggestions needed here. registerChrome() call errors are ignored so
there's no particular problem in adding this call in all versions of this
script. Better, the call could always be conditional "if (platformNode == 'win')"

In any case the currently checked-in langenus.jst files do not match XP
(Assignee)

Comment 6

16 years ago
Created attachment 65368 [details] [diff] [review]
patch for ns tree

Comment 7

16 years ago
If we are moving "messenger-mapi" package from "en-US.jar" to "en-win.jar", then
we should register it as "messenger-mapi-platform". You will need to do the same
thing  for both package and locale providers and the referencing urls.
(Assignee)

Comment 8

16 years ago
Created attachment 65442 [details] [diff] [review]
changed the package name from messenger-mapi to messenger-mapi-platform

I changed the package name from meesnger-mapi to messenger-mapi-platform based
on the suggestion from Tao.
Attachment #65349 - Attachment is obsolete: true
Attachment #65368 - Attachment is obsolete: true
(Assignee)

Comment 9

16 years ago
Created attachment 65443 [details] [diff] [review]
changed the package name from messenger-mapi to messenger-mapi-platform

I changed the package name from meesnger-mapi to messenger-mapi-platform based
on the suggestion from Tao.
(Assignee)

Comment 10

16 years ago
Comment on attachment 65442 [details] [diff] [review]
changed the package name from messenger-mapi to messenger-mapi-platform

some how got two attachments for the same patch
Attachment #65442 - Attachment is obsolete: true
(Assignee)

Comment 11

16 years ago
Created attachment 65445 [details] [diff] [review]
patch for the ns tree

Changed the name from messenger-mapi to messenger-mapi-platform.
Sean, Dan can you review the patches.

Comment 12

16 years ago
Comment on attachment 65443 [details] [diff] [review]
changed the package name from messenger-mapi to messenger-mapi-platform

looks good.
Attachment #65443 - Flags: review+

Updated

16 years ago
Attachment #65445 - Attachment is patch: true
Attachment #65445 - Flags: review+
I don't see why the name needed to change to -platform since this is an entire
platform-specific feature. For the others we needed a -platform name so we could
distinguish between the xp navigator (for example) locale stuff and the
platform-specific navigator tweaks. Now we have the content and skin called
"messenger-mapi" and the locle not matching as "messenger-mapi-platform".  The
-mapi tag already flagged it as platform-specific, we don't need both tags.

Now that 0.9.8 is frozen I'd feel a lot better with the original smaller patch
(which in fact had my sr= for the mozilla tree).

Tao, you didn't answer my original question -- aren't the langenus.jst scripts
intended to be XP the same? It wasn't an issue about whether -platform should
now be in the package name, it was whether this command should be added to the
mac and linux versions of the scripts and whether the script should be
conditional so mac and linux people don't get the windows content installed.

Comment 14

16 years ago
>I don't see why the name needed to change to -platform since this is an entire
>platform-specific feature. For the others we needed a -platform name so we could
>distinguish between the xp navigator (for example) locale stuff and the
>platform-specific navigator tweaks. Now we have the content and skin called
>"messenger-mapi" and the locle not matching as "messenger-mapi-platform".  The
>-mapi tag already flagged it as platform-specific, we don't need both tags.
It's a naming convention so people can tell from the package name that this
resource is platform-specific.  In addition, having a consistent naming convention
help us to locate all platform specifc resource by searching substring,
"*-platform".

>Now that 0.9.8 is frozen I'd feel a lot better with the original smaller patch
>(which in fact had my sr= for the mozilla tree).
I think doing it in the right way precedes the tree freeze-ness. Has the previous 
patch being checked in? If not, why does this matter?

>Tao, you didn't answer my original question -- aren't the langenus.jst scripts
>intended to be XP the same? It wasn't an issue about whether -platform should
>now be in the package name, it was whether this command should be added to the
>mac and linux versions of the scripts and whether the script should be
>conditional so mac and linux people don't get the windows content installed.

You are right. The registerChrome() command part should be conditioned. I'll
revoke the "r=". But, let's reach a conclusion first so srilatha can work on the
final patch.

Comment 15

16 years ago
Comment on attachment 65443 [details] [diff] [review]
changed the package name from messenger-mapi to messenger-mapi-platform

the register chrome part needs to be the script should be conditional so mac
and linux people don't get the windows content installed.

let's wait until a conclusion is reached about the package name.
Attachment #65443 - Flags: review+ → needs-work+

Updated

16 years ago
Attachment #65445 - Flags: review+ → needs-work+
The mac and linux people won't get en-win installed so they won't get the
content, the conditional around the registerChrome() call is just to prevent
potential wierd errors that might happen if you try to register something that
isn't there.

By renaming the locales to "messenger-mapi-platform" the locale URLs no longer
match the content and skin URLs they modify. I realize
chrome://navigator-platform/locale/ applies to chrome://navigator/content/, but
there is a lot of legitimate chrome://navigator/locale/ as well. In this case
the messenger-mapi content is just as single-platform as the locale files.

If you as locale module owner want to insist on the hideous
"messenger-mapi-platform" (could it be just plain "messenger-platform"?) then as
a super-reviewer I'm going to insist that all chrome://messenger-mapi/ content
and skin get changed to match.

If you want this fixed in 0.9.8 you'll need drivers@mozilla.org a= anyway, let
them arbitrate the name issue. I'm betting they'll go for the smaller patch.

Comment 17

16 years ago
>The mac and linux people won't get en-win installed so they won't get the
>content, the conditional around the registerChrome() call is just to prevent
>potential wierd errors that might happen if you try to register something that
>isn't there.
You're right. I obviously slipped my tongue.

>By renaming the locales to "messenger-mapi-platform" the locale URLs no longer
>match the content and skin URLs they modify.
The postfix, "-platform", serves as a modifier to the package name.
"chrome://foo-platform/content" exists because "chrome://foo-platform/locale"
won't be a valid locale provider unless there is a corresponding package (aka, 
content) to apply. This is how the chromeRegistry concept works. We don't see
"chrome://foo-platform/skin/" because we don't package/design skin by platform now.
> I realize chrome://navigator-platform/locale/ applies to 
> chrome://navigator/content/, but there is a lot of legitimate 
> chrome://navigator/locale/ as well. In this case the messenger-mapi content
> is just as single-platform as the locale files.
There are two types resources we package in en-win.jar:
1. resources exist on more than one platform but their values differ by 
   platforms. For example, key-binding, command button name (such as "Quit" 
   and "Exit"), etc.
2. resources exist on a particular platform only. For example, "net2phone", 
   WinHook, Unix PrintDialog, and mapi.

What I am proposing here is a naming convention that we can adopt across
modules (package names in fact), easy to remember and apply.

I admit it does not cover all situations well but it works.

> If you as locale module owner want to insist on the hideous
> "messenger-mapi-platform" (could it be just plain "messenger-platform"?) 
Yes, this is one option to make the package name less hideous. Alternatives are
"chrome://mapi-platform/locale" or "chrome://messenger-platform/locale/mapi". The
latter implies "mapi" is a sub-module of "messenger" and also eliminate the need
two packages, "messenger-mapi" & "messenger-mapi-platform" -> save us the time
to extra chromeregsitry -> performance gain!

> then as a super-reviewer I'm going to insist that 
> all chrome://messenger-mapi/ content and skin get changed to match.
We need "chrome://messenger-mapi-platform/content" to make 
"chrome://messenger-mapi-platform/locale" a valid locale provider. Since there 
is no platform specifc skin for mapi, we don't need to introduce 
"chrome://messenger-mapi-platform/skin".

> If you want this fixed in 0.9.8 you'll need drivers@mozilla.org a= anyway, 
> let them arbitrate the name issue. I'm betting they'll go for the smaller 
> patch.
For fixing the problem described in this bug report, I am fine either way. But,
for the checkin to the trunk, might I suggest that we follow the existing naming
convention? IMO, "chrome://messenger-platform/locale/mapi" is a better solution.

(I will be out until Jan 22, '02.)

Comment on attachment 65368 [details] [diff] [review]
patch for ns tree

>@@ -73,6 +72,7 @@
>       registerChrome(chromeType, cf, localeName + "pipnss/");
>       registerChrome(chromeType, cf, localeName + "pippki/");
> 
>+      registerChrome(chromeType, pf, localeName + "messenger-mapi/");
>       registerChrome(chromeType, pf, localeName + "global-platform/");
>       registerChrome(chromeType, pf, localeName + "communicator-platform/");
>       registerChrome(chromeType, pf, localeName + "navigator-platform/");


sr= dveditz if you change this to

>+      if (platformNode == "win")
>+          registerChrome(chromeType, pf, localeName + "messenger-mapi/");

and I'd prefer if it were at the end of the platform registrations rather
than first. sr=dveditz ditto conditions for the original mozilla patch

I have filed a separate bug to make the three platform langenus files match
again on the trunk, and as part of that bug people can discuss changing
messenger-mapi to messenger-platform or mapi-platform. For now (trying to
get into 0.9.8) go with the smaller change, but include the "if" so
localization
project page continues to work XP.
Attachment #65368 - Flags: superreview+
Attachment #65368 - Flags: review+
Attachment #65368 - Attachment is obsolete: false
(Assignee)

Comment 19

16 years ago
Created attachment 65657 [details] [diff] [review]
patch for mozilla tree

Updated the patch based on dveditz comments. 
Added the if condition
Attachment #65443 - Attachment is obsolete: true
Attachment #65445 - Attachment is obsolete: true
(Assignee)

Comment 20

16 years ago
Comment on attachment 65657 [details] [diff] [review]
patch for mozilla tree

Based on dveditz comments marking it so
Attachment #65657 - Flags: superreview+
Attachment #65657 - Flags: review+
indeed, sr=dveditz for the mozilla patch. You'll do the same on ns, right?

Comment 22

16 years ago
Comment on attachment 65657 [details] [diff] [review]
patch for mozilla tree

don't forget to update the ns tree too.
r=ssu
Attachment #65657 - Flags: superreview+
(Assignee)

Updated

16 years ago
Attachment #65657 - Flags: superreview+
(Assignee)

Comment 23

16 years ago
Yes, I will do the same for ns tree. Didn't post another patch because it is 
exactly the same as the one for mozilla tree.
a=blizzard on behalf of drivers for 0.9.8
Keywords: mozilla0.9.8+
(Assignee)

Comment 25

16 years ago
Fix checked into both mozilla and ns tree
Status: NEW → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED

Comment 26

16 years ago
verified on mozilla and netscape trunk builds 2002022703
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.