RESOLVED FIXED 107062
[CSS Grid Layout] Updating -webkit-grid-rows or -webkit-grid-columns doesn't work as expected 
https://bugs.webkit.org/show_bug.cgi?id=107062
Summary [CSS Grid Layout] Updating -webkit-grid-rows or -webkit-grid-columns doesn't ...
Julien Chaffraix
Reported 2013-01-16 16:42:32 PST
Hinted by Ojan in https://bugs.webkit.org/show_bug.cgi?id=106474#c15. To reproduce, just try to update either properties and watch as we: * don't properly trigger a layout. * don't relayout the grid items, leaving them with a wrong size. Taking this bug as it will likely help me solve bug 106474.
Attachments
Proposed fix 1: Force a relayout if the grid area size changed and fixed a boolean logic in the style diff method. (12.30 KB, patch)
2013-01-16 17:34 PST, Julien Chaffraix 		no flags
DetailsFormatted DiffDiff
Patch for landing (11.92 KB, patch)
2013-01-17 11:10 PST, Julien Chaffraix 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Julien Chaffraix
Comment 1 2013-01-16 17:34:35 PST
Created attachment 183071 [details] Proposed fix 1: Force a relayout if the grid area size changed and fixed a boolean logic in the style diff method.
Tony Chang
Comment 2 2013-01-17 10:54:27 PST
Comment on attachment 183071 [details] Proposed fix 1: Force a relayout if the grid area size changed and fixed a boolean logic in the style diff method. View in context: https://bugs.webkit.org/attachment.cgi?id=183071&action=review > Source/WebCore/rendering/RenderGrid.cpp:236 > + LayoutUnit oldOverrideContainingBlockContentLogicalWidth = child->hasOverrideContainingBlockLogicalWidth() ? child->overrideContainingBlockContentLogicalWidth() : LayoutUnit(); > + LayoutUnit oldOverrideContainingBlockContentLogicalHeight = child->hasOverrideContainingBlockLogicalHeight() ? child->overrideContainingBlockContentLogicalHeight() : LayoutUnit(); > + LayoutUnit newOverrideContainingBlockContentLogicalWidth = columnTracks[columnTrack].m_usedBreadth; > + LayoutUnit newOverrideContainingBlockContentLogicalHeight = rowTracks[rowTrack].m_usedBreadth; This is pretty hard to read. How about: LayoutUnit oldOverrideContainingBlockContentLogicalWidth = child->hasOverrideContainingBlockLogicalWidth() ? child->overrideContainingBlockContentLogicalWidth() : LayoutUnit(); LayoutUnit oldOverrideContainingBlockContentLogicalHeight = child->hasOverrideContainingBlockLogicalHeight() ? child->overrideContainingBlockContentLogicalHeight() : LayoutUnit(); bool widthOrHeightChanged = oldOverrideContainingBlockContentLogicalWidth != columnTracks[columnTrack].m_usedBreadth || oldOverrideContainingBlockContentLogicalHeight != rowTracks[rowTrack].m_usedBreadth; if (widthOrHeightChanged) child->setNeedsLayout(true, MarkOnlyThis); child->setOverride... child->setOverride...
Julien Chaffraix
Comment 3 2013-01-17 11:01:45 PST
Comment on attachment 183071 [details] Proposed fix 1: Force a relayout if the grid area size changed and fixed a boolean logic in the style diff method. View in context: https://bugs.webkit.org/attachment.cgi?id=183071&action=review >> Source/WebCore/rendering/RenderGrid.cpp:236 >> + LayoutUnit newOverrideContainingBlockContentLogicalHeight = rowTracks[rowTrack].m_usedBreadth; > > This is pretty hard to read. How about: > LayoutUnit oldOverrideContainingBlockContentLogicalWidth = child->hasOverrideContainingBlockLogicalWidth() ? child->overrideContainingBlockContentLogicalWidth() : LayoutUnit(); > LayoutUnit oldOverrideContainingBlockContentLogicalHeight = child->hasOverrideContainingBlockLogicalHeight() ? child->overrideContainingBlockContentLogicalHeight() : LayoutUnit(); > > bool widthOrHeightChanged = oldOverrideContainingBlockContentLogicalWidth != columnTracks[columnTrack].m_usedBreadth || oldOverrideContainingBlockContentLogicalHeight != rowTracks[rowTrack].m_usedBreadth; > if (widthOrHeightChanged) > child->setNeedsLayout(true, MarkOnlyThis); > > child->setOverride... > child->setOverride... No issue with transforming the code, will change it before landing.
Julien Chaffraix
Comment 4 2013-01-17 11:10:26 PST
Created attachment 183220 [details] Patch for landing
WebKit Review Bot
Comment 5 2013-01-17 14:36:50 PST
Comment on attachment 183220 [details] Patch for landing Clearing flags on attachment: 183220 Committed r140045: <http://trac.webkit.org/changeset/140045>
WebKit Review Bot
Comment 6 2013-01-17 14:36:53 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.