Closed Bug 284927 Opened 19 years ago Closed 19 years ago

Arabic and Hebrew text with align=justify are rendered LtR when Pango is enabled

Categories

(Core Graveyard :: GFX: Gtk, defect)

x86
Linux
defect
Not set
major

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: zwnj, Assigned: smontagu)

References

(Blocks 1 open bug, )

Details

(Keywords: intl, Whiteboard: fixed on trunk)

Attachments

(3 files, 1 obsolete file)

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.6) Gecko/20050302 Firefox/1.0.1 Fedora/1.0.1-1.3.2
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.6) Gecko/20050302 Firefox/1.0.1 Fedora/1.0.1-1.3.2

Pango rendering of Firefox/Mozilla shows Arabic characters left-to-right instead
of right-to-left in a block (<P>, <DIV>, etc) if align=justify.  Glyphs (Arabic
shaped characters) are all right, but all the glyphs of line sorted LtR.

It may be bug of Pango, not Mozilla's binding.

Reproducible: Always

Steps to Reproduce:
1. Open the URL.

Actual Results:  
You'll see last paragraph unreadable.

Expected Results:  
You should see last paragraph like other ones, or it may be more longer. (If
Pango does Arabic justification well)
What build are you talking about here? Was it built with the patch for bug
214715?  What do you get when you type 'about:buildconfig' in the url bar?
Component: OS Integration → GFX: Gtk
Keywords: intl
Product: Firefox → Core
Summary: Display Arabic characters LtR on align=justify → Arabic text is rendered LtR with align=justify
Version: unspecified → Trunk
I use Fedora Core 3 with updates (and patches I think).  But it sould be in all
builds with MOZ_ENABLE_PANGO.


about:
----------------------------------------
Firefox 1.0.1
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.6) Gecko/20050302 Firefox/1.0.1
Fedora/1.0.1-1.3.2
----------------------------------------


about:buildconfig
----------------------------------------
Build tools
Compiler 	Version 	Compiler flags
gcc 	gcc version 3.4.2 20041017 (Red Hat 3.4.2-6.fc3) 	-Wall -W -Wno-unused
-Wpointer-arith -Wcast-align -Wno-long-long -pedantic -pthread -pipe
c++ 	gcc version 3.4.2 20041017 (Red Hat 3.4.2-6.fc3) 	-fno-rtti -fno-exceptions
-Wall -Wconversion -Wpointer-arith -Wcast-align -Woverloaded-virtual -Wsynth
-Wno-ctor-dtor-privacy -Wno-non-virtual-dtor -Wno-long-long -pedantic
-fshort-wchar -pthread -pipe -I/usr/X11R6/include

Configure arguments
--disable-ldap --disable-mailnews
--enable-extensions=cookie,xml-rpc,xmlextras,pref,transformiix,universalchardet,webservices,inspector,gnomevfs,negotiateauth
--enable-crypto --disable-composer --enable-single-profile
--disable-profilesharing --with-system-jpeg --with-system-zlib --with-system-png
--with-pthreads --disable-tests --disable-debug --disable-jsd
--disable-installer '--enable-optimize=-Os -g -pipe -m32 -march=i386
-mtune=pentium4' --enable-xft --enable-xinerama --enable-default-toolkit=gtk2
--enable-official-branding --disable-xprint --disable-strip --enable-pango
----------------------------------------
Assignee: bugs → blizzard
Blocks: 214715
Simon <smontagu> helped me to find more details about the bug.

Consider the arabic text is "ABCDEF GHIJKL." in logical format.  It should be
rendered as ".LKJIHG FEDCBA".  But it's "ABCDEF GHIJKL." now.

If I select the middle part of the text, it becomes "IJKL.DEF GHIABC".
I think the problem is here:

http://lxr.mozilla.org/seamonkey/source/layout/generic/nsTextFrame.cpp#3113

It used to be the case that nsTextFrame::PaintTextSlowly() ended up sending the
text to the rendering subsystem one character at a time, so we had to reverse
RTL text even on platforms that normally did that themselves. In Pango it seems
that that isn't true, so we need to rework the Bidi code there (like my patch in
bug 268113, but with an extra condition, and maybe an extra rendering hint).
I found another example for the bug.  In [1] there is a problem when attribute
'align' of text/paragraph is 'justify'.  In [2] there's a problem when attribute
'letter-spacing' is non-zero.

IMHO problem is where mozilla tries to position glyphs itself.
Blocks: Persian
I built a browser from the cvs with this configs:

--------------------------------
mk_add_options MOZ_CO_PROJECT=browser
mk_add_options MOZ_CVS_FLAGS=-z9
mk_add_options MOZ_MAKE_FLAGS=-j4

ac_add_options --enable-default-toolkit=gtk2
ac_add_options --enable-application=browser
ac_add_options --disable-mailnews
ac_add_options --disable-ldap
ac_add_options --disable-freetype2
ac_add_options --enable-pango
ac_add_options --enable-xinerama
ac_add_options --disable-composer
ac_add_options --disable-xtf

ac_add_options --prefix=/home/behnam/opt/mozilla/firefox
ac_add_options --enable-debug
--------------------------------


I get this assertion when select Persian lines:
--------------------------------
###!!! ASSERTION: mStart >= 0: '(sdptr->mStart >= 0)', file nsTextFrame.cpp,
line 6189
Break: at file nsTextFrame.cpp, line 6189
--------------------------------


But, when I print the page, positioning is almost right.  Doesn't Xprint related
to Pango render?
Summary: Arabic text is rendered LtR with align=justify → Arabic and Hebrew text with align=justify are rendered LtR when Pango is enabled
*** Bug 285007 has been marked as a duplicate of this bug. ***
See more testcases and screenshots in bug 285007
Attached patch Patch (obsolete) — Splinter Review
This is the same logic as my patch from bug 268113, with an added rendering
hint (0x40 not 0x20 so as not to conflict with blizzard's work in bug 260663).
Assignee: blizzard → smontagu
Status: UNCONFIRMED → ASSIGNED
Blocks: 268113
Are you going to request reviews for this patch, Simon?
Attachment #177367 - Flags: superreview?(roc)
Attachment #177367 - Flags: review?(roc)
You set NS_RENDERING_HINT_SPACING_RUNS, but you don't actually have any code to
pass the character and word spacing down into Gfx, do you?
Not yet :)

I anticipate that in the future we will want to do that for both Pango and
Uniscribe.
So isn't this going to disable character and word spacing, then?
We might have been at cross-purposes in comment 12 and 13. Currently the letter
and word spacing are calculated in nsTextFrame::RenderString and the results are
passed to gfx in the aSpacing argument of nsIRenderingContext::DrawString. I'm
not changing any of this at this point.
In that case we need a better comment for what NS_RENDERING_HINT_SPACING_RUNS
actually means.
there's a patch in ubuntu's firefox build that lets RTL work with pango, but
combining characters is still badly screwed up.

I dont know if its related to this patch or not, just that the non combining
hebrew characters at mechon-mamre work fine, while the combining characters are
still severly screwed up as per my screenshots.
Comment on attachment 177367 [details] [diff] [review]
Patch

r+sr with some changes that we discussed on IRC
Attachment #177367 - Flags: superreview?(roc)
Attachment #177367 - Flags: superreview+
Attachment #177367 - Flags: review?(roc)
Attachment #177367 - Flags: review+
Transferring r+sr=roc and requesting approval. Bidi languages are very broken
on Pango without this patch.
Attachment #177367 - Attachment is obsolete: true
Attachment #180683 - Flags: superreview+
Attachment #180683 - Flags: review+
Attachment #180683 - Flags: approval1.8b2?
Comment on attachment 180683 [details] [diff] [review]
Patch with those changes

a=asa
Attachment #180683 - Flags: approval1.8b2? → approval1.8b2+
Fix checked in. Leaving open because we probably want this on the Aviary and 1.7
branches too.
Whiteboard: fixed on trunk
Blocks: 290567
User: What happen ?
User 2: Somebody set up us the bomb^H^H^H^H bit.

(bug 290567)
Simon, does this affect non-pango builds?  Pango support is not checked into any
of the branches currently.
From comment 3 I thought that there were aviary branch builds with Pango.
*** Bug 261447 has been marked as a duplicate of this bug. ***
(In reply to comment #23)
> Simon, does this affect non-pango builds?  Pango support is not checked into any
> of the branches currently.

The bug has been reported with branch builds, e.g. bug 292842 with Build
Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.7) Gecko/20050416
Fedora/1.0.3-1.3.1 Firefox/1.0.3

Does Fedora have a customized build including Pango?
*** Bug 292842 has been marked as a duplicate of this bug. ***
(In reply to comment #26)

> Does Fedora have a customized build including Pango?

jshin answers this question in bug 292847 comment 4 (the answer is yes). In that
case, no branch checkin is required and this can be resolved.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
arabic is still rendered incorrectly with the patch from comment 17, as can be
verified from the test case attached to bug 261447
Attached image arabic is still broken
*** Bug 268113 has been marked as a duplicate of this bug. ***
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: