| Summary: | [css-grid] Fix grid shorthand expansion of initial values | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | zsun | ||||||||||||||
| Component: | CSS | Assignee: | zsun | ||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||
| Severity: | Normal | CC: | annulen, esprehn+autocc, ews-watchlist, glenn, gyuyoung.kim, jfernandez, macpherson, menard, rego, ryuan.choi, sergio, svillar, webkit-bug-importer | ||||||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||||||
| Version: | WebKit Nightly Build | ||||||||||||||||
| Hardware: | Unspecified | ||||||||||||||||
| OS: | Unspecified | ||||||||||||||||
| Attachments: |
|
||||||||||||||||
|
Description
zsun
2021-12-17 02:58:12 PST
Created attachment 447446 [details]
Patch
Created attachment 447449 [details]
Patch
Can I have a review on this patch please? Many thanks! 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
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
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]. |