[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