Bug 111379

Created attachment 192033 [details] proposed patch I turned the attached test case to different bugs: bug #111604 and bug #111607. This change fixes the bug. The patch file probably doesn't contain the previous test moving to the new directory, but I can add it when committing.

Attachment 192033 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast/regions/shape-inside-on-regions-expected.html', u'LayoutTests/fast/regions/shape-inside-on-regions.html', u'LayoutTests/fast/regions/shape-inside/shape-inside-on-parent-element-expected.html', u'LayoutTests/fast/regions/shape-inside/shape-inside-on-parent-element.html', u'LayoutTests/platform/chromium/TestExpectations', u'LayoutTests/platform/qt/TestExpectations', u'Source/WebCore/ChangeLog', u'Source/WebCore/rendering/RenderBlockLineLayout.cpp']" exit_code: 1 LayoutTests/platform/chromium/TestExpectations:4397: Path does not exist. [test/expectations] [5] LayoutTests/platform/qt/TestExpectations:2280: Path does not exist. [test/expectations] [5] Total errors found: 2 in 7 files If any of these errors are false positives, please file a bug against check-webkit-style.

Comment on attachment 192033 [details] proposed patch 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.

I modified the parent() to containingBlock(): Actually, I wanted to use while (containingBlock && containingBlock->flowThreadContainingBlock()) to run the loop only for the flow, but flowThreadContainingBlock() always returns 0, because their flowThreadState() is NotInsideFlowThread, though they are in a flow thread. Index: Source/WebCore/rendering/RenderBlockLineLayout.cpp =================================================================== --- Source/WebCore/rendering/RenderBlockLineLayout.cpp (revision 145230) +++ Source/WebCore/rendering/RenderBlockLineLayout.cpp (working copy) @@ -83,7 +83,17 @@ if (!shapeInsideInfo && flowThreadContainingBlock()) { LayoutUnit offset = logicalHeight() + logicalHeightForLine(this, false); RenderRegion* region = regionAtBlockOffset(offset); - return region ? region->exclusionShapeInsideInfo() : 0; + if (!region) + return 0; + if (region->exclusionShapeInsideInfo()) + return region->exclusionShapeInsideInfo(); + + RenderBlock* containingBlock = region->containingBlock(); + while (containingBlock && containingBlock->flowThreadContainingBlock()) { + if (containingBlock->exclusionShapeInsideInfo()) + return containingBlock->exclusionShapeInsideInfo(); + containingBlock = containingBlock->containingBlock(); + } } return shapeInsideInfo; }

(In reply to comment #4) > + RenderBlock* containingBlock = region->containingBlock(); > + while (containingBlock && containingBlock->flowThreadContainingBlock()) { > + if (containingBlock->exclusionShapeInsideInfo()) > + return containingBlock->exclusionShapeInsideInfo(); > + containingBlock = containingBlock->containingBlock(); > + } You're going up the region's containing block chain. The region isn't in a flow thread. It renders a portion of the flow thread. I guess I'm just confused why you have to walk up a containing block chain at all. Aren't you propagating exclusion info down to the region? Doesn't the region just know about it? Also, please limit this code to isOutOfFlowRenderFlowThreads (you can call this on the flowThreadContainingBlock), since it's not going to work for in-flow RenderFlowThreads.

Comment on attachment 192033 [details] proposed patch The spec will be going with a more conservative behavior. => This bug is invalid.

Surprise! The spec changed to make this issue valid again.