Bug 221110

Summary: REGRESSION(r270578): Incorrect layout for the SVG image inside a flex container
Product: WebKit Reporter: Dima Tisnek <dimaqq>
Component: ImagesAssignee: Sergio Villar Senin <svillar>
Status: RESOLVED FIXED    
Severity: Normal CC: jonlee, sabouhallawa, smoley, svillar, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Safari Technology Preview   
Hardware: Unspecified   
OS: macOS 11   
See Also: https://bugs.webkit.org/show_bug.cgi?id=221234
https://bugs.webkit.org/show_bug.cgi?id=221450
Bug Depends on: 221484    
Bug Blocks:    
Attachments:
Description Flags
Screenshot w Safari and Safari TP
none
test case
none
test case
none
test case none

Description Dima Tisnek 2021-01-28 16:38:25 PST 
Created attachment 418680 [details]
Screenshot w Safari and Safari TP

Test: https://codepen.io/dimaqq/pen/MWbWWqN

Safari, Chrome, Firefox all display the ellipsis correctly (14x52px)
Safari TP 🧻 (Release 119 (Safari 14.1, WebKit 16611.1.10.1)) displays a huge ellipsis (243x52px)
Thus, I think it's a regression.

The SVG has this content:

<svg width="14" height="3" viewBox="0 0 14 3" fill="none" xmlns="http://www.w3.org/2000/svg">
<path d="M11.8393 3C12.6677 3 13.3393 2.32843 13.3393 1.5C13.3393 0.671573 12.6677 0 11.8393 0C11.0109 0 10.3393 0.671573 10.3393 1.5C10.3393 2.32843 11.0109 3 11.8393 3Z" fill="white"/>
<path d="M7.13922 3C7.96765 3 8.63922 2.32843 8.63922 1.5C8.63922 0.671573 7.96765 0 7.13922 0C6.31079 0 5.63922 0.671573 5.63922 1.5C5.63922 2.32843 6.31079 3 7.13922 3Z" fill="white"/>
<path d="M2.43927 3C3.2677 3 3.93927 2.32843 3.93927 1.5C3.93927 0.671573 3.2677 0 2.43927 0C1.61084 0 0.93927 0.671573 0.93927 1.5C0.93927 2.32843 1.61084 3 2.43927 3Z" fill="white"/>
</svg>

So... given an image 14x3px that needs to go into a 52px heigh div:
prod browsers make a block 14x52px and the image in it (visually) 14x3px (I expected 14x3 element with margin, but whatever)
and safari tp makes a block NNNx52px and the image in it visually NNNx52px.

P.S. I wish I could run the nightly build, but I'm not sure what version `run-webkit-archive` actually runs when every included framework trips on unsigned binary :(
Comment 1 Radar WebKit Bug Importer 2021-01-28 19:06:41 PST 
<rdar://problem/73735772>
Comment 2 Dima Tisnek 2021-01-28 20:32:15 PST 
I tried to dig a bit further and here's a smaller reproducer, with a regular, external image: https://codepen.io/dimaqq/pen/XWNWmym
Comment 3 Smoley 2021-02-02 18:13:56 PST 
Thanks for filing, this does not reproduce for me on macOS Big Sur 11.2 RC 3 (20D64) using stock Safari 14.0.3, but it does reproduce on STP119 and TOT.
Comment 4 Jon Lee 2021-02-03 16:17:22 PST 
This seems to be fallout from r270578
Comment 5 Said Abou-Hallawa 2021-02-03 17:10:15 PST 
Created attachment 419210 [details]
test case

Attaching a more simplified and more readable test case. The green rectangle to should be 10x10. But with the trunk webkit it is always 100x100 regardless how big the size of the <svg> image is.
Comment 6 Said Abou-Hallawa 2021-02-03 18:47:54 PST 
The behavior of RenderFlexibleBox::childCrossSizeIsDefinite() has changed when the child is a RenderImage and the length is auto. Instead of returning false we are now returning true. This make RenderFlexibleBox::useChildAspectRatio() returns true also which makes RenderFlexibleBox::computeInnerFlexBaseSizeForChild() call adjustChildSizeForAspectRatioCrossAxisMinAndMax(). This function returns the size of the RenderFlexibleBox and ignores the childIntrinsicSize which is the size of the image.
Comment 7 Said Abou-Hallawa 2021-02-03 19:22:52 PST 
Created attachment 419215 [details]
test case

The same bug happens with binary images.
Comment 8 Said Abou-Hallawa 2021-02-03 21:36:19 PST 
Created attachment 419229 [details]
test case
Comment 9 Sergio Villar Senin 2021-02-04 01:56:48 PST 
Checking...
Comment 10 Sergio Villar Senin 2021-02-05 04:43:43 PST 
I've a patch that fixes this issue. The original patch was not totally correct but it helped to unveil some other already present issues. This means that in order to fix it properly I'll do the following:

1. Import the most recent versions of WPT flexbox tests (we have in tree some of them that are invalid and that will begin to fail as soon as my patches land)

2. Fix content size suggestion computation for aspect ratio items which is not correct (will fill another bug)

3. Fix the original patch
Comment 11 Sergio Villar Senin 2021-02-05 09:53:08 PST 
Test import ready for review in bug 221484
Comment 12 Sergio Villar Senin 2021-02-11 12:03:33 PST 
Submitted a couple of new tests to WPT based on the 3 test cases mentioned in this bug report https://github.com/web-platform-tests/wpt/pull/27596
Comment 13 Said Abou-Hallawa 2021-02-11 15:34:05 PST 
Given the slow progress in fixing this bug and to have the enough time to fix it properly, I have reverted r270578.

Committed r272755: <https://commits.webkit.org/r272755>
Comment 14 Sergio Villar Senin 2021-02-12 01:02:56 PST 
(In reply to Said Abou-Hallawa from comment #13)
> Given the slow progress in fixing this bug and to have the enough time to
> fix it properly, I have reverted r270578.
> 
> Committed r272755: <https://commits.webkit.org/r272755>

That's unfortunate. IMO the progress was not slow. The patch is ready, I had to wait for the review of bug 221484 and now I'm waiting for https://github.com/web-platform-tests/wpt/pull/27596. Once that last one lands I could upload the patch.