Commit 86d53e7b authored by mstensho's avatar mstensho Committed by Commit bot

Adjust column rows' height better for their offset in the multicol container.

We were missing the case where the first object in a multicol container was a
spanner (the call to previousSiblingMultiColumnSet() should have been
previousSiblingMultiColumnBox(), to catch spanner placeholders in addition to
column sets).

But instead of having a special code path depending on whether we're dealing
with the first box or not (to avoid subtracting the multicol container's top
border and padding from an uncalculated logical top of a column set), always
subtract the margin top edge of the first column box instead.

R=leviw@chromium.org

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

Cr-Commit-Position: refs/heads/master@{#371661}
parent 787c2d79
<!DOCTYPE html>
<script src="../../../resources/check-layout.js"></script>
<p>There should be a blue square below.</p>
<div id="multicol" style="position:relative; padding:25px; -webkit-columns:2; -webkit-column-gap:0; column-fill:auto; line-height:20px; width:60px; height:100px;">
<div style="-webkit-column-span:all; margin-top:20px; height:20px;"></div>
<div style="background:blue;">
<div data-offset-x="25" data-offset-y="65"><br></div>
<div data-offset-x="25" data-offset-y="85"><br></div>
<div data-offset-x="25" data-offset-y="105"><br></div>
<div data-offset-x="55" data-offset-y="65"><br></div>
<div data-offset-x="55" data-offset-y="85"><br></div>
<div data-offset-x="55" data-offset-y="105"><br></div>
</div>
</div>
<script>
checkLayout("#multicol");
</script>
......@@ -170,6 +170,22 @@ MultiColumnFragmentainerGroup& LayoutMultiColumnSet::appendNewFragmentainerGroup
return m_fragmentainerGroups.last();
}
LayoutUnit LayoutMultiColumnSet::logicalTopFromMulticolContentEdge() const
{
// We subtract the position of the first column set or spanner placeholder, rather than the
// "before" border+padding of the multicol container. This distinction doesn't matter after
// layout, but during layout it does: The flow thread (i.e. the multicol contents) is laid out
// before the column sets and spanner placeholders, which means that compesating for a top
// border+padding that hasn't yet been baked into the offset will produce the wrong results in
// the first layout pass, and we'd end up performing a wasted layout pass in many cases.
const LayoutBox& firstColumnBox = *multiColumnFlowThread()->firstMultiColumnBox();
// The top margin edge of the first column set or spanner placeholder is flush with the top
// content edge of the multicol container. The margin here never collapses with other margins,
// so we can just subtract it. Column sets never have margins, but spanner placeholders may.
LayoutUnit firstColumnBoxMarginEdge = firstColumnBox.logicalTop() - multiColumnBlockFlow()->marginBeforeForChild(firstColumnBox);
return logicalTop() - firstColumnBoxMarginEdge;
}
LayoutUnit LayoutMultiColumnSet::logicalTopInFlowThread() const
{
return firstFragmentainerGroup().logicalTopInFlowThread();
......
......@@ -101,6 +101,9 @@ public:
MultiColumnFragmentainerGroup& appendNewFragmentainerGroup();
// Logical top relative to the content edge of the multicol container.
LayoutUnit logicalTopFromMulticolContentEdge() const;
LayoutUnit logicalTopInFlowThread() const;
LayoutUnit logicalBottomInFlowThread() const;
LayoutUnit logicalHeightInFlowThread() const { return logicalBottomInFlowThread() - logicalTopInFlowThread(); }
......
......@@ -292,21 +292,8 @@ unsigned MultiColumnFragmentainerGroup::actualColumnCount() const
LayoutUnit MultiColumnFragmentainerGroup::heightAdjustedForRowOffset(LayoutUnit height) const
{
// Adjust for the top offset within the content box of the multicol container (containing
// block), unless we're in the first set. We know that the top offset for the first set will be
// zero, but if the multicol container has non-zero top border or padding, the set's top offset
// (initially being 0 and relative to the border box) will be negative until it has been laid
// out. Had we used this bogus offset, we would calculate the wrong height, and risk performing
// a wasted layout iteration. Of course all other sets (if any) have this problem in the first
// layout pass too, but there's really nothing we can do there until the flow thread has been
// laid out anyway.
if (m_columnSet.previousSiblingMultiColumnSet()) {
LayoutBlockFlow* multicolBlock = m_columnSet.multiColumnBlockFlow();
LayoutUnit contentLogicalTop = m_columnSet.logicalTop() - multicolBlock->borderAndPaddingBefore();
height -= contentLogicalTop;
}
height -= logicalTop();
return max(height, LayoutUnit(1)); // Let's avoid zero height, as that would probably cause an infinite amount of columns to be created.
// Let's avoid zero height, as that would cause an infinite amount of columns to be created.
return std::max(height - logicalTop() - m_columnSet.logicalTopFromMulticolContentEdge(), LayoutUnit(1));
}
LayoutUnit MultiColumnFragmentainerGroup::calculateMaxColumnHeight() const
......
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