Bug 104818

Summary: [CSS Grid Layout] Include paddings and borders into the grid element's logical height / width
Product: WebKit Reporter: Julien Chaffraix <jchaffraix>
Component: Layout and RenderingAssignee: Julien Chaffraix <jchaffraix>
Status: RESOLVED FIXED    
Severity: Normal CC: eric, ojan.autocc, ojan, tony, webkit.review.bot, xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 60731    
Attachments:
Description Flags
Proposed change.
none
Patch for landing none

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.