Bug 3435

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.

(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.

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.

Comment on attachment 2774 [details] Proposed patch r=me Make sure to add the test case to layout tests when landing.

Created attachment 2927 [details] layout test The old test lacked a font declaration, so it wasn't guaranteed to fail.

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