Bug 219741

Summary: Support aspect-ratio on positioned elements
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, simon.fraser, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 47738    
Attachments:
Description Flags
Patch
ews-feeder: commit-queue-
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

Description Rob Buis 2020-12-10 07:15:42 PST 
Support aspect-ratio on positioned elements
Comment 1 Rob Buis 2020-12-10 07:18:30 PST 
Created attachment 415872 [details]
Patch
Comment 2 Rob Buis 2020-12-10 10:16:36 PST 
Created attachment 415893 [details]
Patch
Comment 3 Rob Buis 2020-12-11 07:22:27 PST 
Created attachment 415994 [details]
Patch
Comment 4 Rob Buis 2020-12-11 11:59:11 PST 
Created attachment 416024 [details]
Patch
Comment 5 Rob Buis 2020-12-11 13:38:34 PST 
Created attachment 416038 [details]
Patch
Comment 6 Radar WebKit Bug Importer 2020-12-17 07:16:23 PST 
<rdar://problem/72425425>
Comment 7 Simon Fraser (smfr) 2020-12-22 11:12:53 PST 
Comment on attachment 416038 [details]
Patch

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

> Source/WebCore/rendering/RenderBox.cpp:5011
> +    if (style().width().isAuto() && style().height().isAuto() && !style().top().isAuto() && !style().bottom().isAuto() && (style().left().isAuto() || style().right().isAuto()))

Maybe all these could become a helper on RenderStyle.

> Source/WebCore/rendering/RenderBox.cpp:5024
> +    LayoutUnit containerWidthIninlineDirection = std::max<LayoutUnit>(0, containingBlockLogicalWidthForContentInFragment(fragment));

containerWidthInIInlineDirection
Comment 8 Rob Buis 2020-12-22 11:40:31 PST 
Created attachment 416678 [details]
Patch
Comment 9 EWS 2020-12-22 12:30:50 PST 
Committed r271061: <https://trac.webkit.org/changeset/271061>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 416678 [details].
Comment 10 zalan 2021-03-31 20:35:32 PDT 
Comment on attachment 416678 [details]
Patch

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

> Source/WebCore/rendering/RenderBox.cpp:5013
> +bool RenderBox::shouldComputeLogicalWidthFromAspectRatio() const

What does inset mean in this context and how would a caller know if shouldComputeLogicalHeightFromAspectRatio() or shouldComputeLogicalWidthFromAspectRatioAndInsets() is the right function to call.
Comment 11 Rob Buis 2021-04-01 08:42:46 PDT 
Comment on attachment 416678 [details]
Patch

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

>> Source/WebCore/rendering/RenderBox.cpp:5013
>> +bool RenderBox::shouldComputeLogicalWidthFromAspectRatio() const
> 
> What does inset mean in this context and how would a caller know if shouldComputeLogicalHeightFromAspectRatio() or shouldComputeLogicalWidthFromAspectRatioAndInsets() is the right function to call.

Inset as defined here https://drafts.csswg.org/css-logical/#inset-properties.
shouldComputeLogicalWidthFromAspectRatioAndInsets is more an implementation detail of shouldComputeLogicalWidthFromAspectRatio/shouldComputeLogicalHeightFromAspectRatio, so maybe it would have been better to keep it private.
Comment 12 zalan 2021-04-01 13:14:49 PDT 
(In reply to Rob Buis from comment #11)
> Comment on attachment 416678 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=416678&action=review
> 
> >> Source/WebCore/rendering/RenderBox.cpp:5013
> >> +bool RenderBox::shouldComputeLogicalWidthFromAspectRatio() const
> > 
> > What does inset mean in this context and how would a caller know if shouldComputeLogicalHeightFromAspectRatio() or shouldComputeLogicalWidthFromAspectRatioAndInsets() is the right function to call.
> 
> Inset as defined here https://drafts.csswg.org/css-logical/#inset-properties.
Awesome! Thanks, apparently I am super behind on css-logical. :(