Created attachment 194289 [details] Test case See the attached test file. Increasing the vertical offset does not affect subsequent blocks.
Created attachment 196499 [details] proposed patch
Comment on attachment 196499 [details] proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=196499&action=review > Source/WebCore/rendering/RenderBlock.cpp:1423 > + // If there is an animation on the block, we need to relayout the shape to apply the shape on the animated content > + if (block->animation()) > + info->setNeedsLayout(true); Should this be more specific? Just wondering if you want to narrow it to certain properties animating? In general we haven't had to special case animation, so I'm trying to understand why this would be needed just for animation.
(In reply to comment #2) > (From update of attachment 196499 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=196499&action=review > > > Source/WebCore/rendering/RenderBlock.cpp:1423 > > + // If there is an animation on the block, we need to relayout the shape to apply the shape on the animated content > > + if (block->animation()) > > + info->setNeedsLayout(true); > > Should this be more specific? Just wondering if you want to narrow it to certain properties animating? In general we haven't had to special case animation, so I'm trying to understand why this would be needed just for animation. The bug appeared with transitions. I found that requesting relayout only for animation is the smallest subset of cases. By requesting the relayout we force RenderBlock::llayoutRunsAndFloatsInRange to use an updated segments set for the lines. Is this an answer to you question?
Comment on attachment 196499 [details] proposed patch All the animation code does is repeatedly change padding-top, so it seems like you're just working around a bug here. If you re-write the test case to manually set padding-top over and over using setTimeout instead of using an animation, won't it just still fail? I don't see why animation has anything to do with the bug. It's just how you happened to build the test case.
The bug is not animation specific, but has to do with children not laying out when a previous sibling has changed dimensions. I will upload a simplified testcase shortly.
Created attachment 196521 [details] Simplified testcase using setTimeout
Created attachment 196523 [details] Immediate testcase (without setTimeout) Based off of shape-inside-dynamic-nested.html testcase.
Created attachment 197179 [details] proposed patch Patch is updated. I wanted to do a little refactoring and use layoutExclusionShapeInsideInfo(...) from RenderBlockLineLayout instead of calling view()->layoutState()->exclusionShapeInsideInfo(), but it and logicalHeightForLine are used by other static functions as well, so the refactoring is out of scope for this bug.
Created attachment 197307 [details] proposed patch Please ignore my last comment, I can just use layoutExclusionShapeInsideInfo in updateRegionsAndExclusionsAfterChildLayout, since it's not a static anymore.
Comment on attachment 197307 [details] proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=197307&action=review Looks good, just fix the header file. > Source/WebCore/rendering/RenderBlock.h:582 > + void updateRegionsAndExclusionsAfterChildLayout(RenderFlowThread*, bool = false); You need to include the parameter name here, since it's not clear what the boolean means without it.
Created attachment 197317 [details] proposed patch, header fixed > > Source/WebCore/rendering/RenderBlock.h:582 > > + void updateRegionsAndExclusionsAfterChildLayout(RenderFlowThread*, bool = false); > > You need to include the parameter name here, since it's not clear what the boolean means without it. Fixed.
Comment on attachment 197317 [details] proposed patch, header fixed r=me
Comment on attachment 197317 [details] proposed patch, header fixed Clearing flags on attachment: 197317 Committed r148120: <http://trac.webkit.org/changeset/148120>
All reviewed patches have been landed. Closing bug.