Bug 221110

Summary:

REGRESSION(r270578): Incorrect layout for the SVG image inside a flex container

Product:

WebKit

Reporter:

Dima Tisnek <dimaqq>

Component:

Images

Assignee:

Sergio Villar Senin <svillar>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

jonlee, sabouhallawa, smoley, svillar, webkit-bug-importer

Priority:

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

Dima Tisnek

Reported 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 :(

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
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Radar WebKit Bug Importer

Comment 1 2021-01-28 19:06:41 PST

<rdar://problem/73735772>

Dima Tisnek

Comment 2 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

Smoley

Comment 3 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.

Jon Lee

Comment 4 2021-02-03 16:17:22 PST

This seems to be fallout from r270578

Said Abou-Hallawa

Comment 5 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.

Said Abou-Hallawa

Comment 6 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.

Said Abou-Hallawa

Comment 7 2021-02-03 19:22:52 PST

Created attachment 419215 [details] test case The same bug happens with binary images.

Said Abou-Hallawa

Comment 8 2021-02-03 21:36:19 PST

Created attachment 419229 [details] test case

Sergio Villar Senin

Comment 9 2021-02-04 01:56:48 PST

Checking...

Sergio Villar Senin

Comment 10 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

Sergio Villar Senin

Comment 11 2021-02-05 09:53:08 PST

Test import ready for review in bug 221484

Sergio Villar Senin

Comment 12 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

Said Abou-Hallawa

Comment 13 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>

Sergio Villar Senin

Comment 14 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.

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