Bug 210465

Summary:

[css-flexbox] ChildIntrinsicLogicalWidth should use fit-content, not max-content

Product:

WebKit

Reporter:

Carlos Alberto Lopez Perez <clopez>

Component:

CSS

Assignee:

Sergio Villar Senin <svillar>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

changseok, esprehn+autocc, ews-watchlist, glenn, jfernandez, kondapallykalyan, pdr, rego, simon.fraser, svillar, webkit-bug-importer, zalan

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

See Also:

https://bugs.webkit.org/show_bug.cgi?id=210103
https://bugs.chromium.org/p/chromium/issues/detail?id=876749

Bug Depends on:

Bug Blocks:

209871

Attachments:

Description	Flags
Patch	none
Patch	jfernandez: review+

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>