Bug 11811 – REGRESSION (r11783): Hebrew text in list boxes is reversed

• Status: RESOLVED
• Resolution: FIXED
• Component: Forms

Plain Hebrew text in list boxes is laid out in reverse, that is, left-to-right. See the attached test case. Notice that if you select Debug > Use ATSUI For All Text and reload, the text is laid out correctly.

This bug is a regression from fixing bug 4844 (SVN doesn't seem to have a revision number for that CVS-era change). It is currently affecting the engine-based list boxes but it also affected the previous implementation.

Created attachment 11823 [details]
Test case

Bug 11812 is similar. GraphicsContext::drawText is not suitable for drawing arbitrary strings.

[David Kilzer (:ddkilzer)]

(In reply to comment #0)
> This bug is a regression from fixing bug 4844 (SVN doesn't seem to have a
> revision number for that CVS-era change).

It's r11783.  (I ran "svn log WebKit/WebCoreSupport" and looked for the bug number in the commit text. :)

(In reply to comment #3)
> It's r11783.  (I ran "svn log WebKit/WebCoreSupport" and looked for the bug
> number in the commit text. :)
> 

Thank you!

[Stephanie Lewis]

In Radar 4947184

[Adele Peterson]

Created attachment 12891 [details]
patch

[Adele Peterson]

Comment on attachment 12891 [details]
patch

I'm going to rework this and post a new patch.

Things to address:

-malloc less by using a Vector<UChar>
- don't call bidi algorithm in the directional override case
- If there's one run, and its not reversed, return early

Comment on attachment 12891 [details]
patch

+        TextStyle textStyle(0, 0, 0, style()->direction() == RTL, style()->visuallyOrdered() == true);

Since the characters in the buffer are in LTR visual order (that's the result of bidiReorderCharacters), the flags should always be (..., false, true). This will ensure that ATSUI's own bidi algorithm doesn't kick in. The only exception is that if style()->direction() == RTL and style()->unicodeBidi() == Override, in which case you *may* skip bidiReorderCharacters and use (..., true, true). In the other override cases (LTR+Override or VisuallyOrdered) you skip bidiReorderCharacters and use (..., false, true).

[Adele Peterson]

Created attachment 12949 [details]
updated patch

I tried to address Mitz's comments with this patch.

There may be a cleaner way to put the direction override logic though.

[Adele Peterson]

Created attachment 12955 [details]
new patch!

Comment on attachment 12955 [details]
new patch!

r=me with one change discussed on IRC. Anders okayed the CharacterBuffer typedef.

[Adele Peterson]

Committed revision 19414.