[webkit-reviews] review requested: [Bug 33266] WebCore::InlineFlowBox::determineSpacingForFlowBoxes ReadAV at NULL (43c64e8abbda6766e5f5edbd254c2d57) : [Attachment 46898] patch - addressing the review remarks

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Jan 19 02:29:06 PST 2010


Roland Steiner <rolandsteiner at chromium.org> has asked  for review:
Bug 33266: WebCore::InlineFlowBox::determineSpacingForFlowBoxes ReadAV at NULL
(43c64e8abbda6766e5f5edbd254c2d57)
https://bugs.webkit.org/show_bug.cgi?id=33266

Attachment 46898: patch - addressing the review remarks
https://bugs.webkit.org/attachment.cgi?id=46898&action=review

------- Additional Comments from Roland Steiner <rolandsteiner at chromium.org>
Changed the patch as requested, in particular:

-) Changed moveAllChildrenTo() to use original coding with 2 variables. I don't
think there'd be any speed difference, but then again safe is safe... ;)

-) Renamed methods as suggested, refactored moveInlineChildren() as suggested

-) Changed method signatures to use RenderRubyBase* instead of the more generic
RenderBlock*. This should also address the question about
createsAnonymousWrapper (?).

-) about braces around a single line clause: ISTR there was a discussion on
webkit-dev@ that that's ok if it's consistent between the if and the else
clause (?). But I'm not religious about such things, so I removed them anyway.


More information about the webkit-reviews mailing list