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+

Frédéric Wang (:fredw)
Reported 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.
Attachments
Patch (3.11 KB, patch)
2014-03-27 08:08 PDT, Frédéric Wang (:fredw) 		no flags
DetailsFormatted DiffDiff
Patch (8.75 KB, patch)
2014-04-08 07:04 PDT, Frédéric Wang (:fredw) 		no flags
DetailsFormatted DiffDiff
Patch+130322 for testing (846.78 KB, patch)
2014-04-08 07:06 PDT, Frédéric Wang (:fredw) 		no flags
DetailsFormatted DiffDiff
Patch (9.26 KB, patch)
2014-06-05 01:25 PDT, Frédéric Wang (:fredw) 		cfleizach: review+
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Frédéric Wang (:fredw)
Comment 1 2014-03-27 08:08:34 PDT
Created attachment 227946 [details] Patch
Frédéric Wang (:fredw)
Comment 2 2014-04-08 07:04:37 PDT
Created attachment 228830 [details] Patch
Frédéric Wang (:fredw)
Comment 3 2014-04-08 07:06:14 PDT
Created attachment 228831 [details] Patch+130322 for testing
Frédéric Wang (:fredw)
Comment 4 2014-06-05 01:25:55 PDT
Created attachment 232528 [details] Patch
Frédéric Wang (:fredw)
Comment 5 2014-06-05 09:23:43 PDT
Committed r169617: <http://trac.webkit.org/changeset/169617>
Chris Dumez
Comment 6 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.
Frédéric Wang (:fredw)
Comment 7 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.
Note You need to log in before you can comment on or make changes to this bug.