[webkit-reviews] review denied: [Bug 111379] [CSS Exclusions][CSS Regions] Shape-inside on a parent element does not affect region content : [Attachment 192033] proposed patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Mar 11 09:57:58 PDT 2013


Dave Hyatt <hyatt at apple.com> has denied Zoltan Horvath <zoltan at webkit.org>'s
request for review:
Bug 111379: [CSS Exclusions][CSS Regions] Shape-inside on a parent element does
not affect region content
https://bugs.webkit.org/show_bug.cgi?id=111379

Attachment 192033: proposed patch
https://bugs.webkit.org/attachment.cgi?id=192033&action=review

------- Additional Comments from Dave Hyatt <hyatt at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=192033&action=review


r-

> Source/WebCore/rendering/RenderBlockLineLayout.cpp:86
> +	   if (region) {

if (!region)
    return 0;

// Rest of code...

would read better. That way it's not so indented.

> Source/WebCore/rendering/RenderBlockLineLayout.cpp:95
> +	       RenderBlock* parent = toRenderBlock(region->parent());
> +	       while (parent) {
> +		   if (parent->exclusionShapeInsideInfo())
> +		       return parent->exclusionShapeInsideInfo();
> +		   parent = toRenderBlock(parent->parent());
> +	       }

This code is broken. You can have non-RenderBlock parents in the render tree.
I'm not sure exactly what you're trying to do here, so I'm not sure how to
suggest you patch it. Can exclusions intrude into anything, even out-of-flow
content? If so, then I guess you wanted containingBlock() instead of parent().
If not, then maybe you just need to go out until you establish a block
formatting context? I would think objects that avoid floats would avoid
exclusions also, so it seems like there needs to be some kind of stopping point
here.


More information about the webkit-reviews mailing list