187866 - English text overflows Arabic in RTL pages

Status: RESOLVED FIXED

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

Description

[Ossama Khayat]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.3a) Gecko/20021212
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.3a) Gecko/20021212

In the URL provided
(http://www.ccse.kfupm.edu.sa/~husni/ICS484/WebPAges/Munawes/Aracbic.htm),
English text in some pages flows over the next few letters of Arabic text. The
effect is obvious if you have Bold text.

Reproducible: Always

Steps to Reproduce:
1.
2.
3.




If you view the same page in IE it works fine.

Comment 1

[Ossama Khayat]

Attachment: A screen shot of the page in IE

Comment 2

[Ossama Khayat]

Attachment: The page dispalyed in Mozilla 1.3a

Comment 3

[Simon Montagu :smontagu]

I can reproduce this bug in older builds (e.g. Netscape 7.01 and Mozilla 1.2
final, but not in an up-to-date trunk build. It was probably fixed by bug
131023. Ossama, can you test with a nightly build?

Comment 4

[Ossama Khayat]

I installed the last build and tried the same URL but the problem is still there.
This is the build version
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.3b) Gecko/20030106

Comment 5

[Thamer Mahmoud]

Confirming using mozilla 2003010604 and phoenix 0.5 on WinXP

This bug only affects justified Arabic text rendered using "Traditional Arabic." 
I see no problems with "Times new Roman" or "Tahoma". 

See the attached testcase.

Comment 6

[Thamer Mahmoud]

Attachment: simplified testcase

Comment 7

[Simon Montagu :smontagu]

Accepting. I discovered that the reason that I couldn't reproduce the bug before
in my build was that I had turned off the flag at
http://lxr.mozilla.org/seamonkey/source/gfx/src/windows/nsRenderingContextWin.cpp#655
which makes Windows use its own shaping routines instead of Mozilla's.

It might be better to ignore NS_RENDERING_HINT_ARABIC_SHAPING anyway for
justified text. We already ignore NS_RENDERING_HINT_BIDI_REORDERING, and
ignoring one and not the other is probably problematical.

Comment 8

[Simon Montagu :smontagu]

Attachment: Patch

This is equivalent to code which already exists in PaintTextSlowly() and
GetPositionSlowly(). Not having the same condition in Reflow() was causing
inconsistencies.

Comment 9

[Simon Montagu :smontagu]

Attachment: Same patch with diff -u9w for clarity

Comment 10

[Simon Montagu :smontagu]

Fix checked in.

Comment 11

[Ossama Khayat]

Thank you very much Simon. It's now solved. I'll attach a screen
shot also.

Comment 12

[Ossama Khayat]

Attachment: RTL works excellent :)

Comment 13

[(no longer active)]

Mass-assigning the new rtl keyword to RTL-related (see bug 349193).