[webkit-reviews] review granted: [Bug 46421] Make CSS3 Multicol layout work with vertical text. : [Attachment 80215] Patch

[bugzilla-daemon at webkit.org]

mitz at webkit.org has granted Dave Hyatt <hyatt at apple.com>'s request for review:
Bug 46421: Make CSS3 Multicol layout work with vertical text.
https://bugs.webkit.org/show_bug.cgi?id=46421

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

------- Additional Comments from mitz at webkit.org
View in context: https://bugs.webkit.org/attachment.cgi?id=80215&action=review

> Source/WebCore/rendering/InlineTextBox.h:84
> +    virtual IntRect calculateBoundaries() const { return IntRect(x(), y(),
width(), height()); }

:)

> Source/WebCore/rendering/RenderBlock.cpp:1327
> +		   int overflowRight = style()->isLeftToRightDirection() ?
max(width(), lastRect.x() + lastRect.width()) : 0;

Can say lastRect.right() instead of x() + width().

> Source/WebCore/rendering/RenderBlock.cpp:1331
> +		   IntRect lastRect = columnRectAt(colInfo,
columnCount(colInfo) - 1);

You’ve already defined and initialized this outside.

> Source/WebCore/rendering/RenderBlock.cpp:1333
> +		   int overflowBottom = style()->isLeftToRightDirection() ?
max(height(), lastRect.y() + lastRect.height()) : 0;

Can say lastRect.bottom() instead of y() + height().

> Source/WebCore/rendering/RenderBlock.cpp:2278
> +    IntRect firstColRect = columnRectAt(colInfo, 0);

I don’t think this is used.

> Source/WebCore/rendering/RenderInline.cpp:752
> +    

ZOMG whitespace