Incorrect height for `flex-direction: column` inside `flex-direction: row`

dhdaines commented 1 month ago

Yes, another flexbox bug! I think I have this narrowed down to the specific problem - in the rather common use case of display: flex; flex-direction: column nested inside display: flex; flex-direction: row, the inner elements are not expanded vertically to fit their contents. In other words, this code:

<html>
<head>
<title>Another FlexBox Test</title>
<style>
body {
    max-width: 30em;
    margin: auto;
}
section {
    border: 1px solid blue;
    display: flex;
    flex-direction: row;
}
article {
    border: 1px solid green;
    display: flex;
    flex-direction: column; /* the culprit */
    margin-top: 1ex;
    margin-bottom: 1ex;
}
</style>
</head>
<body>
  <section>
    <article>
      Question 1?  With quite a lot of extra text,
      which should not overflow in the PDF, we hope.
    </article>
    <article>
      Answer 1.  With quite a lot of extra text,
      which should not overflow in the PDF, we hope?
    </article>
  </section>
</body>
</html>

should do this (Chrome):

But actually does this (Weasyprint 62.3):

(the reason I'm not using grid layout is that actually my CSS is a bit more complex and relies on flex-wrap and flex-basis to make a layout of maximum two columns from a sequence of elements, but all of that works fine, it's just the box sizing that's wrong)

liZe commented 1 month ago

Thanks for the report. It’s maybe related to #1171.

dhdaines commented 1 month ago

Yes it seems like it might be similar (the common denominator is flex-direction: column). I can probably fix it for at least my specific case, not certain about the ones in that bug...

dhdaines commented 1 month ago

Definitely related to this comment: https://github.com/Kozea/WeasyPrint/blob/main/weasyprint/layout/flex.py#L255

flex-direction:column seems to just be generally broken (which would be the correct interpretation of that comment ;-)), as can be seen by changing the CSS to:

body {
    max-width: 30em;
    margin: auto;
}
section {
    border: 1px solid blue;
    display: flex;
    flex-direction: column;
}
article {
    flex: 1;
    border: 1px solid green;
    margin-top: 1ex;
    margin-bottom: 1ex;
}

Giving this obviously incorrect output:

dhdaines commented 1 month ago

When flex: 1 (relevant part being flex-grow: 1) is removed from the CSS we get this somewhat better but also incorrect output: So, the basis for children isn't being calculated correctly, and isn't being propagated correctly to the parent.

dhdaines commented 1 month ago

Hmm, the last two comments are a different (though vaguely related) issue, it seems. On the browser the available main space is clearly infinite when the main axis is 'height' (i.e. flex-direction is column), but on print media this isn't so clear - is it the remaining space on the page? (Weasyprint thinks so) or is it also infinite (since a flex container could get split across pages)?

If it's the remaining space on the page, then the sizing of <article> above (expanding to fill the page) is correct, it's just the sizing of <section> (not expanding to fit its content) that is wrong.

If it's infinite, then both are wrong, and the reason is that section 9.3.2.D of https://www.w3.org/TR/css-flexbox-1/#algo-main-item is not implemented, as noted here: https://github.com/Kozea/WeasyPrint/blob/main/weasyprint/layout/flex.py#L207 - this is what causes flex-direction: column to not expand flex items on the browser:

Otherwise, if the used flex basis is content or depends on its available space, the available main size is infinite, and the flex item’s inline axis is parallel to the main axis, lay the item out using the rules for a box in an orthogonal flow [CSS3-WRITING-MODES]. The flex base size is the item’s max-content main size.

Do you know what the CSS print media spec says about this?

dhdaines commented 1 month ago

So, back to my original problem, the issue here is simply not expanding the flex container in the cross-axis to fit its content. This is likely due to the fact that the code is explicitly setting the height style on the children here: https://github.com/Kozea/WeasyPrint/blob/main/weasyprint/layout/flex.py#L607 which it maybe shouldn't do, as it leads future code to believe that said children have a definite size, which they don't.

dhdaines commented 1 month ago

I may be able to make a draft PR this week. I have heavily annotated the flex code already :)

dhdaines commented 3 weeks ago

Note that #2231 fixes all the issues above as it turns out that yes, they were all related ;-)

The flexbox layout code is still broken in many ways though!

Kozea / WeasyPrint

Incorrect height for `flex-direction: column` inside `flex-direction: row` #2222