Dynamically added zero-height float pushed below the line it should fit beside
Reported by
msten...@opera.com,
Apr 29 2016
|
|||
Issue description
This happens because markLinesDirtyInBlockRange() in LayoutBlockFlow::layoutInlineChildren() fails to find any lines to mark dirty, if the height of the float is zero (it just bails if top >= bottom). Even zero-height floats need to be positioned correctly, since there may be actual content inside.
I discovered this because I'm trying to get rid of the direct call to layout() for floats in layoutInlineChildren(), because that's problematic for pagination, since the position of the float hasn't yet been determined and set at this point.
While playing with this code, I got fast/block/do-not-strip-anonymous-blocks-when-block-child-becomes-float-and-other-block-on-line.html to fail, and when I went to look, it turned out that I had actually fixed something. The diff went like this:
@@ -14,5 +14,5 @@
LayoutText {#text} at (0,0) size 0x0
LayoutBlockFlow {P} at (0,0) size 800x0
LayoutBlockFlow (anonymous) at (0,0) size 800x20
- LayoutBlockFlow (floating) {DD} at (800,20) size 0x0
+ LayoutBlockFlow (floating) {DD} at (800,0) size 0x0
LayoutBR {BR} at (0,0) size 0x20
But 800,0 *is* the correct position, while 800,20 is wrong. The attached test case is based on that layout test.
,
May 2 2016
I'd like to have this (no layout() at potentially bogus position) fixed soon, and if I don't have to fix it myself, that would be just great. :)
,
May 2 2016
But it will be fixed for free when you remove layout() for floats in layoutInlineChildren() right? Failing that, if you have something to push in my direction I can take a look this week.
,
May 3 2016
My issue with pagination goes away if I remove layout(), but this bug doesn't. And the current code is buggy, so I'd like to find out what could go wrong if I change which lines to mark dirty (not so concerned about the removal of layout(); we'll do that in positionNewFloats() afterwards anyway, when everything relevant has been positioned correctly). It would be nice if I could just mark only the first line that could possibly be affected by the float, without laying out. That would fix both my issue with pagination and this bug. But I suppose that could lead to performance regressions, since we'd then fail more often than before to re-enter "line-skipping mode" for the remaining lines in the container that were left untouched. Right? I don't know. I'm still trying to learn line layout. :) So, the code currently lays out to figure out the new height of the float, so that it can mark some lines dirty. This seems slightly off. It could be that the height of the float shrinks, so that it was taller before layout than after, so that we'll fail to mark all the lines that the float used to affect. Furthermore, we don't include the float's margin, which also affect adjacent lines. And then there's my issue with pagination: we don't position the float prior to layout, which will mess up the struts (no known bugs because of it at the moment, but this makes it hard to improve or optimize pagination). But it would be the wrong time (too early) to position the floats anyway, since, for all we know, new lines/floats may be inserted, removed or resized above the float.
,
May 3 2017
This issue has been available for more than 365 days, and should be re-evaluated. Please re-triage this issue. The Hotlist-Recharge-Cold label is applied for tracking purposes, and should not be removed after re-triaging the issue. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
May 3 2017
This appears to be fixed now! |
|||
►
Sign in to add a comment |
|||
Comment 1 by e...@chromium.org
, May 2 2016Status: Available (was: Untriaged)