387653 - Combining characters positioned wrongly in RTL text

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Simon Montagu :smontagu]

This is the main reason that reftests/bidi/bidi004.html and reftests/bidi/bidi004-j.html are failing on Linux. I'm making it a separate bug blocking bug 386567 rather than attaching the patch there, because there are some other minor issues with positioning that mean that the test still fails even with this fixed.

Comment 1

[Simon Montagu :smontagu]

Attachment: Patch

Comment 2

[Simon Montagu :smontagu]

Checked in.

Comment 3

[Simon Montagu :smontagu]

reftest checked in.

Comment 4

[Simon Montagu :smontagu]

The reftest isn't great: it fails on both Windows and Mac, and may start to fail on Linux (or pass on the other platforms) if the default font changes.

Comment 5

[Karl Tomlinson (:karlt)]

Interesting...
This relies on the order of glyphs for within each cluster being in reverse-logical order for RTL text.  This is contrary to Pango documentation but consistent with pango_ot_buffer_output().
See bug 375864 comment 6.

(Fortunately?) Hangul seems to work OK because, with ttf-alee, there is only one glyph per cluster.

Comment 6

[Daniel Holbert [:dholbert]]

(In reply to comment #4)
> The reftest isn't great: it fails on both Windows and Mac, and may start to
> fail on Linux (or pass on the other platforms) if the default font changes.

This bug's reftest (mozilla/layout/reftests/bidi/387653.html) has been failing for clean builds on my Ubuntu Linux box for a while now.  (not sure how long, but at least a month, I think)

From comment #4, it sounds like this failure was somewhat expected, though... should the test be edited / removed?

Here are data-urls for what I'm seeing:

  test:       http://tinyurl.com/3avo8n
  reference:  http://tinyurl.com/22rvbq

Comment 7

[Simon Montagu :smontagu]

I would be OK with removing the test if it's causing problems. layout/reftests/bidi/bidi-006.html is a better test that covers the same issue.

Comment 8

[Daniel Holbert [:dholbert]]

Would it be better to remove the test altogether, or to mark it "random" in the manifest?

Comment 9

[Simon Montagu :smontagu]

I removed it.

Comment 10

[(no longer active)]

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