Bug 212869 - [LFC] Add support for width: min/max-content
Summary: [LFC] Add support for width: min/max-content
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: zalan
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-06-06 09:11 PDT by zalan
Modified: 2020-06-06 19:32 PDT (History)
6 users (show)

See Also:


Attachments
Patch (20.96 KB, patch)
2020-06-06 09:37 PDT, zalan
no flags Details | Formatted Diff | Diff
Patch (21.06 KB, patch)
2020-06-06 09:55 PDT, zalan
no flags Details | Formatted Diff | Diff
Patch (21.31 KB, patch)
2020-06-06 10:12 PDT, zalan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description zalan 2020-06-06 09:11:31 PDT
ssia
Comment 1 zalan 2020-06-06 09:37:05 PDT
Created attachment 401263 [details]
Patch
Comment 2 Antti Koivisto 2020-06-06 09:39:28 PDT
Comment on attachment 401263 [details]
Patch

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

> Source/WebCore/layout/FormattingContextGeometry.cpp:107
> +    auto width = widthType == WidthType::Normal ? style.logicalWidth() : widthType == WidthType::Min ? style.logicalMinWidth() : style.logicalMaxWidth();

a lambda might be more readable
Comment 3 Antti Koivisto 2020-06-06 09:41:04 PDT
Comment on attachment 401263 [details]
Patch

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

>> Source/WebCore/layout/FormattingContextGeometry.cpp:107
>> +    auto width = widthType == WidthType::Normal ? style.logicalWidth() : widthType == WidthType::Min ? style.logicalMinWidth() : style.logicalMaxWidth();
> 
> a lambda might be more readable

with a switch
Comment 4 zalan 2020-06-06 09:55:21 PDT
Created attachment 401265 [details]
Patch
Comment 5 zalan 2020-06-06 10:12:11 PDT
Created attachment 401267 [details]
Patch
Comment 6 EWS 2020-06-06 11:02:13 PDT
Committed r262690: <https://trac.webkit.org/changeset/262690>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 401267 [details].
Comment 7 Radar WebKit Bug Importer 2020-06-06 11:03:21 PDT
<rdar://problem/64071478>
Comment 8 Sam Weinig 2020-06-06 16:19:45 PDT
Comment on attachment 401267 [details]
Patch

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

> Source/WebCore/layout/FormattingContextGeometry.cpp:113
> +        auto width = style.logicalWidth();  
> +
> +        switch (widthType) {
> +        case WidthType::Normal:
> +            width = style.logicalWidth();

Is it necessary to compute call style.logicalWidth() twice here?
Comment 9 zalan 2020-06-06 19:32:52 PDT
(In reply to Sam Weinig from comment #8)
> Comment on attachment 401267 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=401267&action=review
> 
> > Source/WebCore/layout/FormattingContextGeometry.cpp:113
> > +        auto width = style.logicalWidth();  
> > +
> > +        switch (widthType) {
> > +        case WidthType::Normal:
> > +            width = style.logicalWidth();
> 
> Is it necessary to compute call style.logicalWidth() twice here?
Haha, yeah absolutely not. Thanks.