Bug 180287 - REGRESSION(r221931): Row stretch doesn't work for grid container with min-height
Summary: REGRESSION(r221931): Row stretch doesn't work for grid container with min-height
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Manuel Rego Casasnovas
URL:
Keywords: InRadar
Depends on: 180345
Blocks:
  Show dependency tree / graph
 
Reported: 2017-12-01 15:12 PST by Javier Fernandez
Modified: 2017-12-11 01:12 PST (History)
10 users (show)

See Also:

Attachments
Test case to reproduce the bug (344 bytes, text/html)
2017-12-01 15:12 PST, Javier Fernandez 		no flags Details
Patch (8.44 KB, patch)
2017-12-06 15:24 PST, Manuel Rego Casasnovas 		no flags Details | Formatted Diff | Diff
Patch (8.44 KB, patch)
2017-12-06 16:05 PST, Manuel Rego Casasnovas 		no flags Details | Formatted Diff | Diff
Patch (11.42 KB, patch)
2017-12-06 16:17 PST, Manuel Rego Casasnovas 		no flags Details | Formatted Diff | Diff
Patch for landing (11.45 KB, patch)
2017-12-11 00:38 PST, Manuel Rego Casasnovas 		no flags Details | Formatted Diff | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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>