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: | P2 | Keywords: | InRadar | ||||||||||||
Version: | Other | ||||||||||||||
Hardware: | PC | ||||||||||||||
OS: | Linux | ||||||||||||||
See Also: | https://github.com/web-platform-tests/wpt/pull/32205 | ||||||||||||||
Bug Depends on: | 235102 | ||||||||||||||
Bug Blocks: | |||||||||||||||
Attachments: |
|
Description
Pedro Paulo
2021-12-21 08:21:01 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. (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. Created attachment 448041 [details]
Patch
Note that I'm going to include the test I uploaded for review to the WPT repo mentioned in the "See Also" field. Created attachment 448729 [details]
Patch
Created attachment 448952 [details]
Patch
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? 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! 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! Created attachment 449032 [details]
Patch
Comment on attachment 449032 [details]
Patch
r=me
Committed r287976 (246005@trunk): <https://commits.webkit.org/246005@trunk> |