Summary: | Handle zero aspect-ratio width/height | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Rob Buis <rbuis> | ||||||||||||||
Component: | CSS | Assignee: | Rob Buis <rbuis> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | changseok, esprehn+autocc, ews-watchlist, glenn, gyuyoung.kim, kondapallykalyan, macpherson, menard, pdr, rego, webkit-bug-importer | ||||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||
OS: | Unspecified | ||||||||||||||||
Bug Depends on: | |||||||||||||||||
Bug Blocks: | 47738 | ||||||||||||||||
Attachments: |
|
Description
Rob Buis
2021-01-26 03:43:12 PST
Created attachment 418389 [details]
Patch
Created attachment 418413 [details]
Patch
Created attachment 418464 [details]
Patch
Comment on attachment 418464 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=418464&action=review > Source/WebCore/ChangeLog:9 > + as auto, but when serializing keep the original input. Are there any tests for the serialization? > Source/WebCore/css/CSSComputedStyleDeclaration.cpp:3475 > + if (style.aspectRatioWidth() > 0 && style.aspectRatioHeight() > 0) > + return cssValuePool.createIdentifierValue(CSSValueAuto); I'm not sure I get this, why if width or height is bigger than zero we return auto? Comment on attachment 418464 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=418464&action=review >> Source/WebCore/ChangeLog:9 >> + as auto, but when serializing keep the original input. > > Are there any tests for the serialization? Yes, it is tested by css/css-sizing/aspect-ratio/parsing/contain-intrinsic-size-computed.html. >> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:3475 >> + return cssValuePool.createIdentifierValue(CSSValueAuto); > > I'm not sure I get this, why if width or height is bigger than zero we return auto? Yes upon reflection this logic is not very clear and it think it may break. I will add a new enum value tomorrow, this should make the code clearer and not error prone. Created attachment 418520 [details]
Patch
Created attachment 418524 [details]
Patch
Comment on attachment 418524 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=418524&action=review r=me, let's wait for EWSs output, but the change looks better now. Thanks. > Source/WebCore/style/StyleBuilderCustom.h:1189 > builderState.style().setAspectRatio(downcast<CSSPrimitiveValue>(list.item(0))->doubleValue(), downcast<CSSPrimitiveValue>(list.item(1))->doubleValue()); Nit: Please use here width and height. Created attachment 418531 [details]
Patch
Committed r271948: <https://trac.webkit.org/changeset/271948> All reviewed patches have been landed. Closing bug and clearing flags on attachment 418531 [details]. |