Bug 234564

Summary:

[css-flexbox] Incorrect height of flex items with aspect-ratio whenever the cross axis intrinsic size is larger than the viewport

Product:

WebKit

Reporter:

Pedro Paulo <pedropaulosuzuki1>

Component:

Images

Assignee:

Sergio Villar Senin <svillar>

Status:

RESOLVED FIXED

Severity:

Major

CC:

709922234, changseok, esprehn+autocc, ews-watchlist, glenn, graouts, jfernandez, jonlee, koivisto, kondapallykalyan, mrobinson, pdr, rbuis, rego, sabouhallawa, simon.fraser, svillar, webkit-bug-importer, webkit-layout-noreply, zalan

Priority:

Keywords:

InRadar

Version:

Other

Hardware:

OS:

Linux

See Also:

https://github.com/web-platform-tests/wpt/pull/32205

Bug Depends on:

235102

Bug Blocks:

Attachments:

Description	Flags
Minimal HTML file which reproduces the bug	none
Patch	none
Patch	none
Patch	none
Patch	rego: review+

Pedro Paulo

Reported 2021-12-21 08:21:01 PST

Created attachment 447717 [details] Minimal HTML file which reproduces the bug Hey! Hello everyone! I was testing one of my websites on GNOME Web and found out that there was a major bug on the <img> tag when it is a child of a column-flexed flex element. The image gets the correct width, but the height does not follow the aspect ratio as it should, pushing the original image height instead, which generates problems for alignment. It can be fixed by putting the actual height you want, but that doesn't work on some cases (like on my website) where the width of the parent is dynamic, and it is non-trivial to calculate the correct height there without cluttering the website with Javascript resizes, which is also not recommended. It works fine on both Firefox and Chromium-based browsers, so it seems to be a Webkit glitch. I later tested the issue on a friend's iPhone on Safari and the glitch happened there too. ## Minimal example: ```html <html> <body> <div style="display: flex;flex-direction: column;"> <img src="https://upload.wikimedia.org/wikipedia/commons/8/8f/Artix_Linux_Logo.svg"> </div> </body> </html> ``` Tested on Epiphany (GNOME Web) 41.0 Flatpak (latest on Flathub) and on a friend's iPhone (don't know which version, but probably up to date).

Attachments
Minimal HTML file which reproduces the bug (183 bytes, text/html) 2021-12-21 08:21 PST, Pedro Paulo	no flags	Details
Patch (7.58 KB, patch) 2021-12-28 03:44 PST, Sergio Villar Senin	no flags	Details Formatted Diff Diff
Patch (9.52 KB, patch) 2022-01-10 02:46 PST, Sergio Villar Senin	no flags	Details Formatted Diff Diff
Patch (10.66 KB, patch) 2022-01-12 09:18 PST, Sergio Villar Senin	no flags	Details Formatted Diff Diff
Patch (10.15 KB, patch) 2022-01-13 00:47 PST, Sergio Villar Senin	rego: review+	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Sergio Villar Senin

Comment 1 2021-12-22 00:36:56 PST

(In reply to Pedro Paulo from comment #0) > Created attachment 447717 [details] > Minimal HTML file which reproduces the bug > > Hey! Hello everyone! > > I was testing one of my websites on GNOME Web and found out that there was a > major bug on the <img> tag when it is a child of a column-flexed flex > element. The image gets the correct width, but the height does not follow > the aspect ratio as it should, pushing the original image height instead, > which generates problems for alignment. It can be fixed by putting the > actual height you want, but that doesn't work on some cases (like on my > website) where the width of the parent is dynamic, and it is non-trivial to > calculate the correct height there without cluttering the website with > Javascript resizes, which is also not recommended. It works fine on both > Firefox and Chromium-based browsers, so it seems to be a Webkit glitch. I > later tested the issue on a friend's iPhone on Safari and the glitch > happened there too. > > ## Minimal example: > ```html > <html> > <body> > <div style="display: flex;flex-direction: column;"> > <img > src="https://upload.wikimedia.org/wikipedia/commons/8/8f/Artix_Linux_Logo. > svg"> > </div> > </body> > </html> > ``` > > Tested on Epiphany (GNOME Web) 41.0 Flatpak (latest on Flathub) and on a > friend's iPhone (don't know which version, but probably up to date). Thanks for the report. Please, avoid things like "major bug" or the like in the bug title because it does not help triagging. It'd be much useful to try to describe the bug, like "aspect ratio of SVG not working on column flexboxes" or whatever you like. The question I have is, did it used to work at some point? I just basically want to know whether it's a recent regression (there were recent changes) or something that has been around for a while.

Pedro Paulo

Comment 2 2021-12-22 17:35:55 PST

(In reply to Sergio Villar Senin from comment #1) > (In reply to Pedro Paulo from comment #0) > > Created attachment 447717 [details] > > Minimal HTML file which reproduces the bug > > > > Hey! Hello everyone! > > > > I was testing one of my websites on GNOME Web and found out that there was a > > major bug on the <img> tag when it is a child of a column-flexed flex > > element. The image gets the correct width, but the height does not follow > > the aspect ratio as it should, pushing the original image height instead, > > which generates problems for alignment. It can be fixed by putting the > > actual height you want, but that doesn't work on some cases (like on my > > website) where the width of the parent is dynamic, and it is non-trivial to > > calculate the correct height there without cluttering the website with > > Javascript resizes, which is also not recommended. It works fine on both > > Firefox and Chromium-based browsers, so it seems to be a Webkit glitch. I > > later tested the issue on a friend's iPhone on Safari and the glitch > > happened there too. > > > > ## Minimal example: > > ```html > > <html> > > <body> > > <div style="display: flex;flex-direction: column;"> > > <img > > src="https://upload.wikimedia.org/wikipedia/commons/8/8f/Artix_Linux_Logo. > > svg"> > > </div> > > </body> > > </html> > > ``` > > > > Tested on Epiphany (GNOME Web) 41.0 Flatpak (latest on Flathub) and on a > > friend's iPhone (don't know which version, but probably up to date). > > Thanks for the report. > > Please, avoid things like "major bug" or the like in the bug title because > it does not help triagging. It'd be much useful to try to describe the bug, > like "aspect ratio of SVG not working on column flexboxes" or whatever you > like. > > The question I have is, did it used to work at some point? I just basically > want to know whether it's a recent regression (there were recent changes) or > something that has been around for a while. Oh! Sorry about that. My mistake. I encountered it while I was testing my website recently, but I found someone relating a similar issue back in 2019 or something, but I cannot find it here. Oh, and it seems to also happen with regular rasterized (non-SVG, such as jpeg and png files) too, but they get stretched instead.

Sergio Villar Senin

Comment 3 2021-12-28 03:44:04 PST

Created attachment 448041 [details] Patch

Sergio Villar Senin

Comment 4 2021-12-28 03:45:32 PST

Note that I'm going to include the test I uploaded for review to the WPT repo mentioned in the "See Also" field.

Radar WebKit Bug Importer

Comment 5 2021-12-28 08:21:15 PST

<rdar://problem/86958389>

Sergio Villar Senin

Comment 6 2022-01-10 02:46:23 PST

Created attachment 448729 [details] Patch

Sergio Villar Senin

Comment 7 2022-01-12 09:18:16 PST

Created attachment 448952 [details] Patch

Manuel Rego Casasnovas

Comment 8 2022-01-12 13:32:39 PST

Comment on attachment 448952 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=448952&action=review > Source/WebCore/rendering/RenderFlexibleBox.cpp:996 > +bool RenderFlexibleBox::childCrossSizeShouldUseContainerCrossSize(const RenderBox& child, SizeType sizeType) const Why are you adding the sizeType as argument, if it's always the default one?

Sergio Villar Senin

Comment 9 2022-01-13 00:44:37 PST

Comment on attachment 448952 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=448952&action=review >> Source/WebCore/rendering/RenderFlexibleBox.cpp:996 >> +bool RenderFlexibleBox::childCrossSizeShouldUseContainerCrossSize(const RenderBox& child, SizeType sizeType) const > > Why are you adding the sizeType as argument, if it's always the default one? Ups, a leftover from a previous version of the patch. I'll remove it. Good catch!

Sergio Villar Senin

Comment 10 2022-01-13 00:44:39 PST

Sergio Villar Senin

Comment 11 2022-01-13 00:47:05 PST

Created attachment 449032 [details] Patch

Manuel Rego Casasnovas

Comment 12 2022-01-13 01:03:21 PST

Comment on attachment 449032 [details] Patch r=me

Sergio Villar Senin

Comment 13 2022-01-13 05:04:13 PST

Committed r287976 (246005@trunk): <https://commits.webkit.org/246005@trunk>

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