Bug 180287

Summary: REGRESSION(r221931): Row stretch doesn't work for grid container with min-height
Product: WebKit Reporter: Javier Fernandez <jfernandez>
Component: Layout and RenderingAssignee: Manuel Rego Casasnovas <rego>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, commit-queue, darin, ews-watchlist, jfernandez, rego, simon.fraser, svillar, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.chromium.org/p/chromium/issues/detail?id=773625
Bug Depends on: 180345    
Bug Blocks:    
Attachments:
Description Flags
Test case to reproduce the bug
none
Patch
none
Patch
none
Patch
none
Patch for landing none

Description Javier Fernandez 2017-12-01 15:12:04 PST
Created attachment 328167 [details]
Test case to reproduce the bug

This was working fine before r221931 ( bug #176783 )

Just open the attached example and both grids should have the same output (the item should have 200px height).
However the one with "min-height: 200px" is not stretching.
Comment 1 Manuel Rego Casasnovas 2017-12-06 15:24:44 PST
Created attachment 328638 [details]
Patch
Comment 2 EWS Watchlist 2017-12-06 15:26:58 PST
Attachment 328638 [details] did not pass style-queue:


ERROR: Source/WebCore/rendering/GridTrackSizingAlgorithm.cpp:941:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Total errors found: 1 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Manuel Rego Casasnovas 2017-12-06 16:05:21 PST
Created attachment 328643 [details]
Patch
Comment 4 Manuel Rego Casasnovas 2017-12-06 16:17:56 PST
Created attachment 328647 [details]
Patch
Comment 5 Darin Adler 2017-12-08 10:27:08 PST
Comment on attachment 328647 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=328647&action=review

> Source/WebCore/rendering/GridTrackSizingAlgorithm.cpp:944
> +    ASSERT(m_algorithm.freeSpace(direction()));
> +    return m_algorithm.freeSpace(direction()).value();

This assertion isn’t needed. Our version of std::optional::value includes an assertion.
Comment 6 Manuel Rego Casasnovas 2017-12-11 00:33:18 PST
Comment on attachment 328647 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=328647&action=review

Thanks for the review.

>> Source/WebCore/rendering/GridTrackSizingAlgorithm.cpp:944
>> +    return m_algorithm.freeSpace(direction()).value();
> 
> This assertion isn’t needed. Our version of std::optional::value includes an assertion.

Ok, removed it.
Comment 7 Manuel Rego Casasnovas 2017-12-11 00:38:44 PST
Created attachment 328949 [details]
Patch for landing
Comment 8 WebKit Commit Bot 2017-12-11 01:11:23 PST
Comment on attachment 328949 [details]
Patch for landing

Clearing flags on attachment: 328949

Committed r225741: <https://trac.webkit.org/changeset/225741>
Comment 9 WebKit Commit Bot 2017-12-11 01:11:24 PST
All reviewed patches have been landed.  Closing bug.
Comment 10 Radar WebKit Bug Importer 2017-12-11 01:12:27 PST
<rdar://problem/35963250>