apply cross axis constraints before aligning children in flexbox

https://bugs.webkit.org/show_bug.cgi?id=82240

Reviewed by Ojan Vafai.

Source/WebCore:

We weren't applying max-height constraints before aligning children.
This would cause center, end, stretch alignment to be wrong if we hit
a max-height on a flexbox.

This patch also moves the repositioning logic to happen after
computeLogicalHeight, which will be useful for flex-line-pack.

New test case in css3/flexbox/flex-align.html

* rendering/RenderFlexibleBox.cpp:
(WebCore::RenderFlexibleBox::layoutBlock):
(WebCore::RenderFlexibleBox::layoutFlexItems):
(WebCore::RenderFlexibleBox::layoutAndPlaceChildren):
* rendering/RenderFlexibleBox.h:

LayoutTests:

* css3/flexbox/flex-align-expected.txt:
* css3/flexbox/flex-align.html:

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

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 662ef0f..b0dce1f 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,13 @@
+2012-03-26  Tony Chang  <tony@chromium.org>
+
+        apply cross axis constraints before aligning children in flexbox
+        https://bugs.webkit.org/show_bug.cgi?id=82240
+
+        Reviewed by Ojan Vafai.
+
+        * css3/flexbox/flex-align-expected.txt:
+        * css3/flexbox/flex-align.html:
+
 2012-03-26  Ojan Vafai  <ojan@chromium.org>
 
         The css3/selectors3/xml tests are flaky on Leopard as well.

diff --git a/LayoutTests/css3/flexbox/flex-align-expected.txt b/LayoutTests/css3/flexbox/flex-align-expected.txt
index 8ead7bc..d982755 100644 (file)
--- a/LayoutTests/css3/flexbox/flex-align-expected.txt
+++ b/LayoutTests/css3/flexbox/flex-align-expected.txt
@@ -19,6 +19,7 @@ PASS
 PASS
 PASS
 PASS
+PASS
 ahem
 
 

diff --git a/LayoutTests/css3/flexbox/flex-align.html b/LayoutTests/css3/flexbox/flex-align.html
index 1d59d2c..ec3d5e0 100644 (file)
--- a/LayoutTests/css3/flexbox/flex-align.html
+++ b/LayoutTests/css3/flexbox/flex-align.html
@@ -222,6 +222,13 @@ if (window.layoutTestController)
   <div data-expected-height="100" data-offset-y="0" style="width: -webkit-flex(1 0 0); height: 100px;"></div>
 </div>
 
+<div class="flexbox" style="max-height: 20px">
+  <div data-expected-height="40" data-offset-y="-10" style="width: -webkit-flex(1 0 0); -webkit-flex-item-align: center; height: 40px;"></div>
+  <div data-expected-height="40" data-offset-y="-20" style="width: -webkit-flex(1 0 0); -webkit-flex-item-align: end; height: 40px;"></div>
+  <div data-expected-height="40" data-offset-y="0" style="width: -webkit-flex(1 0 0); height: 40px;"></div>
+  <div data-expected-height="20" data-offset-y="0" style="width: -webkit-flex(1 0 0);"></div>
+</div>
+
 <div class="flexbox" style="font-family: test;">
   <div id="baseline1" style="width: -webkit-flex(1 0 0); -webkit-flex-item-align: baseline;">ahem</div>
   <div id="baseline2" data-offset-y="0" style="width: -webkit-flex(1 0 0); -webkit-flex-item-align: baseline;"><img src="../../fast/replaced/resources/1x1-blue.png" style="height: 50px;"></div>

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 68ecf10..235c1c7 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,25 @@
+2012-03-26  Tony Chang  <tony@chromium.org>
+
+        apply cross axis constraints before aligning children in flexbox
+        https://bugs.webkit.org/show_bug.cgi?id=82240
+
+        Reviewed by Ojan Vafai.
+
+        We weren't applying max-height constraints before aligning children.
+        This would cause center, end, stretch alignment to be wrong if we hit
+        a max-height on a flexbox.
+
+        This patch also moves the repositioning logic to happen after
+        computeLogicalHeight, which will be useful for flex-line-pack.
+
+        New test case in css3/flexbox/flex-align.html
+
+        * rendering/RenderFlexibleBox.cpp:
+        (WebCore::RenderFlexibleBox::layoutBlock):
+        (WebCore::RenderFlexibleBox::layoutFlexItems):
+        (WebCore::RenderFlexibleBox::layoutAndPlaceChildren):
+        * rendering/RenderFlexibleBox.h:
+
 2012-03-26  Anders Carlsson  <andersca@apple.com>
 
         Find in page overlay and bouncy are not always positioned correctly

diff --git a/Source/WebCore/rendering/RenderFlexibleBox.cpp b/Source/WebCore/rendering/RenderFlexibleBox.cpp
index caabbaa..46b9550 100644 (file)
--- a/Source/WebCore/rendering/RenderFlexibleBox.cpp
+++ b/Source/WebCore/rendering/RenderFlexibleBox.cpp
@@ -250,9 +250,7 @@ void RenderFlexibleBox::layoutBlock(bool relayoutChildren, int, BlockLayoutPass)
     IntSize previousSize = size();
 
     setLogicalHeight(0);
-    // We need to call both of these because we grab both crossAxisExtent and mainAxisExtent in layoutFlexItems.
     computeLogicalWidth();
-    computeLogicalHeight();
 
     m_overflow.clear();
 
@@ -264,10 +262,15 @@ void RenderFlexibleBox::layoutBlock(bool relayoutChildren, int, BlockLayoutPass)
             layer()->setHasVerticalScrollbar(true);
     }
 
-    layoutFlexItems(relayoutChildren);
+    WTF::Vector<LineContext> lineContexts;
+    FlexOrderHashSet flexOrderValues;
+    computeMainAxisPreferredSizes(relayoutChildren, flexOrderValues);
+    FlexOrderIterator flexIterator(this, flexOrderValues);
+    layoutFlexItems(relayoutChildren, flexIterator, lineContexts);
 
     LayoutUnit oldClientAfterEdge = clientLogicalBottom();
     computeLogicalHeight();
+    repositionLogicalHeightDependentFlexItems(flexIterator, lineContexts, oldClientAfterEdge);
 
     if (size() != previousSize)
         relayoutChildren = true;
@@ -291,6 +294,24 @@ void RenderFlexibleBox::layoutBlock(bool relayoutChildren, int, BlockLayoutPass)
     setNeedsLayout(false);
 }
 
+void RenderFlexibleBox::repositionLogicalHeightDependentFlexItems(FlexOrderIterator& iterator, WTF::Vector<LineContext>& lineContexts, LayoutUnit& oldClientAfterEdge)
+{
+    // If we have a single line flexbox, the line height is all the available space.
+    // For flex-direction: row, this means we need to use the height, so we do this after calling computeLogicalHeight.
+    if (!isMultiline() && lineContexts.size() == 1)
+        lineContexts[0].crossAxisExtent = crossAxisContentExtent();
+    alignChildren(iterator, lineContexts);
+
+    if (style()->flexWrap() == FlexWrapReverse) {
+        if (isHorizontalFlow())
+            oldClientAfterEdge = clientLogicalBottom();
+        flipForWrapReverse(iterator, lineContexts);
+    }
+
+    // direction:rtl + flex-direction:column means the cross-axis direction is flipped.
+    flipForRightToLeftColumn(iterator);
+}
+
 bool RenderFlexibleBox::hasOrthogonalFlow(RenderBox* child) const
 {
     // FIXME: If the child is a flexbox, then we need to check isHorizontalFlow.
@@ -589,21 +610,16 @@ LayoutUnit RenderFlexibleBox::computeAvailableFreeSpace(LayoutUnit preferredMain
     return heightResult - preferredMainAxisExtent;
 }
 
-void RenderFlexibleBox::layoutFlexItems(bool relayoutChildren)
+void RenderFlexibleBox::layoutFlexItems(bool relayoutChildren, FlexOrderIterator& iterator, WTF::Vector<LineContext>& lineContexts)
 {
-    FlexOrderHashSet flexOrderValues;
-    computeMainAxisPreferredSizes(relayoutChildren, flexOrderValues);
-
     OrderedFlexItemList orderedChildren;
     LayoutUnit preferredMainAxisExtent;
     float totalPositiveFlexibility;
     float totalNegativeFlexibility;
     LayoutUnit minMaxAppliedMainAxisExtent;
-    WTF::Vector<LineContext> lineContexts;
-    FlexOrderIterator flexIterator(this, flexOrderValues);
 
     LayoutUnit crossAxisOffset = flowAwareBorderBefore() + flowAwarePaddingBefore();
-    while (computeNextFlexLine(flexIterator, orderedChildren, preferredMainAxisExtent, totalPositiveFlexibility, totalNegativeFlexibility, minMaxAppliedMainAxisExtent)) {
+    while (computeNextFlexLine(iterator, orderedChildren, preferredMainAxisExtent, totalPositiveFlexibility, totalNegativeFlexibility, minMaxAppliedMainAxisExtent)) {
         LayoutUnit availableFreeSpace = computeAvailableFreeSpace(preferredMainAxisExtent);
         FlexSign flexSign = (minMaxAppliedMainAxisExtent < preferredMainAxisExtent + availableFreeSpace) ? PositiveFlexibility : NegativeFlexibility;
         InflexibleFlexItemSize inflexibleItems;
@@ -615,14 +631,6 @@ void RenderFlexibleBox::layoutFlexItems(bool relayoutChildren)
 
         layoutAndPlaceChildren(crossAxisOffset, orderedChildren, childSizes, availableFreeSpace, lineContexts);
     }
-//
-    alignChildren(flexIterator, lineContexts);
-
-    if (style()->flexWrap() == FlexWrapReverse)
-        flipForWrapReverse(flexIterator, lineContexts);
-
-    // direction:rtl + flex-direction:column means the cross-axis direction is flipped.
-    flipForRightToLeftColumn(flexIterator);
 }
 
 float RenderFlexibleBox::positiveFlexForChild(RenderBox* child) const
@@ -927,9 +935,8 @@ void RenderFlexibleBox::layoutAndPlaceChildren(LayoutUnit& crossAxisOffset, cons
         layoutColumnReverse(children, childSizes, crossAxisOffset, availableFreeSpace);
     }
 
-    LayoutUnit lineCrossAxisExtent = isMultiline() ? maxChildCrossAxisExtent : crossAxisContentExtent();
-    lineContexts.append(LineContext(crossAxisOffset, lineCrossAxisExtent, children.size(), maxAscent));
-    crossAxisOffset += lineCrossAxisExtent;
+    lineContexts.append(LineContext(crossAxisOffset, maxChildCrossAxisExtent, children.size(), maxAscent));
+    crossAxisOffset += maxChildCrossAxisExtent;
 }
 
 void RenderFlexibleBox::layoutColumnReverse(const OrderedFlexItemList& children, const WTF::Vector<LayoutUnit>& childSizes, LayoutUnit crossAxisOffset, LayoutUnit availableFreeSpace)
@@ -1076,9 +1083,6 @@ void RenderFlexibleBox::flipForRightToLeftColumn(FlexOrderIterator& iterator)
 
 void RenderFlexibleBox::flipForWrapReverse(FlexOrderIterator& iterator, const WTF::Vector<LineContext>& lineContexts)
 {
-    if (!isColumnFlow())
-        computeLogicalHeight();
-
     LayoutUnit contentExtent = crossAxisContentExtent();
     RenderBox* child = iterator.first();
     for (size_t lineNumber = 0; lineNumber < lineContexts.size(); ++lineNumber) {

diff --git a/Source/WebCore/rendering/RenderFlexibleBox.h b/Source/WebCore/rendering/RenderFlexibleBox.h
index c65e6dc..ec83fbe 100644 (file)
--- a/Source/WebCore/rendering/RenderFlexibleBox.h
+++ b/Source/WebCore/rendering/RenderFlexibleBox.h
@@ -100,7 +100,8 @@ private:
     LayoutUnit mainAxisScrollbarExtentForChild(RenderBox* child) const;
     LayoutUnit preferredMainAxisContentExtentForChild(RenderBox* child) const;
 
-    void layoutFlexItems(bool relayoutChildren);
+    void layoutFlexItems(bool relayoutChildren, FlexOrderIterator&, WTF::Vector<LineContext>&);
+    void repositionLogicalHeightDependentFlexItems(FlexOrderIterator&, WTF::Vector<LineContext>&, LayoutUnit& oldClientAfterEdge);
 
     float positiveFlexForChild(RenderBox* child) const;
     float negativeFlexForChild(RenderBox* child) const;
@@ -119,7 +120,7 @@ private:
 
     void setLogicalOverrideSize(RenderBox* child, LayoutUnit childPreferredSize);
     void prepareChildForPositionedLayout(RenderBox* child, LayoutUnit mainAxisOffset, LayoutUnit crossAxisOffset);
-    void layoutAndPlaceChildren(LayoutUnit& crossAxisOffset, const OrderedFlexItemList&, const WTF::Vector<LayoutUnit>& childSizes, LayoutUnit availableFreeSpace, WTF::Vector<LineContext>& lineContexts);
+    void layoutAndPlaceChildren(LayoutUnit& crossAxisOffset, const OrderedFlexItemList&, const WTF::Vector<LayoutUnit>& childSizes, LayoutUnit availableFreeSpace, WTF::Vector<LineContext>&);
     void layoutColumnReverse(const OrderedFlexItemList&, const WTF::Vector<LayoutUnit>& childSizes, LayoutUnit crossAxisOffset, LayoutUnit availableFreeSpace);
     void alignChildren(FlexOrderIterator&, const WTF::Vector<LineContext>&);
     void applyStretchAlignmentToChild(RenderBox*, LayoutUnit lineCrossAxisExtent);