[webkit-reviews] review granted: [Bug 78180] MathML internals for bug 52444 fix - type checking, PassRefPtr : [Attachment 126195] Patch

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has granted Dave Barton
<dbarton at mathscribe.com>'s request for review:
Bug 78180: MathML internals for bug 52444 fix - type checking, PassRefPtr
https://bugs.webkit.org/show_bug.cgi?id=78180

Attachment 126195: Patch
https://bugs.webkit.org/attachment.cgi?id=126195&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=126195&action=review


LGTM.  Keep them coming.  This code needs lots of love.

> Source/WebCore/rendering/mathml/RenderMathMLFenced.cpp:83
> +PassRefPtr<RenderStyle> RenderMathMLFenced::makeOperatorStyle()

This should probably be renamed createOperatorStyle() (to follow the
CreateRule, which the ReftPtr (and Mac OS X style docs) talk about.  That
doesn't have to be today, or in this patch.

> Source/WebCore/rendering/mathml/RenderMathMLOperator.cpp:48
> -RenderMathMLOperator::RenderMathMLOperator(Node* container, UChar
operatorChar)
> -    : RenderMathMLBlock(container)
> +RenderMathMLOperator::RenderMathMLOperator(Node* node, UChar operatorChar)

Odd that these took nodes to begin with, since it would never make sense for a
Comment or Document node to be involved in a MathML rendering tree.  (Nodes
can't even have style, so they couldn't be styled to look like mathtml..)