[New Multicolumn] Pagination mode messed up with non-inline axis and reversed direction.
authorhyatt@apple.com <hyatt@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 18 Apr 2014 02:11:12 +0000 (02:11 +0000)
committerhyatt@apple.com <hyatt@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 18 Apr 2014 02:11:12 +0000 (02:11 +0000)
https://bugs.webkit.org/show_bug.cgi?id=131811

Reviewed by Dean Jackson.

Source/WebCore:
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.

* rendering/RenderMultiColumnSet.cpp:
(WebCore::RenderMultiColumnSet::RenderMultiColumnSet):
(WebCore::RenderMultiColumnSet::setAndConstrainColumnHeight):
(WebCore::RenderMultiColumnSet::computeLogicalHeight):
* rendering/RenderMultiColumnSet.h:
* rendering/RenderView.cpp:
(WebCore::RenderView::pageOrViewLogicalHeight):

LayoutTests:
* fast/multicol/newmulticol/compare-with-old-impl/BottomToTop-tb-expected.html: Added.
* fast/multicol/newmulticol/compare-with-old-impl/BottomToTop-tb.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@167478 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/multicol/newmulticol/compare-with-old-impl/BottomToTop-tb-expected.html [new file with mode: 0644]
LayoutTests/fast/multicol/newmulticol/compare-with-old-impl/BottomToTop-tb.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderMultiColumnSet.cpp
Source/WebCore/rendering/RenderMultiColumnSet.h
Source/WebCore/rendering/RenderView.cpp

index 646b978..9307f96 100644 (file)
@@ -1,3 +1,13 @@
+2014-04-17  David Hyatt  <hyatt@apple.com>
+
+        [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-16  Huang Dongsung  <luxtella@company100.net>
 
         Make RenderLayerBacking get the timingFunction of the correct animation.
diff --git a/LayoutTests/fast/multicol/newmulticol/compare-with-old-impl/BottomToTop-tb-expected.html b/LayoutTests/fast/multicol/newmulticol/compare-with-old-impl/BottomToTop-tb-expected.html
new file mode 100644 (file)
index 0000000..2df0e8b
--- /dev/null
@@ -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>
diff --git a/LayoutTests/fast/multicol/newmulticol/compare-with-old-impl/BottomToTop-tb.html b/LayoutTests/fast/multicol/newmulticol/compare-with-old-impl/BottomToTop-tb.html
new file mode 100644 (file)
index 0000000..08e3ac4
--- /dev/null
@@ -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>
index 7ee5554..b628437 100644 (file)
@@ -1,3 +1,33 @@
+2014-04-17  David Hyatt  <hyatt@apple.com>
+
+        [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.
+
+        * rendering/RenderMultiColumnSet.cpp:
+        (WebCore::RenderMultiColumnSet::RenderMultiColumnSet):
+        (WebCore::RenderMultiColumnSet::setAndConstrainColumnHeight):
+        (WebCore::RenderMultiColumnSet::computeLogicalHeight):
+        * rendering/RenderMultiColumnSet.h:
+        * rendering/RenderView.cpp:
+        (WebCore::RenderView::pageOrViewLogicalHeight):
+
 2014-04-17  Anders Carlsson  <andersca@apple.com>
 
         Build fix.
index 46cb64b..a6d2901 100644 (file)
 #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 @@ RenderMultiColumnSet::RenderMultiColumnSet(RenderFlowThread& flowThread, PassRef
     , 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 @@ void RenderMultiColumnSet::setAndConstrainColumnHeight(LayoutUnit newHeight)
     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.
 }
 
@@ -396,7 +412,7 @@ void RenderMultiColumnSet::layout()
 
 void RenderMultiColumnSet::computeLogicalHeight(LayoutUnit, LayoutUnit logicalTop, LogicalExtentComputedValues& computedValues) const
 {
-    computedValues.m_extent = m_computedColumnHeight;
+    computedValues.m_extent = m_availableColumnHeight;
     computedValues.m_position = logicalTop;
 }
 
index 331ea6c..956761c 100644 (file)
@@ -178,6 +178,7 @@ private:
     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.
index df40522..a528a97 100644 (file)
@@ -295,7 +295,7 @@ LayoutUnit RenderView::pageOrViewLogicalHeight() const
     if (document().printing())
         return pageLogicalHeight();
     
-    if (hasColumns() && !style().hasInlineColumnAxis()) {
+    if ((hasColumns() || multiColumnFlowThread()) && !style().hasInlineColumnAxis()) {
         if (int pageLength = frameView().pagination().pageLength)
             return pageLength;
     }