82347 - Bidi: Persian and Urdu numbers do NOT display left-to-right

Status: VERIFIED FIXED

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

Description

[Roozbeh Pournader]

From Bugzilla Helper:
User-Agent: Mozilla/4.0 (compatible; MSIE 5.5; Windows NT 5.0)
BuildID:    2001051120

In the page at http://www.persianacademy.ir/, the number at the
center of screen should be "1380", from left to right, but in Mozilla,
it is rendered as "0831". This problem is with all Persian and Urdu
digits (the characters in the Unicode range U+06F0..U+06F9), which
should be treated as "European Numbers" in the Unicode bidi algorithm,
but Mozilla treats them otherwise.


Reproducible: Always

Steps to Reproduce:
1. Open the page at "http://www.persianacademy.ir/".

Comment 1

[Roozbeh Pournader]

Attachment: Correct screenshot from IE 5.5

Comment 2

[Simon Montagu :smontagu]

Tried that page on Hebrew and US Windows NT.

On US Windows the numbers display correctly, but on Hebrew Windows they are
reversed. This may be something that will be corrected by the recent
improvements to bidi reordering (bug 81664).

BTW, Mozilla definitely classifies 06F0 - 06F9 as European numbers, but in this
context (after Arabic letters) rule W2 of the Unicode Bidi Algorithm applies so
they are reclassified as Arabic numbers. Whether this is correct for Persian I'm
not qualified to say, but it shouldn't make any difference except to surrounding
terminators and separators. 

In other words I agree that this is a bug, but disagree with your analysis of
the reason :-)

Comment 3

[Roozbeh Pournader]

I just tested the page on US Win98, and it was correctly displayed. So the bug 
should be only on bidi-enabled platforms.

BTW, you are right in your interpretation of the Bidi algorithm, but I was 
worrying that the numbers may have been classified as 'Right-to-Left' or 'Right-
to-Left Arabic'.

Comment 4

[Simon Montagu :smontagu]

Attachment: Suggested patch

Comment 5

[Simon Montagu :smontagu]

The above patch fixes this bug, but probably more investigation is needed to be
sure that no other characters should be excluded from #define IS_ARABIC_ALPHABETIC.

cc-ing Maha Abou El-Rous

Comment 6

[Roozbeh Pournader]

Would someone please check in this patch? It's really small and won't create any
problems for anyone, other than solving one of the important problems of Persian
and Urdu support.

Comment 7

[Mike Kaply [:mkaply]]

r= mkaply

We need to get ftang made an sr to replace erik for i18n.

Comment 8

[Maha Abou El-Rous]

added [e-mail] to the CClist

Comment 9

[Zach Lipton [:zach]]

Mass-move all BiDi Hebrew and Arabic qa to me, [e-mail]. 
Thank you Gilad for your service to this component, and best of luck to you 
in the future.

Sholom. 

Comment 10

[Roozbeh Pournader]

How can I get someone sr this? This is really a small patch provably with no
side effect.

Comment 11

[Zach Lipton [:zach]]

Getting this in for 0.9.4 will be difficult as it requires drivers approval. After 
0.9.4 is branched, I'll try to rope in an sr for it.

Comment 12

[Roozbeh Pournader]

0.9.4 is released now. Would you ask someone for sr?

Comment 13

[Roozbeh Pournader]

Zach, the patch has been available for three months now. Any chance for a sr?

Comment 14

[Zach Lipton [:zach]]

emailed [e-mail] to try to push this along, I'll try irc on 
monday as well.

Comment 15

[kinmoz]

The patch looks pretty simple, but before I give an sr=, has anyone addressed
Simon's comment?

> The above patch fixes this bug, but probably more investigation is needed to
> be sure that no other characters should be excluded from #define > 
IS_ARABIC_ALPHABETIC.

Comment 16

[Simon Montagu :smontagu]

I have double-checked the Unicode Character Database, and I believe that the
only characters that might cause a problem are 

066A;ARABIC PERCENT SIGN
066B;ARABIC DECIMAL SEPARATOR
066C;ARABIC THOUSANDS SEPARATOR

All other characters in the U0600-U06FF range are either actual Arabic letters
or diacriticals, or neutrals.

I haven't been able to produce any broken cases using these characters. Roozbeh
and Maha, can you confirm?

Comment 17

[Roozbeh Pournader]

I can't either produce anything wrong with U+066A, U+066B, and U+066C, using 
0.9.5. I can't get in the details now, I'm just testing them using what I 
remember from Bidi, and compare it with IE. Things should be fine.

BTW, I just tried to build Mozilla from 0.9.5 sources plus the patch in this 
bug, and tested it on Windows 2000, but sadly the bug is not fixed. This is the 
first time I built Mozilla, so the problem may be from my side. Would someone 
please confirm?

Comment 18

[Roozbeh Pournader]

I am sure now that the original patch doesn't fix the problem.

Mahar, any idea for what else needs to get fixed?

Comment 19

[Simon Montagu :smontagu]

I get an error today when I try to connect to http://www.persianacademy.ir/ but
a test case that I prepared displays correctly on W2K with this patch.

Can anyone else test and report results?

Comment 20

[Simon Montagu :smontagu]

Attachment: Testcase - various combinations of text and numerals

Comment 21

[Maha Abou El-Rous]

Simon, tested with your testcase on Win2k & WinNT, the problem is still there 
(number order is reversed). The default locale on both systems was Arabic.
Tried with Farsi as the default locale on Win2k and its not working as well.

Comment 22

[Mike Kaply [:mkaply]]

I am on a Win2K US system, and the testcase and the webpage work fine for me on 
the current Mozilla.

Is this problem only specific to Arabic Windows?

and if so, is a global change the right thing to do here?

Comment 23

[Maha Abou El-Rous]

This problem does not exist on a US system (probably because then we're using 
Mozilla's own ordering & shaping routines. On a Bidi-enabled system, we leave 
both the ordering & shaping to the system.)

Tested with Hebrew Win2k system (no Arabic locale installed), and still have 
the same problem (number order reversed). 
PS. Arabic Locale must be installed on the system to be able to set Farsi or 
Urdu Locales.

Comment 24

[Roozbeh Pournader]

The system I am testing on has a US locale, but has Arabic support enabled. And
the numbers are reversed with or without the patch in this bug.

Michael, would you please go to "Control Panel | Regional Options | General",
and in the "Language settings for the system" put a check beside "Arabic"? Let
us then
see where can we get...

Comment 25

[Simon Montagu :smontagu]

In case the fix was dependent on some other fix in my local tree, I tried a
fresh pulled build with only the patch in attachment 35962 [details] [diff] [review], and was still unable
to reproduce the problem. My system is W2K Version 5 (Build 2195), tested with
both Hebrew and English US locales, with Hebrew and Arabic language support.

I would much appreciate it if some other people could test the patch and compare
results. Please note that after applying the patch you should do a depend or
clobber build of the entire tree, not just the intl subdirectory.

Comment 26

[Simon Montagu :smontagu]

Attachment: Screenshot of correct display of attachment 54477

Comment 27

[Maha Abou El-Rous]

tested after applying the patch and making clobber build of the entire tree, 
not just the intl subdirectory, and it works well

Comment 28

[Roozbeh Pournader]

I think we sould check the patch in. At least it resolves the bug in some cases...

Comment 29

[Simon Montagu :smontagu]

kin, are you ready to give sr= now?

Comment 30

[kinmoz]

Comment on attachment 35962 [details] [diff] [review]
Suggested patch

sr=[e-mail]

Thanks for all the testing.

Comment 31

[Simon Montagu :smontagu]

Fix checked in

Comment 32

[Simon Montagu :smontagu]

Maybe I was too hasty marking this FIXED. Roozbeh, can you test it in a nightly
build from November 14 when it comes out, and reopen if necessary?

Comment 33

[Roozbeh Pournader]

The bug is fixed in 2001111603. Check both attachment 54477 [details] and the website
<http://www.persianacademy.ir>. Verifying.