Commit a0c1edf2 authored by mstensho's avatar mstensho Committed by Commit bot

Don't set bogus height on new fragmentainer groups initially.

This essentially made it impossible to support more than two column rows in
auto-height multicol containers.

When a new fragmentainer group is created, resetColumnHeight() is called right
*before* the fragmentainer group is inserted into the array, so we'd trick
ourselves into believing that height always was non-auto, because the
heightIsAuto() method on MultiColumnFragmentainerGroup would require the
fragmentainer group to be the last one in the array to count as auto.

We could fix the order in which things are done, or just make the whole thing a
bit more robust, which this CL aims to do.

BUG=447718
R=leviw@chromium.org

Review URL: https://codereview.chromium.org/1424913003

Cr-Commit-Position: refs/heads/master@{#360540}
parent 0917ec43
Test support for more than two inner column rows, when column height is auto.
Below there should be a papayawhip box with two lines of text with large letter spacing.
The first line should read "Roger" and the second one "Wilco".
No red should be seen.
R
W
o
i
g
l
e
c
r
o
XXXX
XXXX
PASS
<!DOCTYPE html>
<script src="../../resources/check-layout.js"></script>
<p>Test support for more than two inner column rows, when column height is auto.</p>
<p>Below there should be a papayawhip box with two lines of text with large letter spacing.</p>
<p>The first line should read "Roger" and the second one "Wilco".</p>
<p>No red should be seen.</p>
<div id="outer" style="position:relative; -webkit-columns:3; -webkit-column-gap:0; text-align:center; overflow:hidden; column-fill:auto; line-height:40px; width:180px; height:80px; background:red;">
<div style="-webkit-columns:2; -webkit-column-gap:0; column-fill:auto; background:papayawhip;" data-expected-height="240">
<div data-offset-x="0" data-offset-y="0">R</div><div data-offset-x="0" data-offset-y="40">W</div>
<div data-offset-x="30" data-offset-y="0">o</div><div data-offset-x="30" data-offset-y="40">i</div>
<div data-offset-x="60" data-offset-y="0">g</div><div data-offset-x="60" data-offset-y="40">l</div>
<div data-offset-x="90" data-offset-y="0">e</div><div data-offset-x="90" data-offset-y="40">c</div>
<div data-offset-x="120" data-offset-y="0">r</div><div data-offset-x="120" data-offset-y="40">o</div>
<div><br></div><div><br></div>
</div>
<div data-offset-x="180">XXXX</div>
<div data-offset-x="180">XXXX</div>
</div>
<script>
checkLayout("#outer");
</script>
......@@ -39,20 +39,12 @@ LayoutUnit MultiColumnFragmentainerGroup::blockOffsetInEnclosingFlowThread() con
return logicalTop() + m_columnSet.logicalTop() + m_columnSet.multiColumnFlowThread()->blockOffsetInEnclosingFlowThread();
}
bool MultiColumnFragmentainerGroup::heightIsAuto() const
{
// Only the last row may have auto height, and thus be balanced. There are no good reasons to
// balance the preceding rows, and that could potentially lead to an insane number of layout
// passes as well.
return isLastGroup() && m_columnSet.heightIsAuto();
}
void MultiColumnFragmentainerGroup::resetColumnHeight()
{
m_maxColumnHeight = calculateMaxColumnHeight();
LayoutMultiColumnFlowThread* flowThread = m_columnSet.multiColumnFlowThread();
if (heightIsAuto()) {
if (m_columnSet.heightIsAuto()) {
LayoutMultiColumnFlowThread* enclosingFlowThread = flowThread->enclosingFlowThread();
if (enclosingFlowThread && enclosingFlowThread->isPageLogicalHeightKnown()) {
// Even if height is auto, we set an initial height, in order to tell how much content
......@@ -72,7 +64,10 @@ bool MultiColumnFragmentainerGroup::recalculateColumnHeight(BalancedColumnHeight
m_maxColumnHeight = calculateMaxColumnHeight();
if (heightIsAuto()) {
// Only the last row may have auto height, and thus be balanced. There are no good reasons to
// balance the preceding rows, and that could potentially lead to an insane number of layout
// passes as well.
if (isLastGroup() && m_columnSet.heightIsAuto()) {
LayoutUnit newColumnHeight = calculateColumnHeight(calculationMode);
setAndConstrainColumnHeight(newColumnHeight);
// After having calculated an initial column height, the multicol container typically needs at
......
......@@ -59,7 +59,6 @@ public:
// The height of our flow thread portion
LayoutUnit logicalHeightInFlowThread() const { return m_logicalBottomInFlowThread - m_logicalTopInFlowThread; }
bool heightIsAuto() const;
void resetColumnHeight();
bool recalculateColumnHeight(BalancedColumnHeightCalculation calculationMode);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment