Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[css2][css-flow] Define the intrinsic sizing algorithm for block containers #9120

Open
Loirooriol opened this issue Jul 26, 2023 · 5 comments

Comments

@Loirooriol
Copy link
Contributor

Loirooriol commented Jul 26, 2023

https://drafts.csswg.org/css2/#float-width

Calculation of the shrink-to-fit width is similar to calculating the width of a table cell using the automatic table layout algorithm. Roughly: calculate the preferred width by formatting the content without breaking lines other than where explicit line breaks occur, and also calculate the preferred minimum width, e.g., by trying all possible line breaks. CSS 2 does not define the exact algorithm.

This is especially tricky in the presence of floats, clearance, and children that establish an independent formatting context.

However, we do seem to have good interoperability (at least in the block-level children case), even if there is no WPT coverage. So I would like to discuss resolving on this interoperable behavior.

Some interesting examples:

The max-content size may be bigger than necessary
<article style="display: inline-block; border: solid">
  <div><!-- Just to prevent the parent from establishing an IFC, otherwise WebKit is different --></div>
  <div style="float: left; width: 50px; height: 30px; background: cyan"></div>
  <div style="float: right; width: 30px; height: 30px; background: yellow"></div>
  <div style="float: left; clear: left; width: 70px; height: 20px; background: magenta"></div>
</article>

It uses a width of 100px, even though 80px would suffice to avoid "breaking lines" between the cyan and the yellow floats.

The max-content size may not be big enough to avoid "breaking lines"
<article style="display: inline-block; border: solid; vertical-align: top">
  <div style="float: left; width: 30px; height: 30px; background: cyan"></div>
  <div style="float: left; width: 30px; height: 30px; background: yellow"></div>
  <div style="overflow: hidden; width: 50px; height: 30px; background: magenta"></div>
</article>
<article style="display: inline-block; border: solid; vertical-align: top">
  <div style="float: left; width: 30px; height: 30px; background: cyan"></div>
  <div style="float: left; width: 30px; height: 30px; background: yellow"></div>
  <div>
    <div style="overflow: hidden; width: 50px; height: 30px; background: magenta"></div>
  </div>
</article>
<article style="display: inline-block; border: solid; vertical-align: top">
  <div style="float: left; width: 30px; height: 30px; background: cyan"></div>
  <div style="float: left; width: 30px; height: 30px; background: yellow"></div>
  <div></div>
  <div style="overflow: hidden; width: 50px; height: 30px; background: magenta"></div>
</article>

Note that the 2nd case just wraps the BFC root inside an intermediate container, and the 3rd case inserts and empty block before it. Both changes reduce the max-content size and then the BFC root no longer fits next to the floats.

If the block container contains block-level children (and is not a table wrapper box?):

In Servo I have tried to implement the behavior that I observed on other browsers, I haven't checked their source code but it seems to be:

For the max-content size,

  1. Let max_size be 0px. This tracks the maximum size seen so far, not including trailing uncleared floats.
  2. Let left_floats be 0px. This tracks the size of the trailing uncleared left floats.
  3. Let right_floats be 0px. This tracks the size of the trailing uncleared right floats.
  4. For each block-level child (including floats, ignoring abspos):
    1. Let clear be the the computed value of the clear CSS property of the child.
    2. If the child continues the same BFC (instead of establishing an independent formatting context),
      1. Set clear := both (this is to avoid adding its size to the floats)
    3. If clear is different than none,
      1. Set max_size := max(max_size, left_floats + right_floats)
    4. If clear is left or both,
      1. Set left_floats := 0px
    5. If clear is right or both,
      1. Set right_floats := 0px
    6. Let size be the outer max-content size of the child, floored by 0px
      1. Bug? Firefox doesn't floor when clear is none and the element doesn't float
      2. Bug? WebKit seems to floor left_floats + right_floats instead of each float individually
    7. If the child floats left,
      1. Set left_floats := left_floats + size
    8. Else, if the child floats right,
      1. Set right_floats := right_floats + size
    9. Else (the child doesn't float),
      1. Set max_size := max(max_size, left_floats + right_floats + size)
      2. Set left_floats := 0px
      3. Set right_floats := 0px
  5. The max-content size is max(max_size, left_floats + right_floats)

The min-content size is just the maximum among the outer min-content sizes of each float or in-flow block-level child.

Firefox flooring bug (?)
<article style="display: inline-block; border: solid; margin-right: 100px">
  <div style="float: right; width: 50px; height: 30px; background: cyan"></div>
  <div style="clear: left; overflow: hidden; width: 100px; height: 30px; background: magenta; margin-right: -500px"></div>
</article>
<article style="display: inline-block; border: solid">
  <div style="float: right; width: 50px; height: 30px; background: cyan"></div>
  <div style="overflow: hidden; width: 100px; height: 30px; background: magenta; margin-right: -500px"></div>
</article>

There is no left float, so clear: left should have no effect. However, removing it causes Firefox to stop flooring the size of the BFC root by 0px before adding it to the floats, and then the container becomes 0px wide.

Firefox Blink/WebKit
WebKit flooring bug (?)
<article style="display: inline-block; border: solid; margin-left: 25px">
  <div style="float: left; width: 50px; height: 50px; background: cyan"></div>
  <div style="float: left; width: 50px; height: 40px; background: yellow; margin-left: -75px"></div>
  <div style="overflow: hidden; width: 50px; height: 50px; background: magenta;"></div>
</article>
<article style="display: inline-block; border: solid;">
  <div style="float: left; width: 50px; height: 50px; background: cyan"></div>
  <div style="float: right; width: 50px; height: 40px; background: yellow; margin-left: -75px"></div>
  <div style="overflow: hidden; width: 50px; height: 50px; background: magenta;"></div>
</article>

WebKit doesn't floor the floats individually, so left_floats + right_floats is 25px instead of 50px. This seems suboptimal since it forces the BFC root to move down.

Firefox/Blink WebKit

If the block container establishes an inline formatting context:

There is less interoperability here, e.g.

<article style="display: inline-block; border: solid">
  .<!-- Just to force the parent to establish an IFC on Blink -->
  <div style="float: left; width: 50px; height: 30px; background: cyan"></div>
  <div style="float: right; width: 30px; height: 30px; background: yellow"></div>
  <div style="float: left; clear: left; width: 70px; height: 20px; background: magenta"></div>
</article>
Firefox Blink/WebKit

I haven't implemented this case yet so I don't have the details, but aligning with Firefox seems more consistent with the block-level children case. Blink and WebKit seem to ignore clear: left.

@Loirooriol
Copy link
Contributor Author

@emilio @bfgeek Can you check if Firefox/Blink do something that I missed in the algorithm above, or want to propose a simpler algorithm?

@emilio
Copy link
Collaborator

emilio commented Jul 26, 2023

The algorithm in Firefox is here, and in particular ForceBreak is what seems to be dealing with floats here.

We seem to be preserving more float information than just left / right (we keep actually an array of pointers to all floats). But that might be something that we can just simplify, I haven't looked closely enough to see when it differs from your algorithm.

It seems blame for most of that code points to @upsuper (in https://bugzilla.mozilla.org/show_bug.cgi?id=1260031 and https://bugzilla.mozilla.org/show_bug.cgi?id=1322843) and @dbaron (reviewer, and in the original reflow branch), so they may have opinions.

Per the comments in the first link above we were mostly trying to match Chromium and EdgeHTML which at the time agreed.

@emilio
Copy link
Collaborator

emilio commented Jul 26, 2023

(In any case +1 on specifying this in more detail)

Loirooriol added a commit to Loirooriol/servo that referenced this issue Jul 27, 2023
Calculating the max-content size of a block container may add the outer
max-content sizes of multiple children. The problem is that the outer
size may be negative (due to margins), producing an incorrect result.

In particular, it could happen that the max-content size was 50-25=25,
but the min-content size would just take the maximum and be 50, which
doesn't make sense.

Therefore, this patch floors the size of all children by 0. This seems
to match Blink. Firefox and WebKit don't floor in some cases, but then
the result seems suboptimal to me. Note that there is no spec for this,
see w3c/csswg-drafts#9120 for details.
github-merge-queue bot pushed a commit to servo/servo that referenced this issue Jul 27, 2023
…xes (#30034)

Calculating the max-content size of a block container may add the outer
max-content sizes of multiple children. The problem is that the outer
size may be negative (due to margins), producing an incorrect result.

In particular, it could happen that the max-content size was 50-25=25,
but the min-content size would just take the maximum and be 50, which
doesn't make sense.

Therefore, this patch floors the size of all children by 0. This seems
to match Blink. Firefox and WebKit don't floor in some cases, but then
the result seems suboptimal to me. Note that there is no spec for this,
see w3c/csswg-drafts#9120 for details.
@Loirooriol
Copy link
Contributor Author

WebKit code is in https://searchfox.org/wubkat/rev/ea191c94955ddd2f015f7a677b138109987620b2/Source/WebCore/rendering/RenderBlock.cpp#2275

It seems quite similar to what I wrote above, except that it does weird things with margins.

@Loirooriol
Copy link
Contributor Author

OK, so one detail that I missed above regarding margins:

<!DOCTYPE html>
<div style="float: left; border: solid">
  <div style="float: left; width: 50px; height: 50px; background: cyan"></div>
  <div style="display: flow-root; width: 50px; height: 50px; background: magenta; margin-left: 25px"></div>
</div>
<div style="float: left; clear: left; border: solid">
  <div style="float: left; width: 50px; height: 50px; background: cyan"></div>
  <div style="display: flow-root; width: 50px; height: 50px; background: magenta; margin-left: 75px"></div>
</div>
Gecko Blink/WebKit

My algorithm above matches Gecko, but I think Blink and WebKit make more sense.
Only the border box of a BFC root needs to avoid floats, the margin can still overlap them, so intrinsic sizing should take that into account.

@Loirooriol Loirooriol changed the title Define the intrinsic sizing algorithm for block containers [css2][css-flow] Define the intrinsic sizing algorithm for block containers Sep 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants