search

LadybirdBrowser / ladybird

Truly independent web browser
https://ladybird.org
BSD 2-Clause "Simplified" License
21.86k stars 968 forks source link

Collapsing margins with inline layout, clear, and reset #1462

Closed pavpanchekha closed 1 month ago

pavpanchekha commented 1 month ago

I found an issue on the Web Browser Engineering online book, on pages like http://browser.engineering/html.html, with floats and collapsing margins. Here's Chrome:

image

Here's Ladybird:

image

Note the different gap between the gray nav bar and the paragraph of text underneath. I've minimized it to this HTML:

<!DOCTYPE html>
<style> div { height: 1rem; } </style>
<body style="font-size: 24px; width: 80rex; padding: 0; border: 1px solid black;">

<div id=A style="margin-bottom: 1rem; background: green;"></div>
<div id=B style="float: right; width: 3rem; background: orange;"></div>
<div id=C style="float: right; clear: right; width: 3rem; background: red;"></div>
<div id=D style="background: blue;"></div>

On Chrome:

image

On Ladybird:

image

I'm going to try to solve it myself but figured I'd gotten far enough to file an issue.

pavpanchekha commented 1 month ago

Conclusions so far: LB creates an anonymous block box containing the orange/red floats. That anonymous box is what resets the collapsed-margin state. That box lays out its contents with an IFC; it's that IFC specifically that resets it.

pavpanchekha commented 1 month ago

Ok, further conclusions.

We're looking at this subtree of the layout tree:

BlockContainer (anonymous)
  BlockContainer DIV#B
  BlockContainer DIV#C

Both B and C are floating, and C introduces clearance; that causes the IFC to call reset_margin_state inside generate_line_boxes in the Floating case. Now to figure out why this is invalid to do.

pavpanchekha commented 1 month ago

(Point of pride: Cassius gets it right.)

pavpanchekha commented 1 month ago

Ok, got it, I think here's what happens in the spec (I'm using the CSS 2.1 spec which I'm not sure if it's still controlling). The #B and #C are placed into an anonymous block box, which creates an IFC, which creates a line box. Now consider the the rules for adjoining margins, here: https://www.w3.org/TR/CSS2/box.html#collapsing-margins

Two margins are adjoining if and only if:

  • ...
  • ... no line boxes ... separate them (Note that certain zero-height line boxes (see 9.4.2) are ignored for this purpose.)

So our anonymous block box does have line boxes separating its top and bottom, so the question is whether that is a "certain" zero-height line box:

Line boxes that contain no text, no preserved white space, no inline elements with non-zero margins, padding, or borders, and no other in-flow content (such as images, inline blocks or inline tables), and do not end with a preserved newline must be treated as zero-height line boxes for the purposes of determining the positions of any elements inside of them, and must be treated as not existing for any other purpose.

Our line box contains nothing at all so it qualifies. The other part that might worry us is:

Two margins are adjoining if and only if:

  • ...
  • ... no clearance ... separate them (Note that certain zero-height line boxes (see 9.4.2) are ignored for this purpose.)

But this isn't talking about clearance on floating elements, it's talking about clearance on in-flow children.

pavpanchekha commented 1 month ago

So my current thinking is that we don't need to reset the parent's margin state when we see a Floating box that introduces clearance? Only if some other element introduces clearance. Commenting that line introduces no test failures.