Bug 3435 – Parentheses are backwards in Hebrew text (no bidi mirroring?)

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Ari]

Parentheses in Hebrew text seem to be displayed in the wrong direction. In the attachment, the 
parentheses should open toward the word (i.e. be around the word), but instead open away from the word.

I think right and left parentheses are among the characters that must be replaced with "mirror" characters 
according to the Unicode spec. From a very brief perusal of the code, it seems that WebKit doesn't 
implement this.

See:
http://www.unicode.org/reports/tr9/
http://www.unicode.org/Public/UNIDATA/BidiMirroring.txt

[Ari]

Created attachment 2247 [details]
testcase

(In reply to comment #1)

I can't seem to reproduce this. (I think mirroring is mostly handled by ATSUI, although you can see that 
WebKit/WebCoreSupport.subproj/WebTextRenderer.m:2041 does take it into account in width 
calculations). There are mirroring-related issues in the rendering of "visually-ordered Hebrew", where 
mirroring happens although it shouldn't.

[Ari]

(In reply to comment #2)
> (In reply to comment #1)
> 
> I can't seem to reproduce this. (I think mirroring is mostly handled by ATSUI, although you can see 
that 
> WebKit/WebCoreSupport.subproj/WebTextRenderer.m:2041 does take it into account in width 
> calculations). There are mirroring-related issues in the rendering of "visually-ordered Hebrew", where 
> mirroring happens although it shouldn't.

I've done a little more investigating, and it seems that WebTextRenderer.m has two code paths for text 
rendering, CG and ATSUI, and it uses ATSUI to render Hebrew. WebTextRenderer.m:2041 is on the CG 
code path, and so it never gets executed for Hebrew.

The ATSUI code path sets the text direction with kATSULineDirectionTag in 
_createATSUTextLayoutForRun, 
but its unclear whether ATSUI is supposed to do mirroring automatically. If it is, maybe it's an ATSUI 
bug.

What OS version are you using? I'm on 10.4.1

I've actually managed to fix this by putting in a hack that calls u_charMirror() in the ATSUI code path.

(In reply to comment #3)

> What OS version are you using? I'm on 10.4.1

10.4.1, Safari 2.0 (412), Standard Font: Times 16, Default Encoding: Western (ISO Latin 1), and I get the 
same results with the CVS version. I remember mirroring being an issue in early releases, but I also 
remember that it has been fixed some time ago.

This may provide an important clue:
<http://forums.ort.org.il/scripts/showsm.asp?which_forum=307&mess=2242612>
A user reporting that he's seeing reverse parentheses and that his work around is disabling Arial in Font 
Book. Some Hebrew-speaking Mac users install a version of Arial that comes from Microsoft (and 
includes Hebrew), so maybe that's the culprit.

[Nicholas Shanks]

Arabic also has problems. Where 12345 represents arabic characters as encoded in the bytestream, I see:

<p dir="rtl">(12345)</p>

come out as:

                                                                      )(54321

This is not related to Arial Unicode MS as the font used is Geeza Pro.

(In reply to comment #3)

Looks like an ATSUI issue with certain fonts. Here's how I confirmed this:
Built ATSUIFontPanelDemo from /Developer/Examples/ATSUI/Font Panel Support/, run it, changed the 
text to Hebrew text in parentheses, then tried various fonts. Lucida Grande, New Peninim MT and Arial 
Hebrew do the mirroring fine, while that Arial from Microsoft (Version 2.98; Unique name: Monotype:Arial 
Regular:Version 2.98 (Microsoft)) gives reversed parentheses.

You should probably file a bug against ATSUI on Radar.

(In reply to comment #5)
> Arabic also has problems. Where 12345 represents arabic characters as encoded in the bytestream, I 
see:
> 
> <p dir="rtl">(12345)</p>
> 
> come out as:
> 
>                                                                       )(54321
> 
> This is not related to Arial Unicode MS as the font used is Geeza Pro.

I can reproduce this with the latest shipping Safari but not with the latest CVS version (which includes a 
fix for paragraphs beginning with neutrals). What do you get when the paragraph doesn't begin with a 
neutral, e.g.
<p dir="rtl">12 (345) 67</p>
?

Analysis:

ATSUI is supposed to handle mirroring, but it does so only if the font's glyph properties table ('prop') 
contains the mirrored glyph information. Thus mirroring doesn't happen with fonts that don't have that 
information in their 'prop' (or don't have a 'prop' at all), and when ATSUI uses a fallback font, but the 
original font doesn't have mirroring information.

There seems to be no way (short of looking at the 'prop' table) of finding out what ATSUI is going to do 
with mirrored characters. However, there is a way to force ATSUI to never mirror, and then 
WebTextRenderer could always mirror (like it does for CG). This can be done by reversing the RTL text 
run and prepending an LRE prior to passing to ATSUI, and not to setting the ATSUI line direction to RTL.

Another option is to drop ATSUI altogether for Hebrew (I don't know about Arabic) and use CG. This 
would solve other issues with Hebrew (such as poor choice of fallback fonts and no support for text 
justification, letter spacing and word spacing).

Created attachment 2774 [details]
Proposed patch

This patch tries to guess whether ATSUI will perform mirroring or not, based on
the existence of a 'prop' table for the font, and applies mirroring itself it
ATSUI will not.

[Maciej Stachowiak]

Comment on attachment 2774 [details]
Proposed patch

r=me

Make sure to add the test case to layout tests when landing.

[Geoffrey Garen]

Created attachment 2927 [details]
layout test

The old test lacked a font declaration, so it wasn't guaranteed to fail.

[Adele Peterson]

using <rdar://problem/4215911> to track this.