[webkit-reviews] review granted: [Bug 110349] [css exclusions] overflow:hidden undoes shape-outside offsets : [Attachment 195798] Patch

Mon Apr 1 14:54:01 PDT 2013

Julien Chaffraix <jchaffraix at webkit.org> has granted Bem Jones-Bey
<bjonesbe at adobe.com>'s request for review:
Bug 110349: [css exclusions] overflow:hidden undoes shape-outside offsets
https://bugs.webkit.org/show_bug.cgi?id=110349

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

------- Additional Comments from Julien Chaffraix <jchaffraix at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=195798&action=review

> Source/WebCore/rendering/RenderLayer.cpp:5850
> +	       && !(renderer()->style()->shapeOutside() &&
renderer()->isFloating());

RenderObject::isFloating is probably cheaper to evaluate so you would be better
off changing the order.

This starts to be a pattern in the code where you check
renderer()->style()->shapeOutside() && renderer()->isFloating() so it should be
refactored into its own helper method.

>
LayoutTests/fast/exclusions/shape-outside-floats/shape-outside-floats-overflow-
hidden-expected.html:1
> +<html>

No doctype!

>
LayoutTests/fast/exclusions/shape-outside-floats/shape-outside-floats-overflow-
hidden-expected.html:8
> +  background-color: red;

Let's not put any red in the expected result as it would make the test pass,
even if you are saying we shouldn't see any red.

>
LayoutTests/fast/exclusions/shape-outside-floats/shape-outside-floats-overflow-
hidden.html:1
> +<html>

Same comment.