Bug 210465 - [css-flexbox] ChildIntrinsicLogicalWidth should use fit-content, not max-content
Summary: [css-flexbox] ChildIntrinsicLogicalWidth should use fit-content, not max-content
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Sergio Villar Senin
URL:
Keywords: InRadar
Depends on:
Blocks: 209871
  Show dependency treegraph
 
Reported: 2020-04-13 16:08 PDT by Carlos Alberto Lopez Perez
Modified: 2020-06-02 04:29 PDT (History)
12 users (show)

See Also:


Attachments
Patch (4.10 KB, patch)
2020-06-01 13:46 PDT, Sergio Villar Senin
no flags Details | Formatted Diff | Diff
Patch (4.14 KB, patch)
2020-06-02 03:32 PDT, Sergio Villar Senin
jfernandez: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Alberto Lopez Perez 2020-04-13 16:08:29 PDT
A fixed-layout table at 100% width expands to 800,000px when in a flex-flow: column wrap

Example URL:
https://jsfiddle.net/zhv79eub/14/

Steps to reproduce the problem:
1. Have a flexbox container with `flex-flow: column wrap`.
2. Create multiple items such that a wrap occurs.
3. If one of the items is a div with a table which has `width: 100%` and `table-layout: fixed`, it will expand to become ~800,000px wide instead of a reasonable size.

What is the expected behavior?
From a UI perspective, the table should be a sane size - either the width of a single column prior to wrapping or however big it needs to be to fit the content (i.e., table-layout: auto).

What went wrong?
Each table grows to be enormous (800k pixels).

The table has a width of 100%, so it's possible there is some broken loop where the container's width is 'as big as needed' and each table effectively grows as large as it can before hitting some weird maximum?

Does it occur on multiple sites: Yes


This very same bug was reported for chrome in https://bugs.chromium.org/p/chromium/issues/detail?id=876749
It was fixed there by changing ChildIntrinsicLogicalWidth to use fit-content instead of max-content

There are two WPT tests for this at:
http://wpt.live/css/css-flexbox/flex-wrap-002.html
http://wpt.live/css/css-flexbox/flex-wrap-003.html

The first one its currently failing on WebKit
Comment 1 Sergio Villar Senin 2020-06-01 13:46:24 PDT
Created attachment 400754 [details]
Patch
Comment 2 Javier Fernandez 2020-06-01 14:11:33 PDT
Comment on attachment 400754 [details]
Patch

r=me
Comment 3 Javier Fernandez 2020-06-01 14:30:04 PDT
Comment on attachment 400754 [details]
Patch

The change looks good, but tt seems that the patch causes some regression in other flexbox related tests.
Comment 4 Javier Fernandez 2020-06-01 15:28:12 PDT
(In reply to Javier Fernandez from comment #3)
> Comment on attachment 400754 [details]
> Patch
> 
> The change looks good, but tt seems that the patch causes some regression in
> other flexbox related tests.

imported/w3c/web-platform-tests/css/css-flexbox/flexbox_width-wrapping-column.html
Comment 5 Sergio Villar Senin 2020-06-02 03:31:47 PDT
Comment on attachment 400754 [details]
Patch

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

Uploading a new version now

> Source/WebCore/rendering/RenderFlexibleBox.cpp:472
> +        std::min(child.maxPreferredLogicalWidth(), std::max(child.minPreferredLogicalWidth(), contentLogicalWidth()));

Cannot believe I forgot the return....
Comment 6 Sergio Villar Senin 2020-06-02 03:32:38 PDT
Created attachment 400800 [details]
Patch
Comment 7 Sergio Villar Senin 2020-06-02 04:28:37 PDT
Committed r262411: <https://trac.webkit.org/changeset/262411>
Comment 8 Radar WebKit Bug Importer 2020-06-02 04:29:18 PDT
<rdar://problem/63869685>