Bug 97228 - [MathML] Increase visual space around fraction parts, italic variables, and operators
Summary: [MathML] Increase visual space around fraction parts, italic variables, and o...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: MathML (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dave Barton
URL:
Keywords:
Depends on: 97293
Blocks:
  Show dependency treegraph
 
Reported: 2012-09-20 09:59 PDT by Dave Barton
Modified: 2012-10-17 12:09 PDT (History)
12 users (show)

See Also:


Attachments
Patch (874.97 KB, patch)
2012-09-20 10:12 PDT, Dave Barton
no flags Details | Formatted Diff | Diff
Patch (875.19 KB, patch)
2012-09-20 11:07 PDT, Dave Barton
eric: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dave Barton 2012-09-20 09:59:37 PDT
[MathML] Increase visual space around fraction parts, italic variables, and operators
Comment 1 Dave Barton 2012-09-20 10:12:35 PDT
Created attachment 164937 [details]
Patch
Comment 2 Eric Seidel (no email) 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.
Comment 3 Dave Barton 2012-09-20 11:07:56 PDT
Created attachment 164949 [details]
Patch
Comment 4 Dave Barton 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. :)
Comment 5 Eric Seidel (no email) 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. :)
Comment 6 Dave Barton 2012-09-20 11:26:44 PDT
Committed r129146: <http://trac.webkit.org/changeset/129146>
Comment 7 Chris Dumez 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.
Comment 8 Dave Barton 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.
Comment 9 spartha80 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?
Comment 10 Dave Barton 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).