[Webkit-unassigned] [Bug 118053] Parsing of MathML length

[bugzilla-daemon at webkit.org]

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





--- Comment #6 from chris fleizach <cfleizach at apple.com>  2013-06-26 14:04:51 PST ---
(In reply to comment #4)
> (In reply to comment #2)
> > it looks like ParseNamedSpace is only used inside ParseLength… that method should be static inside the class file
> 
> As indicated in bug 115610 comment 5, I've made two separated functions because of mpadded attribute parsing which require calling the two functions plus doing some other stuff with pseudo-units. However, this is not implemented in WebKit yet, so I can make ParseNamedSpace static for the moment.

That's fine then to leave as is

> 
> > 
> > > Source/WebCore/rendering/mathml/RenderMathMLFraction.cpp:82
> > > +        ParseLength(thickness, m_lineThickness, false);
> > 
> > you might want to check if the conversion succeeded before setting the value
> 
> Not sure what you mean here... m_lineThickness = gLineMedium sets m_lineThickness to the default value. If ParseLength fails, then the attribute is ignored and the default value is used. If ParseLength succeeds, then m_lineThickness is set to the parsed value (perhaps a multiple of the specified default for % and unitless). So the boolean success result is not really used here, but I think it may be useful in other situations.

Yes I see that. the question is whether the function should return the result  or return success.

I think it could go either way, but I see that other methods return the result instead of success.

As to not checking the result value -- it obviously works, but it's possible someone in the future might set the value to zero on a failure and this might fail, so if the method returns a result you should probably check it for completeness

> 
> I'd like to have clarification on what you expect exactly for the tests and about this last point before submitting a new version.

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