Bug 220582 - Handle min-width/min-height:auto for aspect-ratio
Summary: Handle min-width/min-height:auto for aspect-ratio
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Rob Buis
URL:
Keywords: InRadar
Depends on:
Blocks: 47738
  Show dependency tree / graph
 
Reported: 2021-01-13 01:50 PST by Rob Buis
Modified: 2021-02-11 02:55 PST (History)
8 users (show)

See Also:

Attachments
Patch (9.42 KB, patch)
2021-01-13 01:53 PST, Rob Buis 		no flags Details | Formatted Diff | Diff
Patch (9.27 KB, patch)
2021-01-17 12:19 PST, Rob Buis 		no flags Details | Formatted Diff | Diff
Patch (9.34 KB, patch)
2021-01-20 07:58 PST, Rob Buis 		no flags Details | Formatted Diff | Diff
Patch (9.57 KB, patch)
2021-01-20 13:06 PST, Rob Buis 		no flags Details | Formatted Diff | Diff
Patch (9.99 KB, patch)
2021-01-27 06:46 PST, Rob Buis 		no flags Details | Formatted Diff | Diff
Patch (9.89 KB, patch)
2021-02-04 03:48 PST, Rob Buis 		no flags Details | Formatted Diff | Diff
Patch (9.57 KB, patch)
2021-02-09 00:46 PST, Rob Buis 		no flags Details | Formatted Diff | Diff
Patch (9.60 KB, patch)
2021-02-11 01:54 PST, Rob Buis 		no flags Details | Formatted Diff | Diff
Show Obsolete (7) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Rob Buis 2021-01-13 01:50:36 PST 
Handle min-width/min-height:auto for aspect-ratio.
Comment 1 Rob Buis 2021-01-13 01:53:36 PST 
Created attachment 417516 [details]
Patch
Comment 2 Rob Buis 2021-01-17 12:19:13 PST 
Created attachment 417781 [details]
Patch
Comment 3 Radar WebKit Bug Importer 2021-01-20 01:51:12 PST 
<rdar://problem/73395736>
Comment 4 Rob Buis 2021-01-20 07:58:27 PST 
Created attachment 417968 [details]
Patch
Comment 5 Rob Buis 2021-01-20 13:06:02 PST 
Created attachment 417992 [details]
Patch
Comment 6 Rob Buis 2021-01-27 06:46:01 PST 
Created attachment 418541 [details]
Patch
Comment 7 Rob Buis 2021-02-04 03:48:26 PST 
Created attachment 419265 [details]
Patch
Comment 8 Rob Buis 2021-02-09 00:46:55 PST 
Created attachment 419685 [details]
Patch
Comment 9 Antti Koivisto 2021-02-11 01:14:20 PST 
Comment on attachment 419685 [details]
Patch

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

> Source/WebCore/rendering/RenderBox.cpp:665
> +    Length minLength = styleToUse.logicalMinWidth();

could use auto

> Source/WebCore/rendering/RenderBox.cpp:3810
> +    if (widthType == MinSize && logicalWidth.isAuto()) {
> +        if (shouldComputeLogicalWidthFromAspectRatio()) {
> +            LayoutUnit minLogicalWidth;
> +            LayoutUnit maxLogicalWidth;
> +            computeIntrinsicLogicalWidths(minLogicalWidth, maxLogicalWidth);
> +            logicalWidth = Length(minLogicalWidth, Fixed);
> +        } else
> +            logicalWidth = Length(0, Fixed);
> +    } else if (widthType == MainOrPreferredSize && logicalWidth.isAuto() && shouldComputeLogicalWidthFromAspectRatio())
>          logicalWidth = Length(computeLogicalWidthFromAspectRatio(), Fixed);
>      else if (logicalWidth.isIntrinsic())
>          logicalWidth = Length(computeIntrinsicLogicalWidthUsing(logicalWidth, containerLogicalWidth, bordersPlusPadding) - bordersPlusPadding, Fixed);

This sort of code can often made read better by using lambdas and early returns.
Comment 10 Antti Koivisto 2021-02-11 01:15:58 PST 
Comment on attachment 419685 [details]
Patch

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

> LayoutTests/TestExpectations:4421
> +webkit.org/b/214463 imported/w3c/web-platform-tests/css/css-sizing/aspect-ratio/flex-aspect-ratio-008.html [ ImageOnlyFailure ]

Why does this break?
Comment 11 Rob Buis 2021-02-11 01:19:03 PST 
Comment on attachment 419685 [details]
Patch

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

>> LayoutTests/TestExpectations:4421
>> +webkit.org/b/214463 imported/w3c/web-platform-tests/css/css-sizing/aspect-ratio/flex-aspect-ratio-008.html [ ImageOnlyFailure ]
> 
> Why does this break?

Flex support still needs to land:
https://bugs.webkit.org/show_bug.cgi?id=219679
I am waiting on an Igalia colleague for some flexbox fixes so that patch can be rebased and made smaller.
Comment 12 Rob Buis 2021-02-11 01:54:31 PST 
Created attachment 419966 [details]
Patch
Comment 13 EWS 2021-02-11 02:55:25 PST 
Committed r272718: <https://commits.webkit.org/r272718>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 419966 [details].