RESOLVED FIXED 104818
[CSS Grid Layout] Include paddings and borders into the grid element's logical height / width 
https://bugs.webkit.org/show_bug.cgi?id=104818
Summary [CSS Grid Layout] Include paddings and borders into the grid element's logica...
Julien Chaffraix
Reported 2012-12-12 08:58:24 PST
Currently we don't account for paddings and borders on the grid element in computePreferredLogicalWidths and layoutGridItems. This means that we don't compute the right width / height during layout and needs to be fixed. Patch forthcoming.
Attachments
Proposed change. (5.23 KB, patch)
2012-12-12 09:05 PST, Julien Chaffraix 		no flags
DetailsFormatted DiffDiff
Patch for landing (5.18 KB, patch)
2012-12-12 17:58 PST, Julien Chaffraix 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Julien Chaffraix
Comment 1 2012-12-12 09:05:07 PST
Created attachment 179062 [details] Proposed change.
Tony Chang
Comment 2 2012-12-12 11:14:34 PST
Comment on attachment 179062 [details] Proposed change. View in context: https://bugs.webkit.org/attachment.cgi?id=179062&action=review > Source/WebCore/rendering/RenderGrid.cpp:203 > + LayoutUnit borderAndPaddingInBlockDirection = borderAndPaddingLogicalHeight(); > + setLogicalHeight(logicalHeight() + borderAndPaddingInBlockDirection); Nit: I would remove the temporary variable. > LayoutTests/fast/css-grid-layout/grid-element-padding-margin.html:9 > +.grid { > + display: -webkit-grid; We should consider making a .css file in resources that define classes for the various grid properties. This makes it easier to handle property name changes and makes it possible to make the tests cross platform by adding -ms prefixed properties. See css3/flexbox/resources/flexbox.css for an example of this. Would be good as a separate patch.
Julien Chaffraix
Comment 3 2012-12-12 17:52:28 PST
Comment on attachment 179062 [details] Proposed change. View in context: https://bugs.webkit.org/attachment.cgi?id=179062&action=review >> Source/WebCore/rendering/RenderGrid.cpp:203 >> + setLogicalHeight(logicalHeight() + borderAndPaddingInBlockDirection); > > Nit: I would remove the temporary variable. Will do. >> LayoutTests/fast/css-grid-layout/grid-element-padding-margin.html:9 >> + display: -webkit-grid; > > We should consider making a .css file in resources that define classes for the various grid properties. This makes it easier to handle property name changes and makes it possible to make the tests cross platform by adding -ms prefixed properties. See css3/flexbox/resources/flexbox.css for an example of this. Would be good as a separate patch. Filed https://bugs.webkit.org/show_bug.cgi?id=104869 about that.
Julien Chaffraix
Comment 4 2012-12-12 17:58:21 PST
Created attachment 179171 [details] Patch for landing
WebKit Review Bot
Comment 5 2012-12-12 18:50:56 PST
Comment on attachment 179171 [details] Patch for landing Clearing flags on attachment: 179171 Committed r137560: <http://trac.webkit.org/changeset/137560>
WebKit Review Bot
Comment 6 2012-12-12 18:51:00 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.