[webkit-reviews] review denied: [Bug 33964] Added RenderMathMLBlock as based object for MathML : [Attachment 47135] RenderMathMLBlock Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Jan 21 14:25:23 PST 2010


Dave Hyatt <hyatt at apple.com> has denied Alex Milowski <alex at milowski.com>'s
request for review:
Bug 33964: Added RenderMathMLBlock as based object for MathML
https://bugs.webkit.org/show_bug.cgi?id=33964

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

------- Additional Comments from Dave Hyatt <hyatt at apple.com>
In RenderMathMLBlock::styleDidChange, why are you marking the block as needing
layout and pref width recalc always?  That's really inefficient. I'm also not
clear on wny it's necessary.  Isn't the style diff doing the right thing in the
base classes?

In isChildAllowed you don't have to do ? true : false.	Just return the
conditional itself.

stretchToHeight would be cleaner using a for loop to walk the children in my
opinion.

for (RenderObject* current = firstChild(); current; current =
current->nextSibling());

It seems like makeBlockStyle should return a PassRefPtr, since you're wanting
to have it be set on some RenderObject presumably.


More information about the webkit-reviews mailing list