Only block container children support pagination struts.

Only block container children support pagination struts.

Removed partial support for pagination struts on table captions, since it
didn't work too well (caused inconsistent layout and also overlap with the
table section that followed). Instead, set a strut on the content inside the
caption (first line, for instance), just like we already do for table cells.

We had a check to avoid setting pagination struts on table cells, but there was
no similar check for table captions. Replaced the check with examining the
containing block. We may only set a pagination strut on a block if its
containing block is a block container (LayoutBlockFlow). This automatically
fixes similar issues with flexboxes and possible other layout modes too.

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

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

[leviw_travelin_and_unemployed, 2015-09-22 17:20:31]

Some comment nits.

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

https://codereview.chromium.org/1360753002/diff/1/Source/core/layout/LayoutBl...
Source/core/layout/LayoutBlockFlow.cpp:784: // If we want to break before the
block, one final check is needed, since some block object
This is no longer one final check, and is really trying to say something similar
to what you're saying below (but better).

https://codereview.chromium.org/1360753002/diff/1/Source/core/layout/LayoutBl...
Source/core/layout/LayoutBlockFlow.cpp:788: if (block.isOutOfFlowPositioned())
Combine this with the above?

https://codereview.chromium.org/1360753002/diff/1/Source/core/layout/LayoutBl...
Source/core/layout/LayoutBlockFlow.cpp:792: // this limitation is simply that
block container child layout code is the only place where we
as you imply in your description, block container here is a block flow in Blink
jargon, no? I'd use that language for consistency.

[mstensho (USE GERRIT), 2015-09-22 18:11:50]

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

https://codereview.chromium.org/1360753002/diff/1/Source/core/layout/LayoutBl...
Source/core/layout/LayoutBlockFlow.cpp:784: // If we want to break before the
block, one final check is needed, since some block object
On 2015/09/22 17:20:31, leviw wrote:
> This is no longer one final check, and is really trying to say something
similar
> to what you're saying below (but better).

I shuffled things around a little bit, hopefully to your liking. :)

https://codereview.chromium.org/1360753002/diff/1/Source/core/layout/LayoutBl...
Source/core/layout/LayoutBlockFlow.cpp:788: if (block.isOutOfFlowPositioned())
On 2015/09/22 17:20:31, leviw wrote:
> Combine this with the above?

Done.

https://codereview.chromium.org/1360753002/diff/1/Source/core/layout/LayoutBl...
Source/core/layout/LayoutBlockFlow.cpp:792: // this limitation is simply that
block container child layout code is the only place where we
On 2015/09/22 17:20:31, leviw wrote:
> as you imply in your description, block container here is a block flow in
Blink
> jargon, no? I'd use that language for consistency.

Yeah, "block container" is CSS sp33k. Changing to native sp33k.

[leviw_travelin_and_unemployed, 2015-09-22 19:26:54]

The CQ bit was checked by leviw@chromium.org

[leviw_travelin_and_unemployed, 2015-09-22 19:26:55]

lgtm

Poi-fect!

[commit-bot: I haz the power, 2015-09-22 19:27:18]

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

[commit-bot: I haz the power, 2015-09-22 21:26:23]

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