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

Wed Jun 19 14:02:18 PDT 2013

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

chris fleizach <cfleizach at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #203758|review?                     |review-
               Flag|                            |

--- Comment #43 from chris fleizach <cfleizach at apple.com>  2013-06-19 14:00:55 PST ---
(From update of attachment 203758)
View in context: https://bugs.webkit.org/attachment.cgi?id=203758&action=review

You should separate the value parsing from the MathMLSpace patch into two patch I think, so you can address the line thickness issue separately.

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

> 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.

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?

> 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

> 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

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