Bug 221479

Summary: [css-flexbox] Add flex-basis: content support
Product: WebKit Reporter: Sergio Villar Senin <svillar>
Component: Layout and RenderingAssignee: Sergio Villar Senin <svillar>
Status: RESOLVED FIXED    
Severity: Normal CC: ayumi_kojima, bfulgham, changseok, esprehn+autocc, ews-watchlist, glenn, graouts, gyuyoung.kim, jfernandez, joepeck, kondapallykalyan, macpherson, menard, pdr, rbuis, rego, simon.fraser, svillar, webkit-bug-importer, ysuzuki, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 232481    
Bug Blocks: 136754    
Attachments:
Description Flags
Patch
none
Patch
none
Patch jfernandez: review+

Sergio Villar Senin
Reported 2021-02-05 09:29:31 PST
Newly imported WPT tests are failing: imported/w3c/web-platform-tests/css/css-flexbox/flexbox-flex-basis-content-001a.html imported/w3c/web-platform-tests/css/css-flexbox/flexbox-flex-basis-content-001b.html imported/w3c/web-platform-tests/css/css-flexbox/flexbox-flex-basis-content-002a.html imported/w3c/web-platform-tests/css/css-flexbox/flexbox-flex-basis-content-002b.html imported/w3c/web-platform-tests/css/css-flexbox/flexbox-flex-basis-content-003a.html imported/w3c/web-platform-tests/css/css-flexbox/flexbox-flex-basis-content-004a.html
Attachments
Patch (21.27 KB, patch)
2021-10-18 04:18 PDT, Sergio Villar Senin 		no flags
DetailsFormatted DiffDiff
Patch (23.04 KB, patch)
2021-10-18 07:45 PDT, Sergio Villar Senin 		no flags
DetailsFormatted DiffDiff
Patch (22.56 KB, patch)
2021-11-11 04:32 PST, Sergio Villar Senin 		jfernandez: review+
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Rob Buis
Comment 1 2021-02-11 05:30:42 PST
It is quite possible aspect-ratio like this one depend on this support: css/css-sizing/aspect-ratio/flex-aspect-ratio-021.html
Radar WebKit Bug Importer
Comment 2 2021-02-12 09:30:14 PST
<rdar://problem/74279369>
Sergio Villar Senin
Comment 3 2021-09-24 04:51:44 PDT
Also some other tests that depend on this: imported/w3c/web-platform-tests/css/css-flexbox/parsing/flex-basis-computed.html imported/w3c/web-platform-tests/css/css-flexbox/parsing/flex-basis-valid.html imported/w3c/web-platform-tests/css/css-flexbox/parsing/flex-shorthand.html they currently have FAIL expectations for those content|fit-content|min-content|max-content values
Sergio Villar Senin
Comment 4 2021-10-18 04:18:31 PDT
Created attachment 441583 [details] Patch
Sergio Villar Senin
Comment 5 2021-10-18 07:45:53 PDT
Created attachment 441602 [details] Patch
Javier Fernandez
Comment 6 2021-10-18 14:05:55 PDT
Comment on attachment 441602 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=441602&action=review r=me > Source/WebCore/ChangeLog:10 > + equivalent to the max-content size but it has some adjustments for aspect ratios, Is this patch implementing those aspect-ratio adjustments ? Otherwise, perhaps we should clarify it in the ChangeLog.
Sergio Villar Senin
Comment 7 2021-10-19 01:21:47 PDT
Comment on attachment 441602 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=441602&action=review Thanks for the review! >> Source/WebCore/ChangeLog:10 >> + equivalent to the max-content size but it has some adjustments for aspect ratios, > > Is this patch implementing those aspect-ratio adjustments ? Otherwise, perhaps we should clarify it in the ChangeLog. Yes, those adjustments are implemented in computeFlexBaseSizeForChild(). They were previously applied only to 'auto'. Now they apply also to 'content'.
Sergio Villar Senin
Comment 8 2021-10-19 01:26:44 PDT
(In reply to Sergio Villar Senin from comment #7) > Comment on attachment 441602 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=441602&action=review > > Thanks for the review! > > >> Source/WebCore/ChangeLog:10 > >> + equivalent to the max-content size but it has some adjustments for aspect ratios, > > > > Is this patch implementing those aspect-ratio adjustments ? Otherwise, perhaps we should clarify it in the ChangeLog. > > Yes, those adjustments are implemented in computeFlexBaseSizeForChild(). > They were previously applied only to 'auto'. Now they apply also to > 'content'. I meant, the adjustments were implemented long time ago. It's just that we were applying to those cases were flex-basis is auto. Now they apply also to the cases were flex-basis is content. More precisely both cases are the same because the *used* value for flex basis is 'content' when 'flex-basis:auto' and the main size of the flex item is also 'auto'.
Sergio Villar Senin
Comment 9 2021-10-19 03:14:50 PDT
Committed r284440 (243202@main): <https://commits.webkit.org/243202@main>
Yusuke Suzuki
Comment 10 2021-10-29 13:18:18 PDT
Reverted in https://trac.webkit.org/changeset/285045/webkit Because the relying change causes 100% CPU usage in Twitter
ayumi_kojima
Comment 11 2021-11-02 16:16:28 PDT
After the revert, imported/w3c/web-platform-tests/css/css-sizing/aspect-ratio/flex-aspect-ratio-021.html imported/w3c/web-platform-tests/css/css-sizing/aspect-ratio/flex-aspect-ratio-022.html Started constantly failing. Tracking in https://bugs.webkit.org/show_bug.cgi?id=232518.
Sergio Villar Senin
Comment 12 2021-11-11 04:32:10 PST
Created attachment 443937 [details] Patch
Javier Fernandez
Comment 13 2021-11-11 05:02:20 PST
Comment on attachment 443937 [details] Patch r=me (as long as all the bots finish green)
Sergio Villar Senin
Comment 14 2021-11-12 02:17:33 PST
Committed r285709 (244168@main): <https://commits.webkit.org/244168@main>
Note You need to log in before you can comment on or make changes to this bug.