[webkit-reviews] review granted: [Bug 62098] Update layout for msub, msup, msubsup to handle script changes : [Attachment 96035] New layout code that handles child replacement w/ fixed change log

[bugzilla-daemon at webkit.org]

Kenneth Rohde Christiansen <kenneth at webkit.org> has granted Alex Milowski
<alex at milowski.com>'s request for review:
Bug 62098: Update layout for msub, msup, msubsup to handle script changes
https://bugs.webkit.org/show_bug.cgi?id=62098

Attachment 96035: New layout code that handles child replacement w/ fixed
change log
https://bugs.webkit.org/attachment.cgi?id=96035&action=review

------- Additional Comments from Kenneth Rohde Christiansen
<kenneth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=96035&action=review

> Source/WebCore/rendering/mathml/RenderMathMLSubSup.cpp:66
> +    int position = 0;

unsigned?

> Source/WebCore/rendering/mathml/RenderMathMLSubSup.cpp:68
> +	   if (current->nodeType() == Node::ELEMENT_NODE)

Isnt there an isElementNode() ? im pretty sure that exists

> Source/WebCore/rendering/mathml/RenderMathMLSubSup.cpp:176
> -	       int heightDiff = m_scripts ? (m_scripts->offsetHeight() -
maxHeight) / 2 : 0;
> +	       int heightDiff = m_scripts ?
> +		   (m_scripts->offsetHeight() - maxHeight) / 2 
> +		   : 0;

Im not sure what our style guide says, but I havent run into this style before.
I would just keep it on one line