Floats on an empty line should still get positioned.

Floats on an empty line should still get positioned.

A float can sit on an empty line and so may not have been positioned while
searching for a line break.

Likewise if we have a line break with clearance then ensure it clears the floats
before it in the DOM but not after it. Dirty the lineboxes after a hard-break with
clearance so that we do this correctly; don't try to work it out after we're done
dirtying lines and may have already added in a float we're not supposed to clear.

BUG=477814
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=197288

[rhogan, 2015-05-30 15:25:40]

robhogan@gmail.com changed reviewers:
+ leviw@chromium.org

[leviw_travelin_and_unemployed, 2015-06-02 22:10:44]

https://codereview.chromium.org/1156063002/diff/40001/Source/core/layout/Layo...
File Source/core/layout/LayoutBlockFlowLine.cpp (right):

https://codereview.chromium.org/1156063002/diff/40001/Source/core/layout/Layo...
Source/core/layout/LayoutBlockFlowLine.cpp:1651: if (lastObject->isBR() &&
lastObject->style()->clear() != CNONE)
This seems similar to what
https://blink.lc/blink/commit/?id=712375bd4009d756b411cb80164fa1ed04ee219a was
going for?

[rhogan, 2015-06-03 21:09:23]

On 2015/06/02 at 22:10:44, leviw wrote:
>
https://codereview.chromium.org/1156063002/diff/40001/Source/core/layout/Layo...
> File Source/core/layout/LayoutBlockFlowLine.cpp (right):
> 
>
https://codereview.chromium.org/1156063002/diff/40001/Source/core/layout/Layo...
> Source/core/layout/LayoutBlockFlowLine.cpp:1651: if (lastObject->isBR() &&
lastObject->style()->clear() != CNONE)
> This seems similar to what
https://blink.lc/blink/commit/?id=712375bd4009d756b411cb80164fa1ed04ee219a was
going for?

It's different but related. In that CL we're saying 'give up on syncing with
clean lines if we have floats and a <br> with clearance'. That decision should
really be made in determineEndPosition using the same code here. And in a
separate patch I will propose to do that.

[rhogan, 2015-06-03 21:13:16]

On 2015/06/03 at 21:09:23, rhogan wrote:
> On 2015/06/02 at 22:10:44, leviw wrote:
> >
https://codereview.chromium.org/1156063002/diff/40001/Source/core/layout/Layo...
> > File Source/core/layout/LayoutBlockFlowLine.cpp (right):
> > 
> >
https://codereview.chromium.org/1156063002/diff/40001/Source/core/layout/Layo...
> > Source/core/layout/LayoutBlockFlowLine.cpp:1651: if (lastObject->isBR() &&
lastObject->style()->clear() != CNONE)
> > This seems similar to what
https://blink.lc/blink/commit/?id=712375bd4009d756b411cb80164fa1ed04ee219a was
going for?
> 
> It's different but related. In that CL we're saying 'give up on syncing with
clean lines if we have floats and a <br> with clearance'. That decision should
really be made in determineEndPosition using the same code here. And in a
separate patch I will propose to do that.

What I'm saying here is that instead of identifying a linebox that's clean as
our end point and then subsequently disregarding it because we've set a bit on
layout state elsewhere, we should just notice immediately that we can't identify
a clean linebox and regard every linebox from our start point onwards as dirty.

[leviw_travelin_and_unemployed, 2015-06-08 20:12:35]

Can you add a TODO to the similar code to solve that case the same way as here?

LGTM.

https://codereview.chromium.org/1156063002/diff/40001/Source/core/layout/Layo...
File Source/core/layout/LayoutBlockFlowLine.cpp (right):

https://codereview.chromium.org/1156063002/diff/40001/Source/core/layout/Layo...
Source/core/layout/LayoutBlockFlowLine.cpp:1622: RootInlineBox*
lineBoxWithBreakAndClearance = 0;
Maybe firstLineBoxWithBreakAndClearance?

https://codereview.chromium.org/1156063002/diff/40001/Source/core/layout/Layo...
Source/core/layout/LayoutBlockFlowLine.cpp:1650: LayoutObject* lastObject =
style()->isLeftToRightDirection() ? &lastLeafChild->layoutObject() :
&curr->firstLeafChild()->layoutObject();
You shouldn't always traverse the first and last leaf child in RTL. How about
this:

InlineBox* lastBox = style()->isLeftToRightDirection() ? &curr->lastLeafChild()
: &curr->firstLeafChild();
if (lastBox && lastBox->layoutObject()->isBr() &&
lastBox()->lastObject()->style()->clear() != CNONE)?

[rhogan, 2015-06-17 20:22:08]

The CQ bit was checked by robhogan@gmail.com

[rhogan, 2015-06-17 20:22:08]

The patchset sent to the CQ was uploaded after l-g-t-m from leviw@chromium.org
Link to the patchset: https://codereview.chromium.org/1156063002/#ps60001
(title: "Updated")

[commit-bot: I haz the power, 2015-06-17 20:22:57]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1156063002/60001

[commit-bot: I haz the power, 2015-06-17 22:18:42]

Committed patchset #4 (id:60001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=197288