[Webkit-unassigned] [Bug 79063] MathML internals - code clean-up for RenderMathMLSubSup

[bugzilla-daemon at webkit.org]

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





--- Comment #4 from Dave Barton <dbarton at mathscribe.com>  2012-02-20 20:19:50 PST ---
(From update of attachment 127890)
View in context: https://bugs.webkit.org/attachment.cgi?id=127890&action=review

I appreciate your review & judgment and will do any changes you still want. In the next patch though this stretchToHeight() formatting code & the code in layout() will be combined and revised.

>> Source/WebCore/rendering/mathml/RenderMathMLSubSup.cpp:-149
>> -                }
> 
> We no longer need this update?

There's an old message from David Hyatt on webkit-dev saying that updateBoxModelInfoFromStyle() is an
internal method to styleWillChange()/styleDidChange() and should be considered private.
The supWrapper->setNeedsLayout(true) on new line 141 will cause these margins to be used, right?

>> Source/WebCore/rendering/mathml/RenderMathMLSubSup.cpp:151
>> +    if (m_kind == SubSup && m_scripts) {
> 
> It would be better to early return so we didn't have to indent the whole rest of this function?

I didn't want to rewrite too much. Also we could change someday to doing our own layout() of msub or msup also, instead
of using CSS vertical-align sub/super. That way we could get the baselines of msub or msup to be the same as the baselines
of subscripts & superscripts in <msubsup>s, maybe.

>> Source/WebCore/rendering/mathml/RenderMathMLSubSup.cpp:153
>> +            LayoutUnit heightDiff = (m_scripts->offsetHeight() - base->offsetHeight()) / 2;
> 
> heightDiff sounds like a strange name for this half-height diff?

This variable is going away in the next patch. I just left the name as is for now.

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