Bug 234430 – [css-grid] Fix grid shorthand expansion of initial values

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

We need to  add InitialValue support for 6 grid properties when parsing the grid shorthand. At the moment, it just adds a CSSInitial.

Affected WPT test -

imported/w3c/web-platform-tests/css/css-grid/parsing/grid-shorthand.html

Created attachment 447446 [details]
Patch

Created attachment 447449 [details]
Patch

[Radar WebKit Bug Importer]

<rdar://problem/86880897>

Can I have a review on this patch please? Many thanks!

[Sergio Villar Senin]

Comment on attachment 447449 [details]
Patch

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

> Source/WebCore/style/StyleBuilderConverter.h:1166
> +        ASSERT(downcast<CSSPrimitiveValue>(value).isValueID());

A bit weird to have a if for just an ASSERT. I'd better do

ASSERT(!is<CSSPrimitiveValue>(value) || downcast<CSSPrimitiveValue>(value).isValueID());

> Source/WebCore/style/StyleBuilderConverter.h:1171
>          return RenderStyle::initialGridAutoFlow();

Seems like you have to indent this return

> Source/WebCore/style/StyleBuilderConverter.h:1175
> +    auto* second = downcast<CSSPrimitiveValue>(is<CSSValueList>(value) && downcast<CSSValueList>(value).length() == 2 ? downcast<CSSValueList>(value).item(1) : nullptr);

Looks like we could store is<CSSValueList>(value) in a boolean since it's also used in the if block above.

Created attachment 449154 [details]
Patch

Created attachment 449155 [details]
Patch

[Sergio Villar Senin]

Comment on attachment 449155 [details]
Patch

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

> Source/cmake/OptionsGTK.cmake:65
> +#WEBKIT_OPTION_DEFINE(USE_JPEGXL "Whether to enable support for JPEG-XL images." PUBLIC ${ENABLE_EXPERIMENTAL_FEATURES})

I think you mistakenly changed this

Created attachment 449316 [details]
Patch

Created attachment 449317 [details]
Patch

[EWS]

Committed r288544 (246376@main): <https://commits.webkit.org/246376@main>

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