[wpe-2.38] Incorrect rendering with flexbox layout

filipe-norte-red commented 1 month ago

The attached index.zip contains an example page using flexbox layout where the content is not correctly rendered:

Expected:

Actual: actual

Removing the "flex-direction" or "justify-content" properties in the "menu-title-container" style, causes the content to be correctly rendered (in the later case the content will not be centered when compared to the "expected" result above).

The attached example page renders as expected on Chrome and Firefox.

Internal reference: LLAMA-14257

pgorszkowski-igalia commented 1 month ago

The problem can also be reproduced on the latest WebKit. I will create an upstream report and investigate it there.

pgorszkowski-igalia commented 1 month ago

Just some findings about this issue: so the main problem is when we have an embedded flexbox container inside other flexbox container and the main/outside flexbox container has max-height set to smaller value than the height value set in inside container, simple test case:

<html>
<head>
    <style type="text/css">
        .focusable-menu-item {
            width: 368px;
            height: 70px;
            max-height: 70px;

            color: #fff;

            display: flex;
            align-items: center;
            flex-direction: column;
        }

        .menu-title-container-focused {
            background: red;
        }

        .menu-title-container {
            display: flex;
            justify-content: center;
            flex-direction: column;

            width: 100%;
            height: 300px;

            text-align: center;
        }
    </style>
</head>
<body style="background-color:black">
        <div class="focusable-menu-item">
            <div class="menu-title-container menu-title-container-focused"><span style="font-size: 26px;">OPTION 1</span></div>
        </div>
</body>
</html>

The problem is seen only in case of flex-direction: column/column-reverse and justify-content: center/space-around/space-evenly for embedded container. It seems that the max-height from the outside container is not taken into consideration when the final size(height) of the item is calculated.

I am still investigating the possible solutions without introducing regressions for other cases with flexboxes.

pgorszkowski-igalia commented 1 month ago

The issue is reported in upstream: https://bugs.webkit.org/show_bug.cgi?id=282036

pgorszkowski-igalia commented 3 weeks ago

I am working on the solution in upstream: https://github.com/WebKit/WebKit/pull/35859, The current draft and potential fix introduces regressions in other LayoutTests of FlexBox layouts so I need to find a different way to solve it.

pgorszkowski-igalia commented 2 weeks ago

The fix in upstream is under review right now, but I don't know how long it would take to merge. There are no regressions in the existing layout tests. I tested it also with provided minimal test case and the original case from: https://cloud.rtl.it/hbbtv.rtl.it/skyq/index.html, it works there. @filipe-norte-red: Can you use this change to check possible regressions with your internal test cases? The scope of testing should be flexbox layout. The branch of wpe-2.38 with my change: https://github.com/WebPlatformForEmbedded/WPEWebKit/commit/3ce34e6f1b70e2d3a43d27423cd8df5684604b06

filipe-norte-red commented 1 week ago

@pgorszkowski-igalia, I'll run the changes through some tests and apps to see if there are any obvious regressions and let you know

filipe-norte-red commented 1 week ago

@pgorszkowski-igalia , I did some initial testing using several apps and some testc ases (although not specifically focused on flexbox) and have not observed any visible regressions so far. We are still testing a bit further on other devices. Kindly bare with me a few more days for those results. Thanks

pgorszkowski-igalia commented 1 week ago

I also created a test on WPT (web-platform-tests) for this case to have it covered on other browsers too.

filipe-norte-red commented 14 hours ago

@pgorszkowski-igalia , no regressions were observed after further testing, so the change would seem to be good to be merged. Thank you!

WebPlatformForEmbedded / WPEWebKit

[wpe-2.38] Incorrect rendering with flexbox layout #1417