Bug 221210

Summary: [css-flexbox] Do not use margins when computing aspect ratio cross sizes
Product: WebKit Reporter: Sergio Villar Senin <svillar>
Component: New BugsAssignee: Sergio Villar Senin <svillar>
Status: RESOLVED FIXED    
Severity: Normal CC: changseok, clopez, esprehn+autocc, ews-watchlist, glenn, jfernandez, kondapallykalyan, pdr, rego, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 219343    
Attachments:
Description Flags
Patch
none
Patch jfernandez: review+, ews-feeder: commit-queue-

Description Sergio Villar Senin 2021-02-01 08:31:50 PST
[css-flexbox] Do not use margins when computing aspect ratio cross sizes
Comment 1 Sergio Villar Senin 2021-02-01 08:44:05 PST
Created attachment 418880 [details]
Patch
Comment 2 Radar WebKit Bug Importer 2021-02-08 08:32:14 PST
<rdar://problem/74097534>
Comment 3 Javier Fernandez 2021-03-04 06:54:15 PST
Comment on attachment 418880 [details]
Patch

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

> Source/WebCore/rendering/RenderFlexibleBox.cpp:789
> +        crossSize = adjustForBoxSizing(*this, containerCrossSizeLength) - crossAxisMarginExtentForChild(child);

What should we do if the box-sizing is 0 ? Do we have tests for that case ?
Comment 4 Sergio Villar Senin 2021-04-13 02:26:09 PDT
Comment on attachment 418880 [details]
Patch

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

>> Source/WebCore/rendering/RenderFlexibleBox.cpp:789
>> +        crossSize = adjustForBoxSizing(*this, containerCrossSizeLength) - crossAxisMarginExtentForChild(child);
> 
> What should we do if the box-sizing is 0 ? Do we have tests for that case ?

You mean when the difference is negative? Perhaps we should clamp it indeed
Comment 5 Sergio Villar Senin 2021-05-12 04:57:21 PDT
Created attachment 428368 [details]
Patch
Comment 6 Sergio Villar Senin 2021-05-12 04:58:29 PDT
Comment on attachment 418880 [details]
Patch

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

>>> Source/WebCore/rendering/RenderFlexibleBox.cpp:789
>>> +        crossSize = adjustForBoxSizing(*this, containerCrossSizeLength) - crossAxisMarginExtentForChild(child);
>> 
>> What should we do if the box-sizing is 0 ? Do we have tests for that case ?
> 
> You mean when the difference is negative? Perhaps we should clamp it indeed

I've added clamping in the last version of the patch along with a couple of extra test cases imported from WPT to increase coverage of the cases you mentioned.
Comment 7 EWS Watchlist 2021-05-12 04:58:48 PDT
This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess
Comment 8 Javier Fernandez 2021-05-12 05:44:26 PDT
Comment on attachment 428368 [details]
Patch

r=me
Comment 9 Sergio Villar Senin 2021-05-12 08:59:59 PDT
Committed r277371 (237629@main): <https://commits.webkit.org/237629@main>