Diff
Modified: trunk/LayoutTests/ChangeLog (167596 => 167597)
--- trunk/LayoutTests/ChangeLog 2014-04-21 18:06:20 UTC (rev 167596)
+++ trunk/LayoutTests/ChangeLog 2014-04-21 18:07:15 UTC (rev 167597)
@@ -1,3 +1,13 @@
+2014-04-21 David Hyatt <[email protected]>
+
+ [New Multicolumn] Pagination mode messed up with non-inline axis and reversed direction.
+ https://bugs.webkit.org/show_bug.cgi?id=131811
+
+ Reviewed by Dean Jackson.
+
+ * fast/multicol/newmulticol/compare-with-old-impl/BottomToTop-tb-expected.html: Added.
+ * fast/multicol/newmulticol/compare-with-old-impl/BottomToTop-tb.html: Added.
+
2014-04-21 Alexey Proskuryakov <[email protected]>
REGRESSION (r167530): ASSERT(m_selfTime <= m_totalTime) on tests that run after certain Inspector tests
Added: trunk/LayoutTests/fast/multicol/newmulticol/compare-with-old-impl/BottomToTop-tb-expected.html (0 => 167597)
--- trunk/LayoutTests/fast/multicol/newmulticol/compare-with-old-impl/BottomToTop-tb-expected.html (rev 0)
+++ trunk/LayoutTests/fast/multicol/newmulticol/compare-with-old-impl/BottomToTop-tb-expected.html 2014-04-21 18:07:15 UTC (rev 167597)
@@ -0,0 +1,44 @@
+<!DOCTYPE html>
+
+<html>
+<head>
+ <style>
+ body {
+ -webkit-writing-mode: horizontal-tb;
+ }
+ .box {
+ height: 50px;
+ width: 50px;
+ background-color: blue;
+ }
+ .box.changed {
+ background-color: green;
+ }
+ </style>
+ <script>
+ if (window.internals)
+ internals.setPagination("BottomToTopPaginated", 20, 180);
+ </script>
+</head>
+<body>
+
+<div class="box" _onclick_="this.classList.toggle('changed')"></div>
+<p>1 Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.</p>
+<div class="box" _onclick_="this.classList.toggle('changed')"></div>
+<p>2 Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.</p>
+<div class="box" _onclick_="this.classList.toggle('changed')"></div>
+<p>3 Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.</p>
+<div class="box" _onclick_="this.classList.toggle('changed')"></div>
+<p>4 Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum. </p>
+<div class="box" _onclick_="this.classList.toggle('changed')"></div>
+<p>5 Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.</p>
+<div class="box" _onclick_="this.classList.toggle('changed')"></div>
+<p>6 Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.</p>
+<div class="box" _onclick_="this.classList.toggle('changed')"></div>
+<p>7 Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum. </p>
+<div class="box" _onclick_="this.classList.toggle('changed')"></div>
+<p>8 Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum. </p>
+
+
+</body>
+</html>
Added: trunk/LayoutTests/fast/multicol/newmulticol/compare-with-old-impl/BottomToTop-tb.html (0 => 167597)
--- trunk/LayoutTests/fast/multicol/newmulticol/compare-with-old-impl/BottomToTop-tb.html (rev 0)
+++ trunk/LayoutTests/fast/multicol/newmulticol/compare-with-old-impl/BottomToTop-tb.html 2014-04-21 18:07:15 UTC (rev 167597)
@@ -0,0 +1,46 @@
+<!DOCTYPE html>
+
+<html>
+<head>
+ <style>
+ body {
+ -webkit-writing-mode: horizontal-tb;
+ }
+ .box {
+ height: 50px;
+ width: 50px;
+ background-color: blue;
+ }
+ .box.changed {
+ background-color: green;
+ }
+ </style>
+ <script>
+ if (window.internals) {
+ internals.settings.setRegionBasedColumnsEnabled(true);
+ internals.setPagination("BottomToTopPaginated", 20, 180);
+ }
+ </script>
+</head>
+<body>
+
+<div class="box" _onclick_="this.classList.toggle('changed')"></div>
+<p>1 Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.</p>
+<div class="box" _onclick_="this.classList.toggle('changed')"></div>
+<p>2 Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.</p>
+<div class="box" _onclick_="this.classList.toggle('changed')"></div>
+<p>3 Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.</p>
+<div class="box" _onclick_="this.classList.toggle('changed')"></div>
+<p>4 Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum. </p>
+<div class="box" _onclick_="this.classList.toggle('changed')"></div>
+<p>5 Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.</p>
+<div class="box" _onclick_="this.classList.toggle('changed')"></div>
+<p>6 Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.</p>
+<div class="box" _onclick_="this.classList.toggle('changed')"></div>
+<p>7 Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum. </p>
+<div class="box" _onclick_="this.classList.toggle('changed')"></div>
+<p>8 Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum. </p>
+
+
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (167596 => 167597)
--- trunk/Source/WebCore/ChangeLog 2014-04-21 18:06:20 UTC (rev 167596)
+++ trunk/Source/WebCore/ChangeLog 2014-04-21 18:07:15 UTC (rev 167597)
@@ -1,3 +1,48 @@
+2014-04-21 David Hyatt <[email protected]>
+
+ [New Multicolumn] Pagination mode messed up with non-inline axis and reversed direction.
+ https://bugs.webkit.org/show_bug.cgi?id=131811
+
+ Reviewed by Dean Jackson.
+
+ Added fast/multicol/newmulticol/compare-with-old-impl/BottomToTop-tb.html
+
+ With block axis pagination mode, it is possible to set a column height that is not the same
+ as the available fill height for a block. The new multi-column code had the assumption that
+ the column height was the same as the amount of fill room you had available. This is not
+ the case.
+
+ To correct the issue, I added a member variable to RenderMultiColumnSet that stores the
+ available column height as a separate variable from the computed column height. This allows
+ the pagination API to specify a different column height that is not the same as the view's
+ content height.
+
+ Even though it isn't involved in the solution, I also patched pageOrViewLogicalHeight on
+ RenderView to work with the new column code as well.
+
+ To address the layout test failures (that caused the previous rollout), I made sure to
+ initialize m_availableHeight to 0 when m_computedColumnHeight also gets reset to 0.
+
+ The assertion is not something I could reproduce on any machine, but I can see the problem.
+ I patched Page's pageCount method to not have column code directly in Page.cpp,
+ and to make a new pageCount() method on RenderView that Page calls
+ into. This method is now patched to handle the new column code as well as the old. I have
+ no real way of testing this method though, since I can't reproduce the assertion that the
+ bots were experiencing.
+
+ * page/Page.cpp:
+ (WebCore::Page::pageCount):
+ * rendering/RenderMultiColumnSet.cpp:
+ (WebCore::RenderMultiColumnSet::RenderMultiColumnSet):
+ (WebCore::RenderMultiColumnSet::setAndConstrainColumnHeight):
+ (WebCore::RenderMultiColumnSet::prepareForLayout):
+ (WebCore::RenderMultiColumnSet::computeLogicalHeight):
+ * rendering/RenderMultiColumnSet.h:
+ * rendering/RenderView.cpp:
+ (WebCore::RenderView::pageOrViewLogicalHeight):
+ (WebCore::RenderView::pageCount):
+ * rendering/RenderView.h:
+
2014-04-18 Dean Jackson <[email protected]>
[Media] Clean up localized strings in controls
Modified: trunk/Source/WebCore/page/Page.cpp (167596 => 167597)
--- trunk/Source/WebCore/page/Page.cpp 2014-04-21 18:06:20 UTC (rev 167596)
+++ trunk/Source/WebCore/page/Page.cpp 2014-04-21 18:07:15 UTC (rev 167597)
@@ -858,7 +858,7 @@
document->updateLayoutIgnorePendingStylesheets();
RenderView* contentRenderer = mainFrame().contentRenderer();
- return contentRenderer ? contentRenderer->columnCount(contentRenderer->columnInfo()) : 0;
+ return contentRenderer ? contentRenderer->pageCount() : 0;
}
void Page::setIsInWindow(bool isInWindow)
Modified: trunk/Source/WebCore/rendering/RenderMultiColumnSet.cpp (167596 => 167597)
--- trunk/Source/WebCore/rendering/RenderMultiColumnSet.cpp 2014-04-21 18:06:20 UTC (rev 167596)
+++ trunk/Source/WebCore/rendering/RenderMultiColumnSet.cpp 2014-04-21 18:07:15 UTC (rev 167597)
@@ -26,10 +26,12 @@
#include "config.h"
#include "RenderMultiColumnSet.h"
+#include "FrameView.h"
#include "PaintInfo.h"
#include "RenderLayer.h"
#include "RenderMultiColumnFlowThread.h"
#include "RenderMultiColumnSpannerPlaceholder.h"
+#include "RenderView.h"
namespace WebCore {
@@ -38,6 +40,7 @@
, m_computedColumnCount(1)
, m_computedColumnWidth(0)
, m_computedColumnHeight(0)
+ , m_availableColumnHeight(0)
, m_maxColumnHeight(RenderFlowThread::maxLogicalHeight())
, m_minSpaceShortage(RenderFlowThread::maxLogicalHeight())
, m_minimumColumnHeight(0)
@@ -151,6 +154,19 @@
m_computedColumnHeight = newHeight;
if (m_computedColumnHeight > m_maxColumnHeight)
m_computedColumnHeight = m_maxColumnHeight;
+
+ // FIXME: The available column height is not the same as the constrained height specified
+ // by the pagination API. The column set in this case is allowed to be bigger than the
+ // height of a single column. We cache available column height in order to use it
+ // in computeLogicalHeight later. This is pretty gross, and maybe there's a better way
+ // to formalize the idea of clamped column heights without having a view dependency
+ // here.
+ m_availableColumnHeight = m_computedColumnHeight;
+ if (multiColumnFlowThread() && !multiColumnFlowThread()->progressionIsInline() && parent()->isRenderView()) {
+ int pageLength = view().frameView().pagination().pageLength;
+ if (pageLength)
+ m_computedColumnHeight = pageLength;
+ }
// FIXME: the height may also be affected by the enclosing pagination context, if any.
}
@@ -327,8 +343,10 @@
if (initial)
m_maxColumnHeight = calculateMaxColumnHeight();
if (requiresBalancing()) {
- if (initial)
+ if (initial) {
m_computedColumnHeight = 0;
+ m_availableColumnHeight = 0;
+ }
} else
setAndConstrainColumnHeight(heightAdjustedForSetOffset(multiColumnFlowThread()->columnHeightAvailable()));
@@ -396,7 +414,7 @@
void RenderMultiColumnSet::computeLogicalHeight(LayoutUnit, LayoutUnit logicalTop, LogicalExtentComputedValues& computedValues) const
{
- computedValues.m_extent = m_computedColumnHeight;
+ computedValues.m_extent = m_availableColumnHeight;
computedValues.m_position = logicalTop;
}
Modified: trunk/Source/WebCore/rendering/RenderMultiColumnSet.h (167596 => 167597)
--- trunk/Source/WebCore/rendering/RenderMultiColumnSet.h 2014-04-21 18:06:20 UTC (rev 167596)
+++ trunk/Source/WebCore/rendering/RenderMultiColumnSet.h 2014-04-21 18:07:15 UTC (rev 167597)
@@ -178,6 +178,7 @@
unsigned m_computedColumnCount; // Used column count (the resulting 'N' from the pseudo-algorithm in the multicol spec)
LayoutUnit m_computedColumnWidth; // Used column width (the resulting 'W' from the pseudo-algorithm in the multicol spec)
LayoutUnit m_computedColumnHeight;
+ LayoutUnit m_availableColumnHeight;
// The following variables are used when balancing the column set.
LayoutUnit m_maxColumnHeight; // Maximum column height allowed.
Modified: trunk/Source/WebCore/rendering/RenderView.cpp (167596 => 167597)
--- trunk/Source/WebCore/rendering/RenderView.cpp 2014-04-21 18:06:20 UTC (rev 167596)
+++ trunk/Source/WebCore/rendering/RenderView.cpp 2014-04-21 18:07:15 UTC (rev 167597)
@@ -295,7 +295,7 @@
if (document().printing())
return pageLogicalHeight();
- if (hasColumns() && !style().hasInlineColumnAxis()) {
+ if ((hasColumns() || multiColumnFlowThread()) && !style().hasInlineColumnAxis()) {
if (int pageLength = frameView().pagination().pageLength)
return pageLength;
}
@@ -1257,4 +1257,18 @@
return columnNumber;
}
+unsigned RenderView::pageCount() const
+{
+ const Pagination& pagination = frameView().frame().page()->pagination();
+ if (pagination.mode == Pagination::Unpaginated)
+ return 0;
+
+ if (hasColumns())
+ return columnCount(columnInfo());
+ if (multiColumnFlowThread())
+ return multiColumnFlowThread()->columnCount();
+
+ return 0;
+}
+
} // namespace WebCore
Modified: trunk/Source/WebCore/rendering/RenderView.h (167596 => 167597)
--- trunk/Source/WebCore/rendering/RenderView.h 2014-04-21 18:06:20 UTC (rev 167596)
+++ trunk/Source/WebCore/rendering/RenderView.h 2014-04-21 18:07:15 UTC (rev 167597)
@@ -160,6 +160,8 @@
// function and should not be mistaken for a general page number API.
unsigned pageNumberForBlockProgressionOffset(int offset) const;
+ unsigned pageCount() const;
+
// FIXME: These functions are deprecated. No code should be added that uses these.
int bestTruncatedAt() const { return m_legacyPrinting.m_bestTruncatedAt; }
void setBestTruncatedAt(int y, RenderBoxModelObject* forRenderer, bool forcedBreak = false);
