Bug 228076

Summary:

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

Product:

WebKit

Reporter:

Ian Stewart <ian.c.stewart>

Component:

Layout and Rendering

Assignee:

cathiechen <cathiechen>

Status:

RESOLVED FIXED

Severity:

Major

CC:

bfulgham, cathiechen, changseok, esprehn+autocc, ews-watchlist, glenn, jonlee, koivisto, kondapallykalyan, pdr, rbuis, sabouhallawa, simon.fraser, svillar, webkit-bug-importer, zalan, zsun

Priority:

Keywords:

InRadar

Version:

Safari Technology Preview

Hardware:

Mac (Intel)

OS:

macOS 11

URL:

https://gashouder.nl/partners/

See Also:

https://bugs.webkit.org/show_bug.cgi?id=228872

Bug Depends on:

228734

Bug Blocks:

Attachments:

Description	Flags
safari 14 & safari 15 side by side showing the image stretching problem	none
test-case	none
Possible WPT test	none
test-case	none
Patch	none
Patch	none
Patch	none

Ian Stewart

Reported 2021-07-19 02:37:29 PDT

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

Attachments
safari 14 & safari 15 side by side showing the image stretching problem (1.52 MB, image/png) 2021-07-19 02:37 PDT, Ian Stewart	no flags	Details
test-case (393 bytes, text/html) 2021-07-23 02:01 PDT, cathiechen	no flags	Details
Possible WPT test (606 bytes, text/html) 2021-07-23 05:51 PDT, Rob Buis	no flags	Details
test-case (572 bytes, text/html) 2021-07-23 19:02 PDT, cathiechen	no flags	Details
Patch (7.22 KB, patch) 2021-08-02 11:16 PDT, cathiechen	no flags	Details Formatted Diff Diff
Patch (8.08 KB, patch) 2021-08-03 11:21 PDT, cathiechen	no flags	Details Formatted Diff Diff
Patch (8.08 KB, patch) 2021-08-04 01:42 PDT, cathiechen	no flags	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Radar WebKit Bug Importer

Comment 1 2021-07-19 07:48:24 PDT

<rdar://problem/80783648>

Simon Fraser (smfr)

Comment 2 2021-07-19 11:07:13 PDT

Caused by r277997.

cathiechen

Comment 3 2021-07-20 06:13:35 PDT

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

Jon Lee

Comment 4 2021-07-20 15:20:05 PDT

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

cathiechen

Comment 5 2021-07-23 02:01:20 PDT

Created attachment 434076 [details] test-case

Rob Buis

Comment 6 2021-07-23 05:51:13 PDT

Created attachment 434082 [details] Possible WPT test

cathiechen

Comment 7 2021-07-23 19:02:29 PDT

Created attachment 434155 [details] test-case Added a case that img uses aspect-ratio directly.

Jon Lee

Comment 8 2021-07-27 12:03:42 PDT

any update on the investigation?

cathiechen

Comment 9 2021-07-27 19:28:12 PDT

(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

Comment 10 2021-08-02 11:16:57 PDT

Created attachment 434775 [details] Patch

cathiechen

Comment 11 2021-08-03 11:21:10 PDT

Created attachment 434842 [details] Patch

cathiechen

Comment 12 2021-08-03 11:30:40 PDT

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

Comment 13 2021-08-04 01:41:39 PDT

Hi Antti, Thanks for the review!

cathiechen

Comment 14 2021-08-04 01:42:49 PDT

Created attachment 434893 [details] Patch

EWS

Comment 15 2021-08-04 03:57:16 PDT

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 16 2021-08-04 08:40:48 PDT

Comment on attachment 434893 [details] Patch It's a fake warning.

Note You need to log in before you can comment on or make changes to this bug.