Bug 191889 - [css-grid] Fix grid container sizing under min-content height
Summary: [css-grid] Fix grid container sizing under min-content 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: BlinkMergeCandidate, InRadar
Depends on:
Blocks:
 
Reported: 2018-11-21 08:41 PST by Manuel Rego Casasnovas
Modified: 2018-11-26 00:07 PST (History)
8 users (show)

See Also:


Attachments
Example to reproduce the issue (257 bytes, text/html)
2018-11-21 08:41 PST, Manuel Rego Casasnovas
no flags Details
Patch (45.96 KB, patch)
2018-11-21 08:47 PST, Manuel Rego Casasnovas
no flags Details | Formatted Diff | Diff
Rebased patch for landing (45.93 KB, patch)
2018-11-23 03:41 PST, Manuel Rego Casasnovas
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews206 for win-future (12.82 MB, application/zip)
2018-11-23 10:20 PST, EWS Watchlist
no flags Details
Rebased patch for landing (45.90 KB, patch)
2018-11-23 15:16 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 Manuel Rego Casasnovas 2018-11-21 08:41:15 PST
Created attachment 355414 [details]
Example to reproduce the issue

Open the attached example, it's a grid container with "height: min-content" and "grid-template-rows: minmax(50px, 100px)".
The output should be a 100x100 green square, but it's a 100x50 rectangle.
BTW, this works fine in Firefox (we need to use -moz-min-content there).

We're behaving the same for "height: min-content" than for "width: min-content", but that's wrong per spec (https://drafts.csswg.org/css-sizing/#valdef-width-min-content):
  "min-content
    If specified for the inline axis, use the min-content inline size; otherwise behaves as the property’s initial value."

In the block axis "height: min-content" should behave like "height: auto".
Comment 1 Manuel Rego Casasnovas 2018-11-21 08:47:45 PST
Created attachment 355416 [details]
Patch
Comment 2 Sergio Villar Senin 2018-11-22 06:21:39 PST
Comment on attachment 355416 [details]
Patch

Awesome thanks. Happy to see all that hacky code being removed. Glad that the specs specify a sensible and compatible behaviour now.
Comment 3 Manuel Rego Casasnovas 2018-11-23 03:41:48 PST
Created attachment 355507 [details]
Rebased patch for landing
Comment 4 EWS Watchlist 2018-11-23 10:20:16 PST
Comment on attachment 355507 [details]
Rebased patch for landing

Attachment 355507 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/10120111

New failing tests:
webanimations/leak-document-with-web-animation.html
Comment 5 EWS Watchlist 2018-11-23 10:20:27 PST
Created attachment 355523 [details]
Archive of layout-test-results from ews206 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews206  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Comment 6 Manuel Rego Casasnovas 2018-11-23 13:22:33 PST
Comment on attachment 355507 [details]
Rebased patch for landing

win EWS failure is unrelated.
Comment 7 WebKit Commit Bot 2018-11-23 13:24:53 PST
Comment on attachment 355507 [details]
Rebased patch for landing

Rejecting attachment 355507 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-02', 'apply-attachment', '--no-update', '--non-interactive', 355507, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Logging in as commit-queue@webkit.org...
Fetching: https://bugs.webkit.org/attachment.cgi?id=355507&action=edit
Fetching: https://bugs.webkit.org/show_bug.cgi?id=191889&ctype=xml&excludefield=attachmentdata
Processing 1 patch from 1 bug.
Processing patch 355507 from bug 191889.
Fetching: https://bugs.webkit.org/attachment.cgi?id=355507
Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Parsed 11 diffs from patch file(s).
patching file Source/WebCore/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file Source/WebCore/rendering/RenderGrid.cpp
Hunk #1 FAILED at 245.
1 out of 4 hunks FAILED -- saving rejects to file Source/WebCore/rendering/RenderGrid.cpp.rej
patching file Source/WebCore/rendering/RenderGrid.h
patching file LayoutTests/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file LayoutTests/imported/w3c/ChangeLog
patching file LayoutTests/fast/css-grid-layout/grid-track-sizing-with-orthogonal-flows-expected.txt
patching file LayoutTests/fast/css-grid-layout/grid-track-sizing-with-orthogonal-flows.html
patching file LayoutTests/fast/css-grid-layout/maximize-tracks-definite-indefinite-height.html
patching file LayoutTests/imported/w3c/web-platform-tests/css/css-grid/grid-model/grid-container-sizing-constraints-001-expected.txt
patching file LayoutTests/imported/w3c/web-platform-tests/css/css-grid/grid-model/grid-container-sizing-constraints-001.html
patching file LayoutTests/imported/w3c/web-platform-tests/css/css-grid/grid-model/w3c-import.log

Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Full output: https://webkit-queues.webkit.org/results/10122076
Comment 8 Manuel Rego Casasnovas 2018-11-23 15:16:17 PST
Created attachment 355538 [details]
Rebased patch for landing
Comment 9 WebKit Commit Bot 2018-11-26 00:06:59 PST
Comment on attachment 355538 [details]
Rebased patch for landing

Clearing flags on attachment: 355538

Committed r238488: <https://trac.webkit.org/changeset/238488>
Comment 10 WebKit Commit Bot 2018-11-26 00:07:01 PST
All reviewed patches have been landed.  Closing bug.
Comment 11 Radar WebKit Bug Importer 2018-11-26 00:07:45 PST
<rdar://problem/46238029>