Changeset 167478 in webkit

Timestamp:

Apr 17, 2014 7:11:12 PM (10 years ago)

Author:

hyatt@apple.com

Message:

[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.

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.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/multicol/newmulticol/compare-with-old-impl/BottomToTop-tb-expected.html (added)
• LayoutTests/fast/multicol/newmulticol/compare-with-old-impl/BottomToTop-tb.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/RenderMultiColumnSet.cpp (modified) (4 diffs)
• Source/WebCore/rendering/RenderMultiColumnSet.h (modified) (1 diff)
• Source/WebCore/rendering/RenderView.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/rendering/RenderMultiColumnSet.cpp

@@ -27 +27 @@
 #include "RenderMultiColumnSet.h"
 
+#include "FrameView.h"
 #include "PaintInfo.h"
 #include "RenderLayer.h"
 #include "RenderMultiColumnFlowThread.h"
 #include "RenderMultiColumnSpannerPlaceholder.h"
+#include "RenderView.h"
 
 namespace WebCore {
@@ -39 +41 @@
     , m_computedColumnWidth(0)
     , m_computedColumnHeight(0)
+    , m_availableColumnHeight(0)
     , m_maxColumnHeight(RenderFlowThread::maxLogicalHeight())
     , m_minSpaceShortage(RenderFlowThread::maxLogicalHeight())
@@ -152 +155 @@
     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.
 }
@@ -397 +413 @@
 void RenderMultiColumnSet::computeLogicalHeight(LayoutUnit, LayoutUnit logicalTop, LogicalExtentComputedValues& computedValues) const
 {
-    computedValues.m_extent = m_computedColumnHeight;
+    computedValues.m_extent = m_availableColumnHeight;
     computedValues.m_position = logicalTop;
 }

trunk/Source/WebCore/rendering/RenderMultiColumnSet.h

@@ -179 +179 @@
     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.

trunk/Source/WebCore/rendering/RenderView.cpp

@@ -296 +296 @@
         return pageLogicalHeight();
     
-    if (hasColumns() && !style().hasInlineColumnAxis()) {
+    if ((hasColumns() || multiColumnFlowThread()) && !style().hasInlineColumnAxis()) {
         if (int pageLength = frameView().pagination().pageLength)
             return pageLength;