RESOLVED FIXED 97228
[MathML] Increase visual space around fraction parts, italic variables, and operators
https://bugs.webkit.org/show_bug.cgi?id=97228
Summary [MathML] Increase visual space around fraction parts, italic variables, and o...
Dave Barton
Reported 2012-09-20 09:59:37 PDT
[MathML] Increase visual space around fraction parts, italic variables, and operators
Attachments
Patch (874.97 KB, patch)
2012-09-20 10:12 PDT, Dave Barton
no flags
Patch (875.19 KB, patch)
2012-09-20 11:07 PDT, Dave Barton
eric: review+
Dave Barton
Comment 1 2012-09-20 10:12:35 PDT
Eric Seidel (no email)
Comment 2 2012-09-20 10:16:57 PDT
Comment on attachment 164937 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=164937&action=review Otherwise looks fine to me. I don't know if you want your MathML peeps to sign off or not. I'm not much help in evaluating the beauty of the Math. I'm ready to r+ with teh enum. > Source/WebCore/rendering/mathml/RenderMathMLFenced.h:44 > + RenderMathMLOperator* createMathMLOperator(UChar, bool isFence); Much better to use an enum here. Makes the callsites clearer. enum OperatorType { FenceOperator, NormalOperator? } createMathMLOperator can even have OperatorType = NormalOperator so most callsites don't have to pass anything if you like.
Dave Barton
Comment 3 2012-09-20 11:07:56 PDT
Dave Barton
Comment 4 2012-09-20 11:12:57 PDT
I added the enum. I cc'd David C. & Fred on the bug, but they're in Europe & may not see this soon. I don't think it needs their expert MathML review before landing. I'm just adding some space, making our layout look more similar to Firefox and TeX. I think we can't ask them to look at every tweak I make. We'll get plenty of reactions to layout issues from real users soon. :)
Eric Seidel (no email)
Comment 5 2012-09-20 11:17:01 PDT
Comment on attachment 164949 [details] Patch LGTM. I agree, we don't need to consult the mathml council for every tweak. :)
Dave Barton
Comment 6 2012-09-20 11:26:44 PDT
Chris Dumez
Comment 7 2012-09-20 22:56:20 PDT
I think it would it would be nice in the future to move the tests to TestExpectations with a comment like "Needs rebaseline after rxxxx" for ports you don't rebaseline. That would avoid making our bots red until we do the rebaseline. Thanks in advance.
Dave Barton
Comment 8 2012-09-20 23:49:59 PDT
(In reply to comment #7) > I think it would it would be nice in the future to move the tests to TestExpectations with a comment like "Needs rebaseline after rxxxx" for ports you don't rebaseline. That would avoid making our bots red until we do the rebaseline. Thanks in advance. Good idea, I will do this.
spartha80
Comment 9 2012-10-16 03:37:36 PDT
From what I have checked, Firefox refers to an operator dictionary for Operator spacing. Are there any plans for Webkit to use something similar?
Dave Barton
Comment 10 2012-10-17 12:09:11 PDT
(In reply to comment #9) > From what I have checked, Firefox refers to an operator dictionary for Operator spacing. Are there any plans for Webkit to use something similar? Yes! See bug 99620 (or bug 99623).
Note You need to log in before you can comment on or make changes to this bug.