Last Comment Bug 689736 - parentheses are flipped in RTL (e.g. Hebrew) text
: parentheses are flipped in RTL (e.g. Hebrew) text
Status: RESOLVED FIXED
: rtl
Product: Core
Classification: Components
Component: Layout: Text (show other bugs)
: Trunk
: ARM Android
: -- normal (vote)
: mozilla10
Assigned To: Jonathan Kew (:jfkthame)
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-09-27 14:00 PDT by Tsahi Asher
Modified: 2011-10-07 04:57 PDT (History)
7 users (show)
emorley: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch, enable harfbuzz on android for hebrew and thai (1021 bytes, patch)
2011-09-30 01:59 PDT, Jonathan Kew (:jfkthame)
smontagu: review+
Details | Diff | Splinter Review
reftest for parens within hebrew (2.03 KB, patch)
2011-09-30 07:16 PDT, Jonathan Kew (:jfkthame)
smontagu: review+
Details | Diff | Splinter Review

Description Tsahi Asher 2011-09-27 14:00:51 PDT
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 2011-09-27 14:04:37 PDT
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) 2011-09-30 01:55:11 PDT
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) 2011-09-30 01:59:11 PDT
Created attachment 563694 [details] [diff] [review]
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 2011-09-30 02:21:34 PDT
(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] 2011-09-30 02:24:43 PDT
(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 2011-09-30 02:32:57 PDT
Okay, my mistake. I see that in the patch the code 71 include all these languages.
Comment 7 Jonathan Kew (:jfkthame) 2011-09-30 07:16:00 PDT
Created attachment 563733 [details] [diff] [review]
reftest for parens within hebrew
Comment 10 Jonathan Kew (:jfkthame) 2011-10-06 09:15:00 PDT
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] 2011-10-07 04:57:17 PDT
Follow-up: https://hg.mozilla.org/mozilla-central/rev/c38b54d06778

Note You need to log in before you can comment on or make changes to this bug.