Bug 246638

Summary: [leading-trim] Add support for parsing "leading-trim" CSS property
Product: WebKit Reporter: zalan <zalan>
Component: Layout and RenderingAssignee: zalan <zalan>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, changseok, esprehn+autocc, ews-watchlist, glenn, gyuyoung.kim, joepeck, koivisto, kondapallykalyan, macpherson, menard, ntim, pdr, simon.fraser, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 245599    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
[fast-cq]Patch none

Description zalan 2022-10-17 10:01:22 PDT 
ssia
Comment 1 zalan 2022-10-17 10:04:49 PDT 
Created attachment 463035 [details]
Patch
Comment 2 zalan 2022-10-17 10:05:08 PDT 
no tests yet.
Comment 3 Tim Nguyen (:ntim) 2022-10-17 10:36:16 PDT 
Comment on attachment 463035 [details]
Patch

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

> Source/WebCore/rendering/style/StyleRareNonInheritedData.h:250
> +    unsigned leadingTrim : 3; // LeadingTrim

This fits in 2
Comment 4 Tim Nguyen (:ntim) 2022-10-17 10:37:15 PDT 
(Also, does this need a `settings-flag` field in CSSProperties.json and an associated preference?)
Comment 5 Antti Koivisto 2022-10-17 11:00:25 PDT 
Comment on attachment 463035 [details]
Patch

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

> Source/WebCore/css/CSSProperties.json:2959
> +        "leading-trim": {
> +            "values": [
> +                "normal",
> +                "start",
> +                "end",
> +                "both"
> +            ],
> +            "specification": {
> +                "category": "css-inline",
> +                "url": "https://www.w3.org/TR/css-inline-3/#leading-trim"
> +            }
> +        },

This should probably have a setting which can be given using "settings-flag" codegen option.
Comment 6 Antti Koivisto 2022-10-18 08:00:55 PDT 
Comment on attachment 463035 [details]
Patch

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

>> Source/WebCore/rendering/style/StyleRareNonInheritedData.h:250
>> +    unsigned leadingTrim : 3; // LeadingTrim
> 
> This fits in 2

You should initialise this field.

It also needs to be present in the copy constructor and operator==.
Comment 7 Antti Koivisto 2022-10-18 08:06:29 PDT 
Comment on attachment 463035 [details]
Patch

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

> Source/WebCore/css/CSSPrimitiveValueMappings.h:1089
> +template<> inline CSSPrimitiveValue::CSSPrimitiveValue(LeadingTrim e)

"e"

> Source/WebCore/css/CSSPrimitiveValueMappings.h:1106
> +        break;
> +    }

Could ASSERT_NOT_REACHED here
Comment 8 Radar WebKit Bug Importer 2022-10-24 10:02:20 PDT 
<rdar://problem/101504835>
Comment 9 zalan 2022-11-27 16:12:18 PST 
Created attachment 463745 [details]
Patch
Comment 10 zalan 2022-11-27 20:39:42 PST 
Created attachment 463748 [details]
Patch
Comment 11 zalan 2022-11-28 07:11:16 PST 
Created attachment 463760 [details]
[fast-cq]Patch
Comment 12 EWS 2022-11-28 07:33:00 PST 
Committed 257070@main (8fe0bbfa84f9): <https://commits.webkit.org/257070@main>

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