Skip to content

[css-flexbox] incorrect layout with max-height and flex-direction column and justify-content center#35859

Open
pgorszkowski-igalia wants to merge 1 commit intoWebKit:mainfrom
pgorszkowski-igalia:eng/css-flexbox-incorrect-layout-with-max-height-and-flex-direction-column-and-justify-content-center
Open

[css-flexbox] incorrect layout with max-height and flex-direction column and justify-content center#35859
pgorszkowski-igalia wants to merge 1 commit intoWebKit:mainfrom
pgorszkowski-igalia:eng/css-flexbox-incorrect-layout-with-max-height-and-flex-direction-column-and-justify-content-center

Conversation

@pgorszkowski-igalia
Copy link
Copy Markdown
Contributor

@pgorszkowski-igalia pgorszkowski-igalia commented Oct 29, 2024

ebca6b1bdd254c8a39d39ad9e651345b623fbc92

[css-flexbox] incorrect layout with max-height and flex-direction column and justify-content center
https://bugs.webkit.org/show_bug.cgi?id=282036

Reviewed by NOBODY (OOPS!).

After calculation of the flex item size and adjusting it to min/max size
of the flex item we should also check how the calculated size
corresponds to specified max size of the whole container. In case the
adjusted flex item size is bigger than the max size of the whole
container we should use the flex item size calculated before adjusting
to flex item min/max size.

There is new layout test which tests the problem.

* LayoutTests/css3/flexbox/nested-flexbox-max-height-expected.txt: Added.
* LayoutTests/css3/flexbox/nested-flexbox-max-height.html: Added.

59a2273

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ✅ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@pgorszkowski-igalia pgorszkowski-igalia self-assigned this Oct 29, 2024
@pgorszkowski-igalia pgorszkowski-igalia added the Layout and Rendering For bugs with layout and rendering of Web pages. label Oct 29, 2024
@pgorszkowski-igalia pgorszkowski-igalia marked this pull request as draft October 29, 2024 13:25
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Oct 29, 2024
@pgorszkowski-igalia pgorszkowski-igalia force-pushed the eng/css-flexbox-incorrect-layout-with-max-height-and-flex-direction-column-and-justify-content-center branch from ebca6b1 to a7e7723 Compare October 30, 2024 12:58
@pgorszkowski-igalia pgorszkowski-igalia force-pushed the eng/css-flexbox-incorrect-layout-with-max-height-and-flex-direction-column-and-justify-content-center branch from a7e7723 to c6bea82 Compare November 7, 2024 09:10
@pgorszkowski-igalia pgorszkowski-igalia force-pushed the eng/css-flexbox-incorrect-layout-with-max-height-and-flex-direction-column-and-justify-content-center branch from c6bea82 to c84db3b Compare November 7, 2024 12:11
@pgorszkowski-igalia pgorszkowski-igalia marked this pull request as ready for review November 7, 2024 12:12
@svillar
Copy link
Copy Markdown
Contributor

svillar commented Nov 7, 2024

As a general comment would it be possible to have a WPT test instead?

@pgorszkowski-igalia
Copy link
Copy Markdown
Contributor Author

As a general comment would it be possible to have a WPT test instead?

good point, I will create the test also on WPT

@pgorszkowski-igalia pgorszkowski-igalia removed the merging-blocked Applied to prevent a change from being merged label Nov 8, 2024
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Nov 8, 2024
@pgorszkowski-igalia
Copy link
Copy Markdown
Contributor Author

As a general comment would it be possible to have a WPT test instead?

The PR with test on WPT is under review: web-platform-tests/wpt#49219

…umn and justify-content center

https://bugs.webkit.org/show_bug.cgi?id=282036

Reviewed by NOBODY (OOPS!).

After calculation of the flex item size and adjusting it to min/max size
of the flex item we should also check how the calculated size
corresponds to specified max size of the whole container. In case the
adjusted flex item size is bigger than the max size of the whole
container we should use the flex item size calculated before adjusting
to flex item min/max size.

* Source/WebCore/rendering/RenderFlexibleBox.cpp:
(WebCore::RenderFlexibleBox::resolveFlexibleLengths):
@pgorszkowski-igalia pgorszkowski-igalia force-pushed the eng/css-flexbox-incorrect-layout-with-max-height-and-flex-direction-column-and-justify-content-center branch from c84db3b to 59a2273 Compare March 17, 2025 09:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Layout and Rendering For bugs with layout and rendering of Web pages. merging-blocked Applied to prevent a change from being merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants