Bug 107062

Summary:

[CSS Grid Layout] Updating -webkit-grid-rows or -webkit-grid-columns doesn't work as expected

Product:

WebKit

Reporter:

Julien Chaffraix <jchaffraix>

Component:

Layout and Rendering

Assignee:

Julien Chaffraix <jchaffraix>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

donggwan.kim, eric, ojan.autocc, ojan, tony, webkit.review.bot, xan.lopez

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Bug Depends on:

Bug Blocks:

60731

Attachments:

Description	Flags
Proposed fix 1: Force a relayout if the grid area size changed and fixed a boolean logic in the style diff method.	none
Patch for landing	none

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	Details Formatted Diff Diff
Patch for landing (11.92 KB, patch) 2013-01-17 11:10 PST, Julien Chaffraix	no flags	Details Formatted Diff Diff
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.