[webkit-reviews] review granted: [Bug 243209] Fix aspect-ratio/flex-aspect-ratio-031.html : [Attachment 461247] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Rob Buis <rbuis at igalia.com>'s request
for review:
Bug 243209: Fix aspect-ratio/flex-aspect-ratio-031.html
https://bugs.webkit.org/show_bug.cgi?id=243209

Attachment 461247: Patch

https://bugs.webkit.org/attachment.cgi?id=461247&action=review




--- Comment #4 from Darin Adler <darin at apple.com> ---
Comment on attachment 461247
  --> https://bugs.webkit.org/attachment.cgi?id=461247
Patch

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

> Source/WebCore/rendering/RenderFlexibleBox.cpp:930
> +	   std::optional<LayoutUnit> childSize =
mainAxisIsChildInlineAxis(child) ?
child.computePercentageLogicalHeight(crossSizeLength) :
adjustBorderBoxLogicalWidthForBoxSizing(valueForLength(crossSizeLength,
contentWidth()), crossSizeLength.type());

Slightly better if we just use auto here, I think.

> Source/WebCore/rendering/RenderFlexibleBox.cpp:947
> +	       if (child.style().boxSizingForAspectRatio() ==
BoxSizing::ContentBox)
> +		   crossSize -= isHorizontalFlow() ?
child.verticalBorderAndPaddingExtent() :
child.horizontalBorderAndPaddingExtent();
> +	       else
> +		   borderAndPadding = isHorizontalFlow() ?
child.horizontalBorderAndPaddingExtent() :
child.verticalBorderAndPaddingExtent();

Do we have enough test cases that we know we got all 4 of these cases right?

> Source/WebCore/rendering/RenderFlexibleBox.cpp:955
> +	   return std::max(LayoutUnit(), LayoutUnit(crossSize * ratio) -
borderAndPadding);

Could consider 0_lu instead of LayoutUnit().