Bug 221110 - REGRESSION(r270578): Incorrect layout for the SVG image inside a flex container
Summary: REGRESSION(r270578): Incorrect layout for the SVG image inside a flex container
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Images (show other bugs)
Version: Safari Technology Preview
Hardware: Unspecified macOS 11
: P2 Normal
Assignee: Sergio Villar Senin
URL:
Keywords: InRadar
Depends on: 221484
Blocks:
  Show dependency treegraph
 
Reported: 2021-01-28 16:38 PST by Dima Tisnek
Modified: 2021-02-12 01:02 PST (History)
5 users (show)

See Also:


Attachments
Screenshot w Safari and Safari TP (73.81 KB, image/png)
2021-01-28 16:38 PST, Dima Tisnek
no flags Details
test case (583 bytes, text/html)
2021-02-03 17:10 PST, Said Abou-Hallawa
no flags Details
test case (930 bytes, text/html)
2021-02-03 19:22 PST, Said Abou-Hallawa
no flags Details
test case (930 bytes, text/html)
2021-02-03 21:36 PST, Said Abou-Hallawa
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
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.