Bug 159253

Summary: [css-grid] Inline size is never indefinite during layout
Product: WebKit Reporter: Manuel Rego Casasnovas <rego>
Component: Layout and RenderingAssignee: Manuel Rego Casasnovas <rego>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, esprehn+autocc, glenn, jfernandez, kondapallykalyan, rego, simon.fraser, svillar
Priority: P2 Keywords: BlinkMergeCandidate
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.chromium.org/p/chromium/issues/detail?id=616716
Bug Depends on:    
Bug Blocks: 60731    
Attachments:
Description Flags
Patch
none
Patch
none
Patch for landing none

Manuel Rego Casasnovas
Reported 2016-06-29 00:32:03 PDT
This bug is related with the following text on the spec [1]: "If the inline or block size of the grid container is indefinite, <percentage> values relative to that size are treated as auto." The thing is that the inline size is only indefinite while we're computing the intrinsic sizes of the grid container. However during layout the inline size is always definite, so we can resolve the percentage tracks against it. See a more detailed explanation on the CSS WG mailing list: https://lists.w3.org/Archives/Public/www-style/2016Jun/0007.html And also on the Firefox bugzilla discussion: https://bugzilla.mozilla.org/show_bug.cgi?id=1264607 This has been already fixed in Blink: https://codereview.chromium.org/2033033002 [1] https://drafts.csswg.org/css-grid/#valdef-grid-template-columns-percentage
Attachments
Patch (41.11 KB, patch)
2016-06-29 00:37 PDT, Manuel Rego Casasnovas 		no flags
DetailsFormatted DiffDiff
Patch (48.68 KB, patch)
2016-07-06 07:29 PDT, Manuel Rego Casasnovas 		no flags
DetailsFormatted DiffDiff
Patch for landing (48.68 KB, patch)
2016-07-08 02:41 PDT, Manuel Rego Casasnovas 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Manuel Rego Casasnovas
Comment 1 2016-06-29 00:37:24 PDT
Created attachment 282330 [details] Patch
Sergio Villar Senin
Comment 2 2016-07-06 02:59:03 PDT
Comment on attachment 282330 [details] Patch There are many more gridTrackSize() calls in the code, are you completely sure that all the other ones are only called during the TrackSizing phase?
Manuel Rego Casasnovas
Comment 3 2016-07-06 07:29:42 PDT
Created attachment 282892 [details] Patch
Manuel Rego Casasnovas
Comment 4 2016-07-06 07:31:17 PDT
(In reply to comment #2) > Comment on attachment 282330 [details] > Patch > > There are many more gridTrackSize() calls in the code, are you completely > sure that all the other ones are only called during the TrackSizing phase? Yep you're right, I just make the SizingOperation a mandatory argument for gridTrackSize(). So we're sure we're passing the right one in every call. Otherwise, it was hard to tell if all of them were called during the TrackSizing phase only.
Sergio Villar Senin
Comment 5 2016-07-08 01:44:02 PDT
Comment on attachment 282892 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=282892&action=review > Source/WebCore/ChangeLog:12 > + This makes Grid Layout compatible with regular blocks regarding Put everything in the same paragraph. > Source/WebCore/rendering/RenderGrid.cpp:763 > + // For the inline axis this only happens when we're computing the intrinsic sizes (AvailableSpaceIndefinite). Better use IntrinsicSizeComputation which is what we're checking bellow. > LayoutTests/fast/css-grid-layout/percent-track-breadths-regarding-container-size.html:122 > + <!-- The height of the row is wrong calculated in the following 2 examples because of a bug in FIXME needed?
Manuel Rego Casasnovas
Comment 6 2016-07-08 02:41:51 PDT
Created attachment 283131 [details] Patch for landing Thanks for the review. Applied suggested changes.
WebKit Commit Bot
Comment 7 2016-07-08 03:11:49 PDT
Comment on attachment 283131 [details] Patch for landing Clearing flags on attachment: 283131 Committed r202974: <http://trac.webkit.org/changeset/202974>
WebKit Commit Bot
Comment 8 2016-07-08 03:11:54 PDT
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.