689736 - parentheses are flipped in RTL (e.g. Hebrew) text

Status: RESOLVED FIXED

(Core :: Layout: Text and Fonts, defect)

Description

[Tsahi Asher]

when displaying hebrew text which contains parentheses on android, the direction of the parentheses is reversed. the opening parentheses looks like a closing one, and vice versa. kind of like )text in parentheses(. we had a bug like this years ago on the x86 platform.

Comment 1

[Tomer Cohen :tomer]

Hi Tsahi. 

Thanks for the bug report. Can you please add a simple testcase and a screenshot of the issue? 

Thanks.

Comment 2

[Jonathan Kew [:jfkthame]]

This can be seen, for example, in the second paragraph of http://www.unicode.org/standard/translations/hebrew.html.

The problem is that Hebrew requires "mirroring" of characters such as parentheses, but the simple gfxFT2Fonts backend does not have any support for this.

The simplest solution will be to turn on Harfbuzz rendering for Hebrew on Android.

Comment 3

[Jonathan Kew [:jfkthame]]

Attachment: patch, enable harfbuzz on android for hebrew and thai

This sets the harfbuzz script flags to include Hebrew. This provides the mirroring support needed for Hebrew.

While we're here, it also enables HB for Thai (like we do on OS X). As we don't have an alternative platform backend that might do a nicer job of Thai, there's no reason to hold off on sending it through HB as well.

Comment 4

[Tomer Cohen :tomer]

(In reply to Jonathan Kew (:jfkthame) from comment #3)
> This provides the mirroring support needed for Hebrew.

I am not an expert, but doesn't we need to do the same for Farsi and Arabic as well? (Ehsan?)

Comment 5

[Anas Husseini [:linostar]]

(In reply to Tomer Cohen :tomer from comment #4)
> I am not an expert, but doesn't we need to do the same for Farsi and Arabic
> as well? (Ehsan?)

Sure. It's the same case for Arabic and Persian.

Comment 6

[Tomer Cohen :tomer]

Okay, my mistake. I see that in the patch the code 71 include all these languages.

Comment 7

[Jonathan Kew [:jfkthame]]

Attachment: reftest for parens within hebrew

Comment 8

[Jonathan Kew [:jfkthame]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/658986bab59d (patch)
https://hg.mozilla.org/integration/mozilla-inbound/rev/a256a4174461 (reftest)

Comment 9

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/658986bab59d
https://hg.mozilla.org/mozilla-central/rev/a256a4174461

Comment 10

[Jonathan Kew [:jfkthame]]

This caused some reftests to "unexpectedly" pass on Android:

REFTEST TEST-UNEXPECTED-PASS | http://10.250.48.201:30064/tests/layout/reftests/bidi/bidi-004.html | image comparison (==)
REFTEST TEST-UNEXPECTED-PASS | http://10.250.48.201:30064/tests/layout/reftests/bidi/bidi-004-j.html | image comparison (==)
REFTEST TEST-UNEXPECTED-PASS | http://10.250.48.201:30064/tests/layout/reftests/bidi/bidi-006.html | image comparison (==)
REFTEST TEST-UNEXPECTED-PASS | http://10.250.48.201:30064/tests/layout/reftests/bidi/bidi-006-j.html | image comparison (==)
REFTEST TEST-UNEXPECTED-PASS | http://10.250.48.201:30064/tests/layout/reftests/bidi/386339.html | image comparison (==) 

As these are all tests that use Hebrew text, it's nice that they now pass. :) So I pushed an update to remove the fails-if(Android) annotations from the reftest manifest:

https://hg.mozilla.org/integration/mozilla-inbound/rev/c38b54d06778

Comment 11

[Ed Morley [:emorley]]

Follow-up: https://hg.mozilla.org/mozilla-central/rev/c38b54d06778