99108 - Bidi (Hebrew and Arabic) Encoding can not be set as the default encoding for composing mail

Status: RESOLVED FIXED

(Core :: Layout: Text and Fonts, defect)

Description

[Shoshannah Forbes]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Win98; en-US; rv:0.9.3+) Gecko/20010910
BuildID:    2001091003

In the prefrences, the user can set a Bidi (Hebrew and Arabic) Encoding for
reading (incoming) mail. However, that option is absent of the list of avalible
 default encoding for composing (outgoing) mail.

Reproducible: Always
Steps to Reproduce:
1. Open Mozilla, go to Edit --> Prefrences --> Mail and Newsgroups --> Messege
display
2. Notice that Hebrew and Arabic Encodings are present in the list of languages
the user can set as default.
3. Go to Edit --> Prefrences --> Mail and Newsgroups --> Messege composing
4. Notice the list of avalible default encodings.

Actual Results:  Bidi encodings are not in the list for composing mail

Expected Results:  Bidi languages should be in the list

It doesn't help much (or make very much sense) to be able to read mail in
Hebrew, but not really reply in Hebrew...

Comment 1

[Simon Montagu :smontagu]

If you open a new message, and then go to View->Character Coding->Customize..,
you can add and remove encodings from the list. If you then go back to
Preferences->Mail & News->Message Composition, the list of encodings will
reflect your changes.

This behaviour seems unintuitive. Could/Should there be a link to the Customize
dialog from the prefs? cc-ing UI folks.

See also bug 84181 concerning the initial list of default encodings.

Comment 2

[Katsuhiko Momoi]

When we designed the Mail Compose Character Coding menu,
we selected what were then known default/standard encodings
for mail for each lang/script groups. Currently you should find
Hebrew (ISO-8859-8-I) on the default list for Mail Composer.
It was thought that if the default list was constructed well, 
most users will not have to tinker with the pref dialog to add new
ones to choose from. 

I don't mind seeing a link to the Customize link from the Pref UI.
However, adding a good set of default encodings (for both Hebrew &
Arabic) might be sufficient and we may not have to create
a link in the Pref UI.

Here's one question -- is the current default insufficient for Hebrew?
Is this one of the scripts that need to list multiple encodings?
We made an exception to one script/one default mail compose encoding
in the case of Cyrillic because Russians used KOI8-R while Bulgarians
overwhelmingly preferred Windows-1251 and Ukrainians use KOI-U.

I prefer not to proliferate Mail encodings unless there are strong reasons
for doing so. So, let's hear about Hebrew the default mail encoding(s).

Comment 3

[Simon Montagu :smontagu]

Hebrew and Arabic are not currently on the default list. I'll attach a patch.

Comment 4

[Katsuhiko Momoi]

> Currently you should find Hebrew (ISO-8859-8-I) on 
> the default list for Mail Composer.

I misspoke in the above remark. I happened to be testing
ISO-8859-8-I a few weeks back and I added to my Mail
Compose character coding list and had forgotten about it.
Sorry. 
The mail default encodings for Hebrew and Arabic should have been
added when the support for these script got into the Character
Coding menu a while back. I think it's been our oversight not
to do that.

Comment 5

[Simon Montagu :smontagu]

Attachment: Add Arabic and Hebrew ISO standard encodings to the default list

Comment 6

[Katsuhiko Momoi]

Adding 2 (missing ) standand mail encodings for Arabic & Hebrew, 
ISO-8859-6 and ISO-8859-8-I, respectively, to the Mail Compose 
Character Coding menu. 
Looks good to me. r=momoi

Comment 7

[Simon Montagu :smontagu]

Does this change require super-review?

Comment 8

[Katsuhiko Momoi]

Seth, please you super-review this. 0.9.4 is already out
and so wil try to check into the current branch first.

It would be great if we can also check this into the
branch, however.
juraj, can you help with the branch issue?

Comment 9

[Katsuhiko Momoi]

A very simple, safe fix with no impact to L10n. 
We should have added these 2 encoding names when we started
supporting Arabic and Hebrew.
Asking for PDT approval for 0.9.4.

Comment 10

[Jaime Rodriguez, Jr.]

Momoi-san: We need to see a r and sr before we consider taking this one.

Comment 11

[Katsuhiko Momoi]

Jaime, there is already an "r" above by me.
We are awaiting a sr from Seth.

Comment 12

[Jaime Rodriguez, Jr.]

Ftang/Momoi - Is this a stop ship? We are only taking stop ships at this time.
Why is this needed for this release? Did we get a lot feedback on 6.1 requesting
this change?

Pls have the status next to the patch attachment read "has-reviewed".

Comment 13

[Katsuhiko Momoi]

The lack of this simple UI change (without an effect to L10n)
significantly impedes mail composition for Arabic & Hebrew
users. These 2 should have been added when we put in support
for these languages. The change is simple and safe.

Comment 14

[[e-mail] (away - not reading bugmail)]

>juraj, can you help with the branch issue?

I'd be happy to. I believe we'd need PDT+ to land this on the branch.

Comment 15

[kinmoz]

Comment on attachment 49018 [details] [diff] [review]
Add Arabic and Hebrew ISO standard encodings to the default list

sr=[e-mail]

Comment 16

[Jaime Rodriguez, Jr.]

Looks like a good thing to take. Ftang and Momoit, pls come by the PIT today @
noon for Express Approval.

Comment 17

[Jaime Rodriguez, Jr.]

Giving it an nsbranch+, so it can be reviewed by the PDT.

Comment 18

[Jaime Rodriguez, Jr.]

check it in today - PDT+

Robinf - Any docs impact on this?

Comment 19

[Katsuhiko Momoi]

Simon, please check the fix into the trunk.
Juraj, please check the fix into the branch.

Comment 20

[robinf]

Jaime - no doc impact. Thanks for checking.

Comment 21

[Simon Montagu :smontagu]

fix checked into trunk

Comment 22

[[e-mail] (away - not reading bugmail)]

per offline discussion with Kat Momoi - I'm ready to check this into the 0.9.4 
branch as soon as it opens.

Comment 23

[Katsuhiko Momoi]

Someone from NS IQA, please verify this fix when the branch 
build gets the fix checked in. The fix should appear in tomorrow's
0.9.4 branch build.

The trunk already has the fix in and it can be verified with today's
trunk build.

Comment 24

[marina]

in today's trunk build ( 09-20-03) there is no Arabic(ISO-8859-6) nor Visial
Hebrew ( ISO-8859-8) in the default charset list for Mail Composition. Those two
are on the list for Mail Display.

Comment 25

[Simon Montagu :smontagu]

Marina, did you test with a clean profile? If you have customized the mail edit 
encodings (i.e. you have a line beginning 

user_pref("intl.charsetmenu.mailedit",

in prefs.js), it will override navigator.properties and you won't see the 
change.

Notice also that you should be looking for Arabic (ISO-8859-6) and Hebrew (ISO-
8859-8-I), not Visual Hebrew.

Comment 26

[Katsuhiko Momoi]

Simon, I looked at the source of today's runk build published
internally. The fix you put in is not in the navigator.properties
file. 

I think somehow the build this morning did not pick up your
fix. We were having a build problem today and so I am not 
surprised by it. 

I just pulled and built from the trunk source now and the change you made 
is there and working as intended.

Let's wait till tomorrow's trunk build and hear what IQA has to say about it.
It will be OK most probably tomorrow. If not, we will look into build issues.

Comment 27

[Mike Kaply [:mkaply]]

The commercial build has its own navigator.properties as part of the l10n, so 
this change will need to be placed there as well.

Comment 28

[Katsuhiko Momoi]

Tao or Juraj, can you help wit the trunk build issue?

Comment 29

[tao]

Please look at 97606. I had landed the fix on trunk. PDT suggested we bake it 
on the trunk first and check it into the branch. 

Comment 30

[Katsuhiko Momoi]

> Please look at 97606. I had landed the fix on trunk. PDT 
> suggested we bake it  on the trunk first and check it into 
> the branch. 

Comments on the above:

1. I don't think we have to bake and shake this simple fix
   on the trunk before checking it into the branch. Bug 97606 involved
   makefile changes. This one simply modifies one property in
   the navigator.properties file.
2. The fix here is not platform-dependent as in Bug 97606. The same
   changes apply to all platforms. There is no L10n implication.

So for trunk, all that needs to be done is to check the fix into

...xpfe/browser/resources/locale/en-US/navigator.properties

Isn't that all that needs to be done? 

In any case, this needs to get checked into the 0.9.4 branch
ASAP. Also we need to verify that the fix shows up in the build tomorrow
for the trunk. Juraj, please take care of the branch check-in and
if the trunk checkin needs further assistance, please do so.

Comment 31

[marina]

I am looking at this morning trunk build : 2001-09-21-05 and there is no Arabic
(ISO-8859-6) nor Hebrew (ISO-8859-8-I) in the default list for message compose (
i have a clean build, starting with new profile.

Comment 32

[[e-mail] (away - not reading bugmail)]

per discussion with Kat Momoi, we need to change the commercial 
navigator.properties as well:

http://lxr.mcom.com/commercial/source/xpfe/browser/resources/locale/en-
US/navigator.properties

Roy can you help with this?

Comment 33

[[e-mail] (away - not reading bugmail)]

the patch has just been checked into the Mozilla 0.9.4 branch, Roy said he 
might be able to help with the commercial navigator.properties. I'm reassigning 
the bug to him as suggested by Kat Momoi.

Comment 34

[Roy Yokoyama]

accepting. I'll check in the patch to ns 0_9_4_branch in a minute.

Comment 35

[Roy Yokoyama]

checked-in

Comment 36

[Simon Montagu :smontagu]

I thought this was the most trivial fix possible, and am surprised at how much 
trouble it has caused. Thanks to everyone who helped out.

Comment 37

[Katsuhiko Momoi]

Re-opening ...

The fix is in Mozilla trunk and 094 branch. It is also in commercial
0.9.4 branch. But it is not in commercial trunk builds as of 9/22/2001.

Comment 38

[Roy Yokoyama]

Accepting.
I'll check in the patch to the commercial trunk today.
Thanks

Comment 39

[Roy Yokoyama]

Changing the whiteboard and keyword to reflect the current status
of this bug.

Comment 40

[Roy Yokoyama]

checked in to the commercial trunk tree.