Fix cross-direction stretch for replaced elements in column flexbox
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 23 Aug 2012 23:41:09 +0000 (23:41 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 23 Aug 2012 23:41:09 +0000 (23:41 +0000)
https://bugs.webkit.org/show_bug.cgi?id=94604

Patch by Shezan Baig <shezbaig.wk@gmail.com> on 2012-08-23
Reviewed by Ojan Vafai.

Source/WebCore:

Moved the logic that constrains logical width by MinSize and MaxSize to
a new helper function called constrainLogicalWidthInRegionByMinMax.
This helper function is used from both computeLogicalWidthInRegion and
RenderFlexibleBox::applyStretchAlignmentToChild.

RenderFlexibleBox no longer checks for isMultiline when stretching
elements in a column flexbox. Instead, we now constrain the available
width by the child's min-width and max-width, and set the override
width only if that constrained width is different from the child's
current logicalWidth.

No new tests. The existing css3/flexbox/flexitem.html test was extended
to exercise the new code.

* rendering/RenderBox.cpp:
(WebCore::RenderBox::constrainLogicalWidthInRegionByMinMax): New helper
method to constrain logical width by min-width and max-width.
(WebCore):
(WebCore::RenderBox::computeLogicalWidthInRegion): Changed to use the
new constrainLogicalWidthInRegionByMinMax helper method.
* rendering/RenderBox.h:
(RenderBox):
* rendering/RenderFlexibleBox.cpp:
(WebCore::RenderFlexibleBox::applyStretchAlignmentToChild): Changed to
use constrainLogicalWidthInRegionByMinMax to determine the override
width for the child.

LayoutTests:

Fixed failing test cases. Add tests for min/max width/height.

* css3/flexbox/flexitem-expected.txt:
* css3/flexbox/flexitem.html:

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

LayoutTests/ChangeLog
LayoutTests/css3/flexbox/flexitem-expected.txt
LayoutTests/css3/flexbox/flexitem.html
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderBox.cpp
Source/WebCore/rendering/RenderBox.h
Source/WebCore/rendering/RenderFlexibleBox.cpp

index c83f453..86f885c 100644 (file)
@@ -1,3 +1,15 @@
+2012-08-23  Shezan Baig  <shezbaig.wk@gmail.com>
+
+        Fix cross-direction stretch for replaced elements in column flexbox
+        https://bugs.webkit.org/show_bug.cgi?id=94604
+
+        Reviewed by Ojan Vafai.
+
+        Fixed failing test cases. Add tests for min/max width/height.
+
+        * css3/flexbox/flexitem-expected.txt:
+        * css3/flexbox/flexitem.html:
+
 2012-08-23  Kenneth Russell  <kbr@google.com>
 
         Unreviewed Chromium gardening. Generalized a few Snow Leopard-only
index 625ffd0..2f51355 100644 (file)
@@ -25,38 +25,17 @@ PASS
 
 PASS
 
-FAIL:
-Expected 100px for width, but got 300. 
-
-<div class="flexbox column" style="width:100px">
-  <!-- FIXME: The iframe should shrink in the cross direction: https://webkit.org/b/94604 -->
-  <iframe data-expected-display="block" data-expected-width="100px" src="data:text/html,&lt;body bgcolor=#fff&gt;iframe&lt;/body&gt;"></iframe>
-  <iframe seamless="" data-expected-display="block" data-expected-width="100px" src="data:text/html,&lt;body bgcolor=#fff&gt;iframe&lt;/body&gt;"></iframe>
-</div>
+PASS
 button
 
 object
 
-FAIL:
-Expected 600 for width, but got 34. 
-Expected 600 for width, but got 60. 
-Expected 600 for width, but got 300. 
-Expected 600 for width, but got 56. 
-Expected 600 for width, but got 155. 
-
-<div class="flexbox column" style="height:210px">
-  <!-- FIXME: The button should stretch in the cross direction. -->
-  <button data-expected-display="block" data-expected-width="600" data-expected-height="30">button</button>
-  <!-- FIXME: The canvas should stretch in the cross direction: https://webkit.org/b/94604 -->
-  <canvas data-expected-display="block" data-expected-width="600" data-expected-height="30" style="height:30px">canvas</canvas>
-  <!-- FIXME: The iframe should stretch in the cross direction: https://webkit.org/b/94604 -->
-  <iframe data-expected-display="block" data-expected-width="600" data-expected-height="30" style="height:30px" src="data:text/html,&lt;body bgcolor=#fff&gt;iframe&lt;/body&gt;"></iframe>
-  <iframe seamless="" data-expected-display="block" data-expected-width="600" data-expected-height="30" style="height:30px" src="data:text/html,&lt;body bgcolor=#fff&gt;iframe&lt;/body&gt;"></iframe>
-  <object data-expected-display="block" data-expected-width="600" data-expected-height="30">object</object>
-  <!-- FIXME: The select should stretch in the cross direction. -->
-  <select data-expected-display="block" data-expected-width="600" data-expected-height="30">
-    <option>select</option>
-  </select>
-  <!-- FIXME: The textarea should stretch in the cross direction. -->
-  <textarea data-expected-display="block" data-expected-width="600" data-expected-height="30">textarea</textarea>
-</div>
+PASS
+
+PASS
+
+PASS
+
+PASS
+
+PASS
index 9888abe..78acbb6 100644 (file)
@@ -125,4 +125,54 @@ if (window.testRunner)
   <textarea data-expected-display="block" data-expected-width="600" data-expected-height="30">textarea</textarea>
 </div>
 
+<!-- tests that min-height expands the height of flex items beyond the height of the flexbox -->
+<div class="flexbox" style="height:50px">
+    <img data-expected-height="60" style="min-height:60px" src="../images/resources/blue-100.png">
+    <img data-expected-height="60" style="min-height:60px" src="../images/resources/green-10.png">
+    <img data-expected-height="75" style="min-height:150%" src="../images/resources/blue-100.png">
+    <img data-expected-height="75" style="min-height:150%" src="../images/resources/green-10.png">
+    <!-- TODO: test min-content, max-content, fill-available, fit-content when these are implemented for height -->
+</div>
+
+<!-- tests that max-height shrinks the height of flex items less than the height of the flexbox -->
+<div class="flexbox" style="height:50px">
+    <img data-expected-height="40" style="max-height:40px" src="../images/resources/blue-100.png">
+    <img data-expected-height="40" style="max-height:40px" src="../images/resources/green-10.png">
+    <img data-expected-height="25" style="max-height:50%" src="../images/resources/blue-100.png">
+    <img data-expected-height="25" style="max-height:50%" src="../images/resources/green-10.png">
+    <!-- TODO: test min-content, max-content, fill-available, fit-content when these are implemented for height -->
+</div>
+
+<!-- tests that min-width expands the width of flex items beyond the width of the flexbox -->
+<div class="flexbox column" style="width:50px">
+    <img data-expected-width="60" style="min-width:60px" src="../images/resources/blue-100.png">
+    <img data-expected-width="60" style="min-width:60px" src="../images/resources/green-10.png">
+    <img data-expected-width="75" style="min-width:150%" src="../images/resources/blue-100.png">
+    <img data-expected-width="75" style="min-width:150%" src="../images/resources/green-10.png">
+    <img data-expected-width="100" style="min-width:-webkit-min-content" src="../images/resources/blue-100.png">
+    <img data-expected-width="50" style="min-width:-webkit-min-content" src="../images/resources/green-10.png">
+    <img data-expected-width="100" style="min-width:-webkit-max-content" src="../images/resources/blue-100.png">
+    <img data-expected-width="50" style="min-width:-webkit-max-content" src="../images/resources/green-10.png">
+    <img data-expected-width="50" style="min-width:-webkit-fill-available" src="../images/resources/blue-100.png">
+    <img data-expected-width="50" style="min-width:-webkit-fill-available" src="../images/resources/green-10.png">
+    <img data-expected-width="100" style="min-width:-webkit-fit-content" src="../images/resources/blue-100.png">
+    <img data-expected-width="50" style="min-width:-webkit-fit-content" src="../images/resources/green-10.png">
+</div>
+
+<!-- tests that max-width shrinks the width of flex items less than the width of the flexbox -->
+<div class="flexbox column" style="width:50px">
+    <img data-expected-width="40" style="max-width:40px" src="../images/resources/blue-100.png">
+    <img data-expected-width="40" style="max-width:40px" src="../images/resources/green-10.png">
+    <img data-expected-width="25" style="max-width:50%" src="../images/resources/blue-100.png">
+    <img data-expected-width="25" style="max-width:50%" src="../images/resources/green-10.png">
+    <img data-expected-width="50" style="max-width:-webkit-min-content" src="../images/resources/blue-100.png">
+    <img data-expected-width="10" style="max-width:-webkit-min-content" src="../images/resources/green-10.png">
+    <img data-expected-width="50" style="max-width:-webkit-max-content" src="../images/resources/blue-100.png">
+    <img data-expected-width="10" style="max-width:-webkit-max-content" src="../images/resources/green-10.png">
+    <img data-expected-width="50" style="max-width:-webkit-fill-available" src="../images/resources/blue-100.png">
+    <img data-expected-width="50" style="max-width:-webkit-fill-available" src="../images/resources/green-10.png">
+    <img data-expected-width="50" style="max-width:-webkit-fit-content" src="../images/resources/blue-100.png">
+    <img data-expected-width="10" style="max-width:-webkit-fit-content" src="../images/resources/green-10.png">
+</div>
+
 </html>
index 3663b04..cb22547 100644 (file)
@@ -1,3 +1,37 @@
+2012-08-23  Shezan Baig  <shezbaig.wk@gmail.com>
+
+        Fix cross-direction stretch for replaced elements in column flexbox
+        https://bugs.webkit.org/show_bug.cgi?id=94604
+
+        Reviewed by Ojan Vafai.
+
+        Moved the logic that constrains logical width by MinSize and MaxSize to
+        a new helper function called constrainLogicalWidthInRegionByMinMax.
+        This helper function is used from both computeLogicalWidthInRegion and
+        RenderFlexibleBox::applyStretchAlignmentToChild.
+
+        RenderFlexibleBox no longer checks for isMultiline when stretching
+        elements in a column flexbox. Instead, we now constrain the available
+        width by the child's min-width and max-width, and set the override
+        width only if that constrained width is different from the child's
+        current logicalWidth.
+
+        No new tests. The existing css3/flexbox/flexitem.html test was extended
+        to exercise the new code.
+
+        * rendering/RenderBox.cpp:
+        (WebCore::RenderBox::constrainLogicalWidthInRegionByMinMax): New helper
+        method to constrain logical width by min-width and max-width.
+        (WebCore):
+        (WebCore::RenderBox::computeLogicalWidthInRegion): Changed to use the
+        new constrainLogicalWidthInRegionByMinMax helper method.
+        * rendering/RenderBox.h:
+        (RenderBox):
+        * rendering/RenderFlexibleBox.cpp:
+        (WebCore::RenderFlexibleBox::applyStretchAlignmentToChild): Changed to
+        use constrainLogicalWidthInRegionByMinMax to determine the override
+        width for the child.
+
 2012-08-23  Adam Barth  <abarth@webkit.org>
 
         [V8] ScriptState is using stone knifes and bear skins
index 5ec6df9..6e9c046 100644 (file)
@@ -433,16 +433,22 @@ void RenderBox::updateLayerTransform()
         layer()->updateTransform();
 }
 
+LayoutUnit RenderBox::constrainLogicalWidthInRegionByMinMax(LayoutUnit logicalWidth, LayoutUnit availableWidth, RenderBlock* cb, RenderRegion* region, LayoutUnit offsetFromLogicalTopOfFirstPage)
+{
+    RenderStyle* styleToUse = style();
+    if (!styleToUse->logicalMaxWidth().isUndefined())
+        logicalWidth = min(logicalWidth, computeLogicalWidthInRegionUsing(MaxSize, availableWidth, cb, region, offsetFromLogicalTopOfFirstPage));
+    return max(logicalWidth, computeLogicalWidthInRegionUsing(MinSize, availableWidth, cb, region, offsetFromLogicalTopOfFirstPage));
+}
+
 LayoutUnit RenderBox::constrainLogicalHeightByMinMax(LayoutUnit logicalHeight)
 {
     RenderStyle* styleToUse = style();
     if (!styleToUse->logicalMaxHeight().isUndefined()) {
-        // Constrain by MaxSize.
         LayoutUnit maxH = computeLogicalHeightUsing(MaxSize, styleToUse->logicalMaxHeight());
         if (maxH != -1)
             logicalHeight = min(logicalHeight, maxH);
     }
-    // Constrain by MinSize.
     return max(logicalHeight, computeLogicalHeightUsing(MinSize, styleToUse->logicalMinHeight()));
 }
 
@@ -1688,20 +1694,8 @@ void RenderBox::computeLogicalWidthInRegion(RenderRegion* region, LayoutUnit off
     if (treatAsReplaced)
         setLogicalWidth(logicalWidthLength.value() + borderAndPaddingLogicalWidth());
     else {
-        // Calculate LogicalWidth
-        setLogicalWidth(computeLogicalWidthInRegionUsing(MainOrPreferredSize, containerWidthInInlineDirection, cb, region, offsetFromLogicalTopOfFirstPage));
-
-        // Calculate MaxLogicalWidth
-        if (!styleToUse->logicalMaxWidth().isUndefined()) {
-            LayoutUnit maxLogicalWidth = computeLogicalWidthInRegionUsing(MaxSize, containerWidthInInlineDirection, cb, region, offsetFromLogicalTopOfFirstPage);
-            if (logicalWidth() > maxLogicalWidth)
-                setLogicalWidth(maxLogicalWidth);
-        }
-
-        // Calculate MinLogicalWidth
-        LayoutUnit minLogicalWidth = computeLogicalWidthInRegionUsing(MinSize, containerWidthInInlineDirection, cb, region, offsetFromLogicalTopOfFirstPage);
-        if (logicalWidth() < minLogicalWidth)
-            setLogicalWidth(minLogicalWidth);
+        LayoutUnit preferredWidth = computeLogicalWidthInRegionUsing(MainOrPreferredSize, containerWidthInInlineDirection, cb, region, offsetFromLogicalTopOfFirstPage);
+        setLogicalWidth(constrainLogicalWidthInRegionByMinMax(preferredWidth, containerWidthInInlineDirection, cb, region, offsetFromLogicalTopOfFirstPage));
     }
 
     // Fieldsets are currently the only objects that stretch to their minimum width.
index 4affbc0..f143c5c 100644 (file)
@@ -75,6 +75,7 @@ public:
     LayoutUnit logicalWidth() const { return style()->isHorizontalWritingMode() ? width() : height(); }
     LayoutUnit logicalHeight() const { return style()->isHorizontalWritingMode() ? height() : width(); }
 
+    LayoutUnit constrainLogicalWidthInRegionByMinMax(LayoutUnit, LayoutUnit, RenderBlock*, RenderRegion* = 0, LayoutUnit offsetFromLogicalTopOfFirstPage = ZERO_LAYOUT_UNIT);
     LayoutUnit constrainLogicalHeightByMinMax(LayoutUnit);
 
     int pixelSnappedLogicalHeight() const { return style()->isHorizontalWritingMode() ? pixelSnappedHeight() : pixelSnappedWidth(); }
index f40201f..9f87cf8 100644 (file)
@@ -1246,14 +1246,18 @@ void RenderFlexibleBox::applyStretchAlignmentToChild(RenderBox* child, LayoutUni
                 child->layoutIfNeeded();
             }
         }
-    } else if (isColumnFlow() && child->style()->logicalWidth().isAuto() && isMultiline()) {
-        // FIXME: Handle min-width and max-width.
-        // FIXME: We only need to relayout here if the width changes.
-        // FIXME: The isMultiline check above may not be necessary if the width has not changed. See https://webkit.org/b/94237
-        LayoutUnit childWidth = lineCrossAxisExtent - crossAxisMarginExtentForChild(child);
-        child->setOverrideLogicalContentWidth(std::max(ZERO_LAYOUT_UNIT, childWidth));
-        child->setChildNeedsLayout(true, MarkOnlyThis);
-        child->layoutIfNeeded();
+    } else if (isColumnFlow() && child->style()->logicalWidth().isAuto()) {
+        // FIXME: If the child doesn't have orthogonal flow, then it already has an override width set, so use it.
+        if (hasOrthogonalFlow(child)) {
+            LayoutUnit childWidth = std::max(ZERO_LAYOUT_UNIT, lineCrossAxisExtent - crossAxisMarginExtentForChild(child));
+            childWidth = child->constrainLogicalWidthInRegionByMinMax(childWidth, childWidth, this);
+
+            if (childWidth != child->logicalWidth()) {
+                child->setOverrideLogicalContentWidth(childWidth);
+                child->setChildNeedsLayout(true, MarkOnlyThis);
+                child->layoutIfNeeded();
+            }
+        }
     }
 }