Bug 219741 – Support aspect-ratio on positioned elements

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

[Rob Buis]

Support aspect-ratio on positioned elements

[Rob Buis]

Created attachment 415872 [details]
Patch

[Rob Buis]

Created attachment 415893 [details]
Patch

[Rob Buis]

Created attachment 415994 [details]
Patch

[Rob Buis]

Created attachment 416024 [details]
Patch

[Rob Buis]

Created attachment 416038 [details]
Patch

[Radar WebKit Bug Importer]

<rdar://problem/72425425>

[Simon Fraser (smfr)]

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

[Rob Buis]

Created attachment 416678 [details]
Patch

[EWS]

Committed r271061: <https://trac.webkit.org/changeset/271061>

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

[zalan]

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.

[Rob Buis]

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.

[zalan]

(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. :(