[Webkit-unassigned] [Bug 81387] MathML internals - factor code for almost anonymous blocks

[bugzilla-daemon at webkit.org]

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





--- Comment #5 from Julien Chaffraix <jchaffraix at webkit.org>  2012-03-16 14:30:14 PST ---
(From update of attachment 132346)
View in context: https://bugs.webkit.org/attachment.cgi?id=132346&action=review

A couple of comments too (mostly consistency questions). Not worth holding the change though.

>> Source/WebCore/rendering/mathml/RenderMathMLBlock.cpp:52
>> +RenderMathMLBlock* RenderMathMLBlock::createAlmostAnonymousBlock(EDisplay display)
> 
> Normally these sorts of functions would return PassOwnPtr, but the rendering tree code isn't very consistent about that, sadly.

Unfortunately indeed. Shouldn't we name this function createAlmostAnonymousMathMLBlock? (at some point you will want to create anonymous RenderMathMLBlock not RenderBlock or am I missing something?)

> Source/WebCore/rendering/mathml/RenderMathMLFraction.cpp:106
> +    Length pad(static_cast<int>(style()->fontSize() * gHorizontalPad), Fixed);

I guess this is equivalent here but it used to be row->style()->fontSize().

> Source/WebCore/rendering/mathml/RenderMathMLSubSup.cpp:87
> +            RefPtr<RenderStyle> scriptsStyle = RenderStyle::createAnonymousStyle(style());
>              scriptsStyle->setDisplay(INLINE_BLOCK);
>              scriptsStyle->setVerticalAlign(TOP);
>              scriptsStyle->setMarginLeft(Length(gSubsupScriptMargin, Fixed));
>              scriptsStyle->setTextAlign(LEFT);
>              // Set this wrapper's font-size for its line-height & baseline position.
>              scriptsStyle->setBlendedFontSize(static_cast<int>(0.75 * style()->fontSize()));
> -            m_scripts->setStyle(scriptsStyle.release());
> +            m_scripts = new (renderArena()) RenderMathMLBlock(node());
> +            m_scripts->setStyle(scriptsStyle);

Shouldn't m_scripts also use createAlmostAnonymousBlock() or would you rather wait to do that?

> Source/WebCore/rendering/mathml/RenderMathMLUnderOver.cpp:69
> +    RenderBlock* row = createAnonymousBlock();

Shouldn't we add a new createAnonymousMathMLBlock() as you would rather keep it a MathML block if you want to add more logic into them?

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