[webkit-reviews] review denied: [Bug 137330] RenderMathMLUnderOver adds spacing to the child operator indefinitely when resizing the window : [Attachment 239448] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Said Abou-Hallawa
<sabouhallawa at apple.com>'s request for review:
Bug 137330: RenderMathMLUnderOver adds spacing to the child operator
indefinitely when resizing the window
https://bugs.webkit.org/show_bug.cgi?id=137330

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

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=239448&action=review


review- because of the test failures, otherwise looks fine

> Source/WebCore/rendering/mathml/RenderMathMLOperator.h:69
> +    void resetStretchSize() { m_isVertical ? m_stretchHeightAboveBaseline =
m_stretchDepthBelowBaseline = 0 : m_stretchWidth = 0; }

I think we should start this as private. Seems like the body could be inside
the RenderMathMLOperator.cpp file, too. We could still mark it inline, but
don’t need it in the header.

Also, WebKit coding style explicitly frowns upon setting two variables with a
single chained assignment statement and implicitly frowns upon using the ?:
operator on an expression with side effects. I think we should just use a plain
old if statement for this.

> Source/WebCore/rendering/mathml/RenderMathMLOperator.h:77
> +    virtual void layout() override;

I suggest making this protected, or perhaps even private.