Bug 220970 – Handle zero aspect-ratio width/height

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

[Rob Buis]

Handle zero aspect-ratio width/height

[Rob Buis]

Created attachment 418389 [details]
Patch

[Rob Buis]

Created attachment 418413 [details]
Patch

[Rob Buis]

Created attachment 418464 [details]
Patch

[Manuel Rego Casasnovas]

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?

[Rob Buis]

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.

[Rob Buis]

Created attachment 418520 [details]
Patch

[Rob Buis]

Created attachment 418524 [details]
Patch

[Manuel Rego Casasnovas]

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.

[Rob Buis]

Created attachment 418531 [details]
Patch

[EWS]

Committed r271948: <https://trac.webkit.org/changeset/271948>

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

[Radar WebKit Bug Importer]

<rdar://problem/73659916>