[Webkit-unassigned] [Bug 115610] Add Support for mspace element

Wed Jun 19 14:23:05 PDT 2013

https://bugs.webkit.org/show_bug.cgi?id=115610

--- Comment #44 from Frédéric Wang <fred.wang at free.fr>  2013-06-19 14:21:42 PST ---
(In reply to comment #43)
> You should separate the value parsing from the MathMLSpace patch into two patch I think, so you can address the line thickness issue separately.

Yes that's probably what I'll do once MathJax 2.3 (hopefully this summer) is released and I can work on WebKit MathML again. Thanks for the review!

> 
> Thanks
> 
> > LayoutTests/mathml/presentation/mfrac-linethickness-expected-mismatch.html:4
> > +    <title>mfrac linethickness</title>
> 
> what is this testing? there's nothing related to mfrac in here

This is a != reftest as indicated by the "expected-mismatch" suffix. The explanation is in the corresponding test LayoutTests/mathml/presentation/mfrac-linethickness.html

> 
> > LayoutTests/mathml/presentation/mspace-expected.html:1
> > +<!DOCTYPE html>
> 
> Your layout tests probably shouldn't be suffixed with -expected, because that's the suffix for the expected results.

Not sure what you mean... That's a == reftest to compare with LayoutTests/mathml/presentation/mspace.html so the -expected suffix is necessary?

> 
> I also don't see any results from running these layout tests...
> 
> > Source/WebCore/WebCore.vcxproj/WebCore.vcxproj:-1
> > -ï»¿<?xml version="1.0" encoding="utf-8"?>
> 
> inccorrect addition here?

Ooops, I thought I had fixed that one.

> 
> > Source/WebCore/rendering/mathml/RenderMathMLBlock.cpp:317
> > +    return false;
> 
> It seems a shame this is specific to MathML and not re-usable. I see code in CSSPrimitiveValue that handles some of these cases it appears

That's what I asked above but couldn't find these CSS primitive alone. However some work is needed to parse the MathML-specific stuff like the unitless case you implemented, so a separate function is necessary anyway. Note that using a MathML-specific routine is also what Gecko does.

> 
> > Source/WebCore/rendering/mathml/RenderMathMLBlock.cpp:322
> > +    int i = 0;
> 
> you should choose a better name for this variable, plus it looks like it should be a float anyway

True. (I just copied that from the Gecko code...)

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.