[css-grid] Positioned grid items cannot be dynamically moved |
||||||||
Issue descriptionIf you change the column/row of a positioned grid item, it's not properly relayout. Check the attached example to reproduce the issue. The problem is that this operation performs a simplifiedLayout(), this calls layoutPositionedObjects(), which is not properly overridden in LayoutGrid, so only the one in LayoutBlock is called (which doesn't have a clue about how to place the positioned grid item). The solution is to override properly layoutPositionedObjects(), but then in this situation we'll be hitting an ASSERT. Because LayoutBox::updateGridPositionAfterStyleChange() will be marking the grid as dirty, and we cannot perform layoutPositionedObjects() in a dirty grid. To fix this, we need to protect simplifiedLayout(). You cannot do a simplified layout if you've positioned items and the grid is dirty. Actually we can improve LayoutBox::updateGridPositionAfterStyleChange() as we don't need to mark the grid as dirty in this particular case. But I believe protecting simplifiedLayout() is going to be a good idea anyway, just in case we end up calling it with a dirty grid at some point.
,
Nov 7 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3c5effd4207aa48645264de4eaf3bca6c222ce71 commit 3c5effd4207aa48645264de4eaf3bca6c222ce71 Author: rego <rego@igalia.com> Date: Mon Nov 07 22:35:25 2016 [css-grid] Fix simplified layout of positioned grid items When a simplifiedLayout() is performed the method, for example when a positioned item is moved, LayoutGrid::layoutPositionedObjects() was not called. So the positioned items were not properly painted. To fix this we're properly overriding layoutPositionedObjects(). However we've to be careful and protect simplifiedLayout() so we don't perform it when we've positioned items and a dirty grid, otherwise we'll have troubles as layoutPositionedObjects() uses m_grid. On top of that, a new condition has been added into LayoutBox::updateGridPositionAfterStyleChange(), as for positioned items we don't need to mark the grid as dirty if they change the position. BUG= 662049 TEST=fast/css-grid-layout/grid-positioned-item-dynamic-change-expected.html Review-Url: https://codereview.chromium.org/2471283004 Cr-Commit-Position: refs/heads/master@{#430401} [add] https://crrev.com/3c5effd4207aa48645264de4eaf3bca6c222ce71/third_party/WebKit/LayoutTests/fast/css-grid-layout/grid-positioned-item-dynamic-change-expected.html [add] https://crrev.com/3c5effd4207aa48645264de4eaf3bca6c222ce71/third_party/WebKit/LayoutTests/fast/css-grid-layout/grid-positioned-item-dynamic-change.html [modify] https://crrev.com/3c5effd4207aa48645264de4eaf3bca6c222ce71/third_party/WebKit/Source/core/layout/LayoutBlock.h [modify] https://crrev.com/3c5effd4207aa48645264de4eaf3bca6c222ce71/third_party/WebKit/Source/core/layout/LayoutBox.cpp [modify] https://crrev.com/3c5effd4207aa48645264de4eaf3bca6c222ce71/third_party/WebKit/Source/core/layout/LayoutGrid.cpp [modify] https://crrev.com/3c5effd4207aa48645264de4eaf3bca6c222ce71/third_party/WebKit/Source/core/layout/LayoutGrid.h
,
Nov 8 2016
|
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by r...@igalia.com
, Nov 3 2016