Bug 221328

Summary: Make shouldComputeLogicalWidthFromAspectRatioAndInsets writing mode aware
Product: WebKit Reporter: Rob Buis <rbuis>
Component: CSSAssignee: Rob Buis <rbuis>
Status: RESOLVED FIXED    
Severity: Normal CC: changseok, esprehn+autocc, ews-watchlist, glenn, kondapallykalyan, 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 Flags
Patch none

Description Rob Buis 2021-02-03 06:09:22 PST
Commit r271061 added support for aspect-ratio on positioned elements, taking into account
insets, but it failed to take writing-mode into account when checking insets. This change fixes
that by asking logical values.
Comment 1 Rob Buis 2021-02-03 06:11:49 PST
Created attachment 419128 [details]
Patch
Comment 2 Manuel Rego Casasnovas 2021-02-04 00:57:01 PST
Comment on attachment 419128 [details]
Patch

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

r=me, with a nit comment inline.

> Source/WebCore/rendering/RenderBox.cpp:5100
> +    if (style().width().isAuto() && style().height().isAuto() && !style().logicalTop().isAuto() && !style().logicalBottom().isAuto() && (style().logicalLeft().isAuto() || style().logicalRight().isAuto()))

What about logicalWidth() instead of width()? I guess it doesn't really matter as we have an and for width and height, but maybe it'd be worth changing for consistency dunno.
Comment 3 Rob Buis 2021-02-04 01:00:06 PST
Comment on attachment 419128 [details]
Patch

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

>> Source/WebCore/rendering/RenderBox.cpp:5100
>> +    if (style().width().isAuto() && style().height().isAuto() && !style().logicalTop().isAuto() && !style().logicalBottom().isAuto() && (style().logicalLeft().isAuto() || style().logicalRight().isAuto()))
> 
> What about logicalWidth() instead of width()? I guess it doesn't really matter as we have an and for width and height, but maybe it'd be worth changing for consistency dunno.

Interesting idea, but ultimately we need to check both dimensions and using the logical variants is a bit slower. So I'll keep as-is.
Comment 4 EWS 2021-02-04 01:11:34 PST
Committed r272363: <https://trac.webkit.org/changeset/272363>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 419128 [details].
Comment 5 Radar WebKit Bug Importer 2021-02-04 01:12:14 PST
<rdar://problem/73970893>