Bug 159513

Summary: MathOperator: Add a mapping from combining to non-combining equivalents
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: alex, bfulgham, commit-queue, dbarton, distler, esprehn+autocc, glenn, kondapallykalyan, rego
Priority: P2    
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 161189    
Attachments:
Description Flags
Patch (WIP)
none
Patch darin: review+

Frédéric Wang (:fredw)
Reported 2016-07-07 09:07:19 PDT
Many math fonts provide stretch variants and assemblies for combining characters instead of their non-combining equivalent contrary to what MathML says (https://www.w3.org/TR/MathML3/chapter7.html#chars.comb-chars). For example ̲ U+0332 COMBINING LOW LINE instead of _ U+005F LOW LINE. We should have mapping from combining to non-combining forms in order to be able to use the corresponding variant and assemblies from these math fonts.
Attachments
Patch (WIP) (5.38 KB, patch)
2016-07-22 07:53 PDT, Frédéric Wang (:fredw)
no flags
Patch (10.94 KB, patch)
2016-07-25 06:32 PDT, Frédéric Wang (:fredw)
darin: review+
Frédéric Wang (:fredw)
Comment 1 2016-07-22 00:01:09 PDT
Jacques Distler reported the same issue for ¯ U+00AF MACRON which does not have stretchy variants/assembly in Latin Modern Math. That's also true for U+203E OVERLINE or U+0304 COMBINING MACRON. However, COMBINING OVERLINE U+0305 has the desired strechy data. I think all these overbars should try and find an equivalent constructions in the similar characters. Probably the issue does not happen with Gecko because it relies on its Unicode fallback.
Frédéric Wang (:fredw)
Comment 2 2016-07-22 07:53:00 PDT
Created attachment 284331 [details] Patch (WIP) Here is a WIP patch (missing a test).
Frédéric Wang (:fredw)
Comment 3 2016-07-25 06:32:13 PDT
Darin Adler
Comment 4 2016-07-25 09:44:12 PDT
Comment on attachment 284480 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=284480&action=review > Source/WebCore/rendering/mathml/MathOperator.h:79 > + bool getBaseGlyph(const RenderStyle& style, GlyphData& baseGlyph) const > + { > + return getGlyph(style, m_baseCharacter, baseGlyph); > + } Including function bodies that make the function declaration take multiple lines in the class definition typically ends up messing it up over time. I prefer the idiom where we put the inline function body outside the class definition once it’s going to take multiple lines like this.
Darin Adler
Comment 5 2016-07-25 09:44:36 PDT
Comment on attachment 284480 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=284480&action=review > Source/WebCore/rendering/mathml/MathOperator.cpp:180 > + { 0x005E, 0x0302, 0 }, // CIRCUMFLEX ACCENT > + { 0x005F, 0x0332, 0 }, // LOW LINE > + { 0x007E, 0x0303, 0 }, // TILDE > + { 0x00AF, 0x0304, 0x0305 }, // MACRON > + { 0x02C6, 0x0302, 0 }, // MODIFIER LETTER CIRCUMFLEX ACCENT > + { 0x02C7, 0x030C, 0 } // CARON test should cover all 7 characters, but seems to only cover one of them
Frédéric Wang (:fredw)
Comment 6 2016-07-25 09:59:25 PDT
(In reply to comment #5) > Comment on attachment 284480 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=284480&action=review > > > Source/WebCore/rendering/mathml/MathOperator.cpp:180 > > + { 0x005E, 0x0302, 0 }, // CIRCUMFLEX ACCENT > > + { 0x005F, 0x0332, 0 }, // LOW LINE > > + { 0x007E, 0x0303, 0 }, // TILDE > > + { 0x00AF, 0x0304, 0x0305 }, // MACRON > > + { 0x02C6, 0x0302, 0 }, // MODIFIER LETTER CIRCUMFLEX ACCENT > > + { 0x02C7, 0x030C, 0 } // CARON > > test should cover all 7 characters, but seems to only cover one of them There seems to be a bug with the width of horizontal stretchy operators that made writing tests based on box measuring or on dumping the render tree difficult and I did not want to have such mismatch test for each of the character. (additionally, I also currently have issues getting screenshots with the test runner).
Frédéric Wang (:fredw)
Comment 7 2016-07-26 00:48:10 PDT
Frédéric Wang (:fredw)
Comment 8 2016-07-26 00:49:18 PDT
(In reply to comment #5) > test should cover all 7 characters, but seems to only cover one of them I converted the mismatch test to a pixel test, but the PNG expectations will have to be regenerated later as I can't do it myself at the moment.
Note You need to log in before you can comment on or make changes to this bug.