Bug 130839

Summary: Apply Character-level mirroring to stretchy operators in RTL mode
Product: WebKit Reporter: Frédéric Wang (:fredw) <fred.wang>
Component: MathMLAssignee: Frédéric Wang (:fredw) <fred.wang>
Status: RESOLVED FIXED    
Severity: Normal CC: bunhere, cdumez, cfleizach, commit-queue, dbarton, esprehn+autocc, glenn, gyuyoung.kim, kondapallykalyan, macpherson, menard, mrobinson, sergio
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 130322    
Bug Blocks: 123018    
Attachments:
Description Flags
Patch
none
Patch
none
Patch+130322 for testing
none
Patch cfleizach: review+

Description Frédéric Wang (:fredw) 2014-03-27 08:07:36 PDT
The Unicode spec define some mirroring per character:

http://www.unicode.org/Public/UCD/latest/ucd/BidiMirroring.txt

The attached patch (which applies on top of bug 130322) does the necessary changes to mirror these characters for stretchy Operators drawn with the MATH table. This is just using the appropriate "mirror" boolean when calling glyphDataForCharacter on the base glyph.

It's not really possible to do the same for the old code since for example the parenthesis pieces are not defined as mirrorable in the Unicode spec. However, there are not many fences in the stretchyCharacters table, so it is certainly possible to add some ad hoc code to mirror the base character. But perhaps it's not worth it if we want to move to MATH fonts.

For other operators like sums or integrals, we will need "Glyph-level mirroring" as well as fonts such as XITS that contain the mirrored glyphs:

http://www.microsoft.com/typography/OTSPEC/TTOCHAP1.htm#ltrrtl
https://github.com/khaledhosny/xits-math

However, I'm not sure the 'rtlm' feature is implemented in WebKit so I'll open a separate bug for that.
Comment 1 Frédéric Wang (:fredw) 2014-03-27 08:08:34 PDT
Created attachment 227946 [details]
Patch
Comment 2 Frédéric Wang (:fredw) 2014-04-08 07:04:37 PDT
Created attachment 228830 [details]
Patch
Comment 3 Frédéric Wang (:fredw) 2014-04-08 07:06:14 PDT
Created attachment 228831 [details]
Patch+130322 for testing
Comment 4 Frédéric Wang (:fredw) 2014-06-05 01:25:55 PDT
Created attachment 232528 [details]
Patch
Comment 5 Frédéric Wang (:fredw) 2014-06-05 09:23:43 PDT
Committed r169617: <http://trac.webkit.org/changeset/169617>
Comment 6 Chris Dumez 2014-10-31 10:59:00 PDT
Comment on attachment 232528 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=232528&action=review

> Source/WebCore/rendering/mathml/RenderMathMLOperator.cpp:1587
> +                    index += index % 2 ? -1 : 1;

There is likely a bug here. The new value assigned to index is never used because of the break below. Frederic, could you please check? I am not sure what was the intent because the chunk added here has no effect.
Comment 7 Frédéric Wang (:fredw) 2014-10-31 11:04:57 PDT
(In reply to comment #6)
> Comment on attachment 232528 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=232528&action=review
> 
> > Source/WebCore/rendering/mathml/RenderMathMLOperator.cpp:1587
> > +                    index += index % 2 ? -1 : 1;
> 
> There is likely a bug here. The new value assigned to index is never used
> because of the break below. Frederic, could you please check? I am not sure
> what was the intent because the chunk added here has no effect.

I think the index change should have been done before the

stretchyCharacter = &stretchyCharacters[index];

statement.