Bug 115786

Summary: Invisible Operators should not add space
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: bfulgham, cfleizach, commit-queue, dbarton, esprehn+autocc, glenn, gyuyoung.kim, kondapallykalyan, macpherson, menard, mrobinson
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
URL: http://www.w3.org/TR/MathML/chapter3.html#presm.invisibleops
Bug Depends on: 99620, 115787, 124838    
Bug Blocks: 84019, 115789, 122297, 128907    
Attachments:
Description Flags
testcase
none
Patch
none
Patch
none
Patch + 115787 for testing
none
Patch cfleizach: review+

Description Frédéric Wang (:fredw) 2013-05-08 02:49:50 PDT
Created attachment 201047 [details]
testcase

In the attached test case (from the MathML spec), you see that invisible operators add space to the mathematical formulas. However, they should have width=0 and per the Operator dictionary (http://www.w3.org/TR/MathML/appendixc.html) no space around them.
Comment 1 Frédéric Wang (:fredw) 2013-05-15 05:43:28 PDT
A Chromium bug was reported last January:
http://code.google.com/p/chromium/issues/detail?id=169753
Comment 2 Frédéric Wang (:fredw) 2014-02-26 01:46:22 PST
Created attachment 225243 [details]
Patch
Comment 3 Frédéric Wang (:fredw) 2014-03-06 08:55:43 PST
Created attachment 225996 [details]
Patch

Just refreshing the patch.
Comment 4 chris fleizach 2014-03-06 10:28:11 PST
Comment on attachment 225996 [details]
Patch

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

is this ready for r?

> Source/WebCore/rendering/mathml/RenderMathMLOperator.h:91
> +    bool isInvisibleOperator() { return 0x2061 <= m_operator && m_operator <= 0x2064; }

this can be const
Comment 5 Frédéric Wang (:fredw) 2014-03-06 11:28:55 PST
(In reply to comment #4)
> (From update of attachment 225996 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=225996&action=review
> 
> is this ready for r?

I don't think the patch will change much, but this applies on top of the patch of bug 115787 which itself applies on top of bug 124838... Did you find anything about how to make the accessibility test pass after the patch for bug 124838?
Comment 6 Frédéric Wang (:fredw) 2014-03-11 02:28:51 PDT
Created attachment 226402 [details]
Patch + 115787 for testing
Comment 7 Frédéric Wang (:fredw) 2014-03-11 03:22:21 PDT
Created attachment 226411 [details]
Patch

This applies on top of 115787.
Comment 8 chris fleizach 2014-03-11 09:19:42 PDT
Comment on attachment 226411 [details]
Patch

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

> Source/WebCore/rendering/mathml/RenderMathMLOperator.cpp:1306
> +            // In some fonts, glyphs for invisible operators have nonzero width. Consequently, we substract that width here to avoid wide gaps.

substract -> subtract
Comment 9 Frédéric Wang (:fredw) 2014-03-12 02:50:37 PDT
Committed r165464: <http://trac.webkit.org/changeset/165464>
Comment 10 Frédéric Wang (:fredw) 2014-03-12 03:26:04 PDT
Interestingly, <p>_&#x2062;_</p> does not produce space on Gecko but it does on WebKit. According to Khaled Hosny, HarfBuzz handles these characters specifically:

https://bugzilla.mozilla.org/show_bug.cgi?id=522393#c43