A line that doesn't fit in a column shouldn't pull preceding floats to the next column.

A line that doesn't fit in a column shouldn't pull preceding floats to the next column.

Require that a line box be flush with the content edge of the block if it is to
pull its parent block to the next column.

Also cleaned up the surrounding code and documented it. It had a rather complex
if-expression not really suitable for human consumption as it was, and adding
even more to it would have been just evil. No other behavioral changes
intended, apart from the actual bugfix.

BUG=516296
R=jchaffraix@chromium.org,leviw@chromium.org

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

[mstensho (USE GERRIT), 2015-08-06 12:20:47]

mstensho@opera.com changed reviewers:
+ jchaffraix@chromium.org

[mstensho (USE GERRIT), 2015-08-10 12:16:58]

jchaffraix / leviw: please take a look at my glorified one-liner.

[leviw_travelin_and_unemployed, 2015-08-10 19:46:19]

lgtm

https://codereview.chromium.org/1277613004/diff/1/LayoutTests/fast/multicol/l...
File LayoutTests/fast/multicol/line-pushed-down-by-float.html (right):

https://codereview.chromium.org/1277613004/diff/1/LayoutTests/fast/multicol/l...
LayoutTests/fast/multicol/line-pushed-down-by-float.html:6: <div
style="float:left; width:100%; height:7em; background:cyan;">
I'm going to have to try and convince you of the glory of papayawhip one of
these days ;)

https://code.google.com/p/chromium/codesearch#search/&q=papayawhip%20f:/Layou...

https://codereview.chromium.org/1277613004/diff/1/Source/core/layout/LayoutBl...
File Source/core/layout/LayoutBlockFlow.cpp (right):

https://codereview.chromium.org/1277613004/diff/1/Source/core/layout/LayoutBl...
Source/core/layout/LayoutBlockFlow.cpp:787: if (!style()->hasAutoOrphans() &&
style()->orphans() >= lineIndex) {
It may be worth extracting this if/else block (determining if we want the strut
on the block) into a named function for ease of reading.

[mstensho (USE GERRIT), 2015-08-10 20:14:45]

https://codereview.chromium.org/1277613004/diff/1/LayoutTests/fast/multicol/l...
File LayoutTests/fast/multicol/line-pushed-down-by-float.html (right):

https://codereview.chromium.org/1277613004/diff/1/LayoutTests/fast/multicol/l...
LayoutTests/fast/multicol/line-pushed-down-by-float.html:6: <div
style="float:left; width:100%; height:7em; background:cyan;">
On 2015/08/10 19:46:19, leviw wrote:
> I'm going to have to try and convince you of the glory of papayawhip one of
> these days ;)
> 
>
https://code.google.com/p/chromium/codesearch#search/&q=papayawhip%20f:/Layou...

Lovely color, but how do I type it as fast as cyan?

https://codereview.chromium.org/1277613004/diff/1/Source/core/layout/LayoutBl...
File Source/core/layout/LayoutBlockFlow.cpp (right):

https://codereview.chromium.org/1277613004/diff/1/Source/core/layout/LayoutBl...
Source/core/layout/LayoutBlockFlow.cpp:787: if (!style()->hasAutoOrphans() &&
style()->orphans() >= lineIndex) {
On 2015/08/10 19:46:19, leviw wrote:
> It may be worth extracting this if/else block (determining if we want the
strut
> on the block) into a named function for ease of reading.

Done.

That helper function got some papayawhip-colored parameter soup, but I still
like it better than what I had before.

[leviw_travelin_and_unemployed, 2015-08-10 20:18:10]

On 2015/08/10 at 20:14:45, mstensho wrote:
>
https://codereview.chromium.org/1277613004/diff/1/LayoutTests/fast/multicol/l...
> File LayoutTests/fast/multicol/line-pushed-down-by-float.html (right):
> 
>
https://codereview.chromium.org/1277613004/diff/1/LayoutTests/fast/multicol/l...
> LayoutTests/fast/multicol/line-pushed-down-by-float.html:6: <div
style="float:left; width:100%; height:7em; background:cyan;">
> On 2015/08/10 19:46:19, leviw wrote:
> > I'm going to have to try and convince you of the glory of papayawhip one of
> > these days ;)
> > 
> >
https://code.google.com/p/chromium/codesearch#search/&q=papayawhip%20f:/Layou...
> 
> Lovely color, but how do I type it as fast as cyan?

ctrl-v? ;)

> 
>
https://codereview.chromium.org/1277613004/diff/1/Source/core/layout/LayoutBl...
> File Source/core/layout/LayoutBlockFlow.cpp (right):
> 
>
https://codereview.chromium.org/1277613004/diff/1/Source/core/layout/LayoutBl...
> Source/core/layout/LayoutBlockFlow.cpp:787: if (!style()->hasAutoOrphans() &&
style()->orphans() >= lineIndex) {
> On 2015/08/10 19:46:19, leviw wrote:
> > It may be worth extracting this if/else block (determining if we want the
strut
> > on the block) into a named function for ease of reading.
> 
> Done.
> 
> That helper function got some papayawhip-colored parameter soup, but I still
like it better than what I had before.

I figured that would be the case. I still suspect I'll like it more :)

[mstensho (USE GERRIT), 2015-08-10 20:27:14]

The CQ bit was checked by mstensho@opera.com

[mstensho (USE GERRIT), 2015-08-10 20:27:14]

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/1277613004/#ps20001
(title: "Code review. Add helper function.")

[commit-bot: I haz the power, 2015-08-10 20:27:37]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1277613004/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1277613004/20001

[commit-bot: I haz the power, 2015-08-10 21:52:20]

Committed patchset #2 (id:20001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=200269