Skip to content

GitLab

T
tangled
Commit e4fae21f authored by mstensho@opera.com's avatar mstensho@opera.com
Browse files
Options

[New Multicolumn] balancer confused by content with top margins. 

We need to set zero page logical height in LayoutState when column
height is unknown (when the columns haven't yet been
balanced). There's code that assumes that non-zero page height means
that page height is known. Lying about this makes the pagination code
believe that every top margin is adjacent to a column break, which
makes it eat and ignore all top margins.

This should be cleaned up, but it's easier to wait until the old
multicol code has been removed.

BUG=353587

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

git-svn-id: svn://svn.chromium.org/blink/trunk@170335 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent 8f28a4c1
Hide whitespace changes
Inline Side-by-side
Showing with 129 additions and 5 deletions
third_party/WebKit/LayoutTests/fast/multicol/break-in-scrollable-expected.html 0 → 100644
View file @ e4fae21f
<!DOCTYPE html>
<html>
<head>
<title>Forced break inside scrollable container</title>
<script>
if (window.internals)
internals.settings.setRegionBasedColumnsEnabled(true);
</script>
</head>
<body>
<p>The word 'PASS' should be seen on the left below. The scrollbars shouldn't be scrollable.</p>
<div style="-webkit-columns:2; columns:2; orphans:1; widows:1;">
<div style="overflow:scroll; height:10em;">
&nbsp;
<div>PASS</div>
</div>
</div>
</body>
</html>
third_party/WebKit/LayoutTests/fast/multicol/break-in-scrollable.html 0 → 100644
View file @ e4fae21f
<!DOCTYPE html>
<html>
<head>
<title>Forced break inside scrollable container</title>
<script>
if (window.internals)
internals.settings.setRegionBasedColumnsEnabled(true);
</script>
</head>
<body>
<p>The word 'PASS' should be seen on the left below. The scrollbars shouldn't be scrollable.</p>
<div style="-webkit-columns:2; columns:2; orphans:1; widows:1;">
<div style="overflow:scroll; height:10em;">
&nbsp;
<div style="-webkit-column-break-before:always; break-before:column;">PASS</div>
</div>
</div>
</body>
</html>
third_party/WebKit/LayoutTests/fast/multicol/newmulticol/leading-and-trailing-margin-expected.html 0 → 100644
View file @ e4fae21f
<!DOCTYPE html>
<html>
<head>
<title>Leading and trailing margin</title>
<script>
if (window.internals)
internals.settings.setRegionBasedColumnsEnabled(true);
</script>
</head>
<body>
<p>There should be no scrollbars.</p>
</body>
</html>
third_party/WebKit/LayoutTests/fast/multicol/newmulticol/leading-and-trailing-margin.html 0 → 100644
View file @ e4fae21f
<!DOCTYPE html>
<html>
<head>
<title>Leading and trailing margin</title>
<script>
if (window.internals)
internals.settings.setRegionBasedColumnsEnabled(true);
</script>
</head>
<body>
<p>There should be no scrollbars.</p>
<div style="-webkit-columns:3; columns:3;">
<div style="margin-top:1em; margin-bottom:10em;">&nbsp;</div>
</div>
</body>
</html>
third_party/WebKit/LayoutTests/fast/multicol/newmulticol/leading-margin-expected.html 0 → 100644
View file @ e4fae21f
<!DOCTYPE html>
<html>
<head>
<title>Leading margins</title>
</head>
<body>
<p>There should be no scrollbars.</p>
</body>
</html>
third_party/WebKit/LayoutTests/fast/multicol/newmulticol/leading-margin.html 0 → 100644
View file @ e4fae21f
<!DOCTYPE html>
<html>
<head>
<title>Leading margins</title>
<script>
if (window.internals)
internals.settings.setRegionBasedColumnsEnabled(true);
</script>
<style>
.test { margin-top:100px; border:2px solid transparent; }
</style>
</head>
<body>
<p>There should be no scrollbars.</p>
<div style="-webkit-column-count:2; column-count:2;">
<div class="test"></div>
<div class="test"></div>
<div class="test"></div>
</div>
</body>
</html>
third_party/WebKit/Source/core/rendering/LayoutState.cpp
View file @ e4fae21f
...@@ -88,6 +88,7 @@ LayoutState::LayoutState(LayoutState* prev, RenderBox& renderer, const LayoutSiz ...@@ -88,6 +88,7 @@ LayoutState::LayoutState(LayoutState* prev, RenderBox& renderer, const LayoutSiz
m_pageOffset = LayoutSize(m_layoutOffset.width() + (!isFlipped ? renderer.borderLeft() + renderer.paddingLeft() : renderer.borderRight() + renderer.paddingRight()), m_pageOffset = LayoutSize(m_layoutOffset.width() + (!isFlipped ? renderer.borderLeft() + renderer.paddingLeft() : renderer.borderRight() + renderer.paddingRight()),
m_layoutOffset.height() + (!isFlipped ? renderer.borderTop() + renderer.paddingTop() : renderer.borderBottom() + renderer.paddingBottom())); m_layoutOffset.height() + (!isFlipped ? renderer.borderTop() + renderer.paddingTop() : renderer.borderBottom() + renderer.paddingBottom()));
m_pageLogicalHeightChanged = pageLogicalHeightChanged; m_pageLogicalHeightChanged = pageLogicalHeightChanged;
m_isPaginated = true;
} else { } else {
// If we don't establish a new page height, then propagate the old page height and offset down. // If we don't establish a new page height, then propagate the old page height and offset down.
m_pageLogicalHeight = m_next->m_pageLogicalHeight; m_pageLogicalHeight = m_next->m_pageLogicalHeight;
...@@ -96,8 +97,12 @@ LayoutState::LayoutState(LayoutState* prev, RenderBox& renderer, const LayoutSiz ...@@ -96,8 +97,12 @@ LayoutState::LayoutState(LayoutState* prev, RenderBox& renderer, const LayoutSiz
// Disable pagination for objects we don't support. For now this includes overflow:scroll/auto, inline blocks and // Disable pagination for objects we don't support. For now this includes overflow:scroll/auto, inline blocks and
// writing mode roots. // writing mode roots.
if (renderer.isUnsplittableForPagination()) if (renderer.isUnsplittableForPagination()) {
m_pageLogicalHeight = 0; m_pageLogicalHeight = 0;
m_isPaginated = false;
} else {
m_isPaginated = m_pageLogicalHeight || m_next->m_columnInfo || renderer.flowThreadContainingBlock();
}
} }
if (!m_columnInfo) if (!m_columnInfo)
...@@ -111,8 +116,6 @@ LayoutState::LayoutState(LayoutState* prev, RenderBox& renderer, const LayoutSiz ...@@ -111,8 +116,6 @@ LayoutState::LayoutState(LayoutState* prev, RenderBox& renderer, const LayoutSiz
#endif #endif
} }
m_isPaginated = m_pageLogicalHeight || m_columnInfo || renderer.isRenderFlowThread();
// FIXME: <http://bugs.webkit.org/show_bug.cgi?id=13443> Apply control clip if present. // FIXME: <http://bugs.webkit.org/show_bug.cgi?id=13443> Apply control clip if present.
} }
......
third_party/WebKit/Source/core/rendering/RenderBlockFlow.cpp
View file @ e4fae21f
...@@ -228,8 +228,20 @@ void RenderBlockFlow::checkForPaginationLogicalHeightChange(LayoutUnit& pageLogi ...@@ -228,8 +228,20 @@ void RenderBlockFlow::checkForPaginationLogicalHeightChange(LayoutUnit& pageLogi
if (!hasSpecifiedPageLogicalHeight && !pageLogicalHeight) if (!hasSpecifiedPageLogicalHeight && !pageLogicalHeight)
colInfo->clearForcedBreaks(); colInfo->clearForcedBreaks();
} else if (isRenderFlowThread()) { } else if (isRenderFlowThread()) {
pageLogicalHeight = 1; // This is just a hack to always make sure we have a page logical height. RenderFlowThread* flowThread = toRenderFlowThread(this);
pageLogicalHeightChanged = toRenderFlowThread(this)->pageLogicalSizeChanged();
// FIXME: This is a hack to always make sure we have a page logical height, if said height
// is known. The page logical height thing in LayoutState is meaningless for flow
// thread-based pagination (page height isn't necessarily uniform throughout the flow
// thread), but as long as it is used universally as a means to determine whether page
// height is known or not, we need this. Page height is unknown when column balancing is
// enabled and flow thread height is still unknown (i.e. during the first layout pass). When
// it's unknown, we need to prevent the pagination code from assuming page breaks everywhere
// and thereby eating every top margin. It should be trivial to clean up and get rid of this
// hack once the old multicol implementation is gone.
pageLogicalHeight = flowThread->isPageLogicalHeightKnown() ? LayoutUnit(1) : LayoutUnit(0);
pageLogicalHeightChanged = flowThread->pageLogicalSizeChanged();
} }
} }
......
third_party/WebKit/Source/core/rendering/RenderFlowThread.h
View file @ e4fae21f
...@@ -118,6 +118,7 @@ public: ...@@ -118,6 +118,7 @@ public:
virtual bool addForcedRegionBreak(LayoutUnit, RenderObject* breakChild, bool isBefore, LayoutUnit* offsetBreakAdjustment = 0) { return false; } virtual bool addForcedRegionBreak(LayoutUnit, RenderObject* breakChild, bool isBefore, LayoutUnit* offsetBreakAdjustment = 0) { return false; }
void applyBreakAfterContent(LayoutUnit); void applyBreakAfterContent(LayoutUnit);
virtual bool isPageLogicalHeightKnown() const { return true; }
bool pageLogicalSizeChanged() const { return m_pageLogicalSizeChanged; } bool pageLogicalSizeChanged() const { return m_pageLogicalSizeChanged; }
void collectLayerFragments(LayerFragments&, const LayoutRect& layerBoundingBox, const LayoutRect& dirtyRect); void collectLayerFragments(LayerFragments&, const LayoutRect& layerBoundingBox, const LayoutRect& dirtyRect);
......
third_party/WebKit/Source/core/rendering/RenderMultiColumnFlowThread.cpp
View file @ e4fae21f
...@@ -229,4 +229,14 @@ bool RenderMultiColumnFlowThread::addForcedRegionBreak(LayoutUnit offset, Render ...@@ -229,4 +229,14 @@ bool RenderMultiColumnFlowThread::addForcedRegionBreak(LayoutUnit offset, Render
return false; return false;
} }
bool RenderMultiColumnFlowThread::isPageLogicalHeightKnown() const
{
for (RenderBox* renderer = parentBox()->lastChildBox(); renderer; renderer = renderer->previousSiblingBox()) {
if (renderer->isRenderMultiColumnSet())
return toRenderMultiColumnSet(renderer)->computedColumnHeight();
}
// A column set hasn't been created yet. Height may already be known if column-fill is 'auto', though.
return !requiresBalancing();
}
} }
third_party/WebKit/Source/core/rendering/RenderMultiColumnFlowThread.h
View file @ e4fae21f
...@@ -58,6 +58,7 @@ private: ...@@ -58,6 +58,7 @@ private:
virtual void setPageBreak(LayoutUnit offset, LayoutUnit spaceShortage) OVERRIDE; virtual void setPageBreak(LayoutUnit offset, LayoutUnit spaceShortage) OVERRIDE;
virtual void updateMinimumPageHeight(LayoutUnit offset, LayoutUnit minHeight) OVERRIDE; virtual void updateMinimumPageHeight(LayoutUnit offset, LayoutUnit minHeight) OVERRIDE;
virtual bool addForcedRegionBreak(LayoutUnit, RenderObject* breakChild, bool isBefore, LayoutUnit* offsetBreakAdjustment = 0) OVERRIDE; virtual bool addForcedRegionBreak(LayoutUnit, RenderObject* breakChild, bool isBefore, LayoutUnit* offsetBreakAdjustment = 0) OVERRIDE;
virtual bool isPageLogicalHeightKnown() const OVERRIDE;
unsigned m_columnCount; // The used value of column-count unsigned m_columnCount; // The used value of column-count
LayoutUnit m_columnWidth; // The used value of column-width LayoutUnit m_columnWidth; // The used value of column-width
......
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