Bug 219741 - Support aspect-ratio on positioned elements
Summary: Support aspect-ratio on positioned elements
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Rob Buis
URL:
Keywords: InRadar
Depends on:
Blocks: 47738
  Show dependency treegraph
 
Reported: 2020-12-10 07:15 PST by Rob Buis
Modified: 2021-04-01 13:14 PDT (History)
9 users (show)

See Also:


Attachments
Patch (6.90 KB, patch)
2020-12-10 07:18 PST, Rob Buis
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (8.94 KB, patch)
2020-12-10 10:16 PST, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (9.77 KB, patch)
2020-12-11 07:22 PST, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (10.11 KB, patch)
2020-12-11 11:59 PST, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (10.26 KB, patch)
2020-12-11 13:38 PST, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (10.36 KB, patch)
2020-12-22 11:40 PST, Rob Buis
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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. :(