Bug 221210 - [css-flexbox] Do not use margins when computing aspect ratio cross sizes
Summary: [css-flexbox] Do not use margins when computing aspect ratio cross sizes
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Sergio Villar Senin
URL:
Keywords: InRadar
Depends on:
Blocks: 219343
  Show dependency treegraph
 
Reported: 2021-02-01 08:31 PST by Sergio Villar Senin
Modified: 2021-05-12 08:59 PDT (History)
11 users (show)

See Also:


Attachments
Patch (6.93 KB, patch)
2021-02-01 08:44 PST, Sergio Villar Senin
no flags Details | Formatted Diff | Diff
Patch (9.66 KB, patch)
2021-05-12 04:57 PDT, Sergio Villar Senin
jfernandez: review+
ews-feeder: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>