Bug 220970

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:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

47738

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none

Rob Buis

Reported 2021-01-26 03:43:12 PST

Handle zero aspect-ratio width/height

Attachments
Patch (1.97 KB, patch) 2021-01-26 03:48 PST, Rob Buis	no flags	Details Formatted Diff Diff
Patch (3.62 KB, patch) 2021-01-26 06:55 PST, Rob Buis	no flags	Details Formatted Diff Diff
Patch (6.31 KB, patch) 2021-01-26 12:38 PST, Rob Buis	no flags	Details Formatted Diff Diff
Patch (6.72 KB, patch) 2021-01-27 03:08 PST, Rob Buis	no flags	Details Formatted Diff Diff
Patch (7.44 KB, patch) 2021-01-27 03:41 PST, Rob Buis	no flags	Details Formatted Diff Diff
Patch (7.54 KB, patch) 2021-01-27 05:03 PST, Rob Buis	no flags	Details Formatted Diff Diff
Show Obsolete (5) View All Add attachment proposed patch, testcase, etc.

Rob Buis

Comment 1 2021-01-26 03:48:10 PST

Created attachment 418389 [details] Patch

Rob Buis

Comment 2 2021-01-26 06:55:44 PST

Created attachment 418413 [details] Patch

Rob Buis

Comment 3 2021-01-26 12:38:45 PST

Created attachment 418464 [details] Patch

Manuel Rego Casasnovas

Comment 4 2021-01-26 13:17:45 PST

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 5 2021-01-26 13:24:44 PST

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

Comment 6 2021-01-27 03:08:38 PST

Created attachment 418520 [details] Patch

Rob Buis

Comment 7 2021-01-27 03:41:10 PST

Created attachment 418524 [details] Patch

Manuel Rego Casasnovas

Comment 8 2021-01-27 04:07:19 PST

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

Comment 9 2021-01-27 05:03:21 PST

Created attachment 418531 [details] Patch

EWS

Comment 10 2021-01-27 06:14:03 PST

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

Comment 11 2021-01-27 06:15:14 PST

<rdar://problem/73659916>

Note You need to log in before you can comment on or make changes to this bug.