Bug 228076 – REGRESSION (r277997) Images get stretched with aspect-ratio and max-width: x%

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Ian Stewart]

Created attachment 433781 [details]
safari 14 & safari 15 side by side showing the image stretching problem

When making an image element grow to its parent element aspect ratio seems to win from width, height => auto & max-width, -height => 100%.

This results in a stretched image in safari 15 (Technology Preview 127) while in safari 14 the image sizes correctly.

**not sure if this is a webkit bug or just a css styling mistake regarding the new css specs

[Radar WebKit Bug Importer]

<rdar://problem/80783648>

[Simon Fraser (smfr)]

Caused by r277997.

[cathiechen]

Thanks for reporting this, I'll take a look today!

[Jon Lee]

The attached png has loaded https://gashouder.nl/partners/

[cathiechen]

Created attachment 434076 [details]
test-case

[Rob Buis]

Created attachment 434082 [details]
Possible WPT test

[cathiechen]

Created attachment 434155 [details]
test-case

Added a case that img uses aspect-ratio directly.

[Jon Lee]

any update on the investigation?

[cathiechen]

(In reply to Jon Lee from comment #8)
> any update on the investigation?

Yeah, there's some progress, but still looking for an approach.

This issue seems related to flex + img with aspect-ratio.

* In RenderFlexibleBox::setOverridingMainSizeForChild, the img's OverridingLogicalHeight is generated by img's aspect-ratio * FlexibleBox's width.

* Then we use OverridingLogicalHeight to calculate img's logical width in RenderReplaced::computeReplacedLogicalWidth.

The value of width is correct until now.

* But in RenderFlexibleBox::applyStretchAlignmentToChild which stretches child in the cross Axis, the logical width is changed to the intrinsic size because it has aspect-ratio and OverridingLogicalHeight.

Using the intrinsic size is not right, OverridingLogicalHeight is from FlexibleBox's width, not the intrinsic size.

[cathiechen]

Created attachment 434775 [details]
Patch

[cathiechen]

Created attachment 434842 [details]
Patch

[cathiechen]

Hi,
I think this patch is ready for review now.
Please take a look, thanks:)

The debugging story is long, it ends up with constrainLogicalWidthInFragmentByMinMax:)

> This issue seems related to flex + img with aspect-ratio.
> 
> * In RenderFlexibleBox::setOverridingMainSizeForChild, the img's
> OverridingLogicalHeight is generated by img's aspect-ratio * FlexibleBox's
> width.
> 
> * Then we use OverridingLogicalHeight to calculate img's logical width in
> RenderReplaced::computeReplacedLogicalWidth.
> 
> The value of width is correct until now.
> 
> * But in RenderFlexibleBox::applyStretchAlignmentToChild which stretches
> child in the cross Axis, the logical width is changed to the intrinsic size
> because it has aspect-ratio and OverridingLogicalHeight.

The image get stretched because constrainLogicalWidthInFragmentByMinMax returns the intrinsic width while computing MinSize.

[cathiechen]

Hi Antti,
Thanks for the review!

[cathiechen]

Created attachment 434893 [details]
Patch

[EWS]

Committed r280631 (240244@main): <https://commits.webkit.org/240244@main>

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

[cathiechen]

Comment on attachment 434893 [details]
Patch

It's a fake warning.