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 treegraph
 
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

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 Build Bot 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>