Skip to content

Commit

Permalink
Always add new line box for content pushed down by float.
Browse files Browse the repository at this point in the history
Don't put floats together with inline content inside the same line box,
if the inline content is pushed down by floats. This could happen if
text-indent was involved.

If a container has text indentation, and there's only room for leading
floats on the first "line", and no room for real inline content, also
rewind indentation, to prevent it from creating an overflowed line.
Otherwise we'd try to place the inline content below the float(s) (next
layout opportunity), within the same line box fragment, like this:

  box container fragment
    line box fragment
      float
      inline content below the float

based on this example:

  <div style="text-indent:1px;">
    <div style="float:left; width:100%; height:100px;"></div>
    text
  </div>

This was special-behavior that would only occur for text-indent (due to
the line overflow mentioned further up). With no text indent, on the
other hand:

  box container fragment
    line box fragment
      float
    line box fragment
      inline content below the float

With this code change, there will be no such special-behavior for
text-indent. A new line will be created for inline content that's pushed
down by leading floats.

Placing text content below the float within the same line box fragment
was problematic for block fragmentation, for two reasons:

  1. Correctness: Line boxes are monolithic, so it's impossible to break
     inside them. If the inline content doesn't really fit below the
     float, we'd be unable to push it into the next fragmentainer.

  2. Freeze: The geometry of line boxes only includes the actual inline
     content, not the floats, so that with inline content pushed below a
     float, we'd get a block-offset greater than zero, thanks to the
     float, which in turn would trick the fragmentation machinery into
     thinking that we were able to place something preceding the line,
     meaning that it would be safe to break before it, whereas in
     reality, there was just this one line box, and breaking before it
     would lead to no progress -> infinite loop.

With this change, now that the first line box may no longer necessarily
be the first formatted line (if it only contains leading floats, and
thus isn't a real line), we need to store this information on inline
break tokens, so that we instead apply text indentation in a subsequent
line (the first formatted line). This incidentally fixed another bug,
with ::first-line not being applied on the first actual formatted line,
if there was a preceding wide float.
Test included: css/css-pseudo/first-line-below-float.html

I also realized that there was no test coverage at all for text
indentation being pushed below a float, even if it was working.
Added css/css-text/text-indent/below-float.html for this.

css/css-break/text-indent-and-wide-float.html is the actual test for
this bugfix, which is also a correctness test that verifies that the
line pushed below a float is actually pushed all the way into the next
fragmentainer, if it doesn't fit below the float.

Bug: 365814218
Change-Id: I5ad327b91bf6dd6b87538b3e8e865a8f529a72bd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5921254
Reviewed-by: Koji Ishii <[email protected]>
Commit-Queue: Morten Stenshorne <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1368698}
  • Loading branch information
mstensho authored and chromium-wpt-export-bot committed Oct 15, 2024
1 parent 4732a86 commit 617a623
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 0 deletions.
18 changes: 18 additions & 0 deletions css/css-break/text-indent-and-wide-float.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<!DOCTYPE html>
<link rel="author" title="Morten Stenshorne" href="mailto:[email protected]">
<link rel="help" href="http://crbug.com/365814218">
<meta name="assert" content="Floats are not part of lines, so if a float is too wide to fit any inline content beside it, the first formatted line goes below it, or even into the next fragmentainer if there's insufficient room below.">
<link rel="match" href="../reference/ref-filled-green-100px-square.xht">
<link rel="stylesheet" type="text/css" href="/fonts/ahem.css">
<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
<div style="width:100px; height:100px; background:red;">
<div style="columns:2; column-fill:auto; gap:0; height:110px; font:25px/25px Ahem; text-indent:25px;">
<span style="color:green;">
<div style="float:left; width:100%; height:100px; background:green;">
<!-- Bleed into the next column, to cover the text indentation there. -->
<div style="width:150%; height:25px; background:green;"></div>
</div>
x xx xx xx
</span>
</div>
</div>
23 changes: 23 additions & 0 deletions css/css-pseudo/first-line-below-float.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<!DOCTYPE html>
<link rel="author" title="Morten Stenshorne" href="mailto:[email protected]">
<link rel="help" href="https://www.w3.org/TR/css-pseudo-4/#first-text-line">
<meta name="assert" content="Floats are not part of lines, so if a float is too wide to fit any inline content beside it, the first formatted line goes below it">
<link rel="match" href="../reference/ref-filled-green-100px-square.xht">
<link rel="stylesheet" type="text/css" href="/fonts/ahem.css">
<style>
.container {
width: 100px;
height: 100px;
font: 50px/50px Ahem;
color: red;
background: red;
}
.container::first-line {
color: green;
}
</style>
<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
<div class="container">
<div style="float:left; width:100px; height:50px; background:green;"></div>
xx
</div>
14 changes: 14 additions & 0 deletions css/css-text/text-indent/below-float.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<!DOCTYPE html>
<link rel="author" title="Morten Stenshorne" href="mailto:[email protected]">
<link rel="help" href="https://www.w3.org/TR/css-text-4/#text-indent-property">
<meta name="assert" content="Floats are not part of lines, so if a float is too wide to fit any inline content beside it, the first formatted line goes below it">
<link rel="match" href="../../reference/ref-filled-green-100px-square.xht">
<link rel="stylesheet" type="text/css" href="/fonts/ahem.css">
<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
<div style="position:relative; width:100px; height:100px; font:50px/50px Ahem; text-indent:50px; color:green; background:red;">
<div style="position:absolute; top:50px; width:50px; height:50px; background:green;"></div>
<span>
<div style="float:left; width:100px; height:50px; background:green;"></div>
x
</span>
</div>

0 comments on commit 617a623

Please sign in to comment.