[webkit-reviews] review denied: [Bug 28064] wrong calculation of overflow size for flexbox and table : [Attachment 34258] Patch v1
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Aug 7 08:40:40 PDT 2009
Eric Seidel <eric at webkit.org> has denied Shinichiro Hamaji
<hamaji at chromium.org>'s request for review:
Bug 28064: wrong calculation of overflow size for flexbox and table
https://bugs.webkit.org/show_bug.cgi?id=28064
Attachment 34258: Patch v1
https://bugs.webkit.org/attachment.cgi?id=34258&action=review
------- Additional Comments from Eric Seidel <eric at webkit.org>
It is not immediately apparent to me what was wrong in the table code. Please
explain in your ChangeLog.
I like how the line spacing was where you copied this from:
+ style()->getBoxShadowExtent(shadowTop, shadowRight, shadowBottom,
shadowLeft);
+ m_overflowLeft = min(m_overflowLeft, shadowLeft);
I would prefer there was a line between the shadow lookups and the first
m_overflowLeft setting.
r- because your ChangeLog isn't clear enough (or my eyes aren't good enough) to
understand easily what this does besides moving code.
This change looks GREAT! But I need to know (and future people looking at your
patch need to know) what is changed for Table/Flexbox. A simple pointer in the
ChangeLog to what the error in the old code was would suffice.
Thanks for doing this! Such a fantastic cleanup. :)
More information about the webkit-reviews
mailing list