Fix cross-direction stretch for replaced elements in row flexbox

https://bugs.webkit.org/show_bug.cgi?id=94237 Patch by Shezan Baig <sbaig1@bloomberg.net> on 2012-08-21 Reviewed by Ojan Vafai. Source/WebCore: When stretching, don't take into account the instrinsic size of child replaced elements. Only the fixed size, min size, and max size of the child should be taken into account. The logic that computed this was moved from RenderBox::computeLogicalHeight to a new helper method called logicalHeightConstrainedByMinMax. This helper method is now used from RenderFlexibleBox::applyStretchAlignmentToChild, instead of using RenderBox::computeLogicalHeight. A similar change will need to be made for column-flowing flexboxes. This will be addressed in https://webkit.org/b/94604. No new tests. The existing css3/flexbox/flexitem.html test was extended to cover this case. * rendering/RenderBox.cpp: (WebCore::RenderBox::logicalHeightConstrainedByMinMax): New helper method that is used by RenderBox::computeLogicalHeight and also by RenderFlexibleBox::applyStretchAlignmentToChild. (WebCore): (WebCore::RenderBox::computeLogicalHeight): Updated to use the new logicalHeightConstrainedByMinMax helper method. * rendering/RenderBox.h: (RenderBox): * rendering/RenderFlexibleBox.cpp: (WebCore::RenderFlexibleBox::applyStretchAlignmentToChild): Use logicalHeightConstrainedByMinMax instead of computeLogicalHeight. LayoutTests: Fix test cases for images stretching in the cross direction. Also, added test cases for stretching/shrinking iframes, seamless iframes, and also tests for vertically flowing flexboxes. * css3/flexbox/flexitem-expected.txt: * css3/flexbox/flexitem.html: Canonical link: https://commits.webkit.org/112489@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@126257 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · Aug 22, 2012 · 8de74ae54d84069d4176926c9997f58bcaa5ea2b · 8de74ae
1 parent 25d8522
commit 8de74ae54d84069d4176926c9997f58bcaa5ea2b
Showing 7 changed files with 168 additions and 52 deletions.
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,17 @@
+2012-08-21  Shezan Baig  <sbaig1@bloomberg.net>
+
+        Fix cross-direction stretch for replaced elements in row flexbox
+        https://bugs.webkit.org/show_bug.cgi?id=94237
+
+        Reviewed by Ojan Vafai.
+
+        Fix test cases for images stretching in the cross direction. Also,
+        added test cases for stretching/shrinking iframes, seamless iframes,
+        and also tests for vertically flowing flexboxes.
+
+        * css3/flexbox/flexitem-expected.txt:
+        * css3/flexbox/flexitem.html:
+
 2012-08-21  Kenneth Russell  <kbr@google.com>
 
         Unreviewed Chromium gardening. Rebaseline tests after http://trac.webkit.org/changeset/126225 .

diff --git a/LayoutTests/css3/flexbox/flexitem-expected.txt b/LayoutTests/css3/flexbox/flexitem-expected.txt
@@ -6,25 +6,9 @@ PASS
 
 PASS
 
-FAIL:
-Expected 100 for height, but got 10. 
-
-<div class="flexbox">
-  <img data-expected-display="block" data-expected-width="345" style="-webkit-flex: 1 0 auto;" src="../images/resources/blue-100.png">
-  <!-- FIXME: The image should stretch in the cross direction. -->
-  <img data-expected-display="block" data-expected-width="255" data-expected-height="100" style="-webkit-flex: 1 0 auto;" src="../images/resources/green-10.png">
-</div>
-
-FAIL:
-Expected 100 for height, but got 20. 
-Expected 100 for height, but got 20. 
+PASS
 
-<div class="flexbox">
-  <img data-expected-display="block" data-expected-width="200" style="-webkit-flex: 1 0 auto;" src="../images/resources/blue-100.png">
-  <!-- FIXME: The missing image placeholders should stretch in the cross direction. -->
-  <img data-expected-display="block" data-expected-width="200" data-expected-height="100" style="-webkit-flex: 2 0 0;" src="doesnotexist.png">
-  <img data-expected-display="block" data-expected-width="200" data-expected-height="100" style="-webkit-flex: 2 0 0;" src="doesnotexist.png" alt="placeholder text">
-</div>
+PASS
 PASS
 button
 PASS
@@ -36,3 +20,43 @@ Expected 600 for width, but got 0.
   <!-- FIXME: This table should flex. -->
   <div data-expected-display="table" data-expected-width="600" style="display: inline-table"></div>
 </div>
+
+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>
+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>
diff --git a/LayoutTests/css3/flexbox/flexitem.html b/LayoutTests/css3/flexbox/flexitem.html
@@ -16,22 +16,25 @@
     font-size: 12px;
     min-width: 0;
 }
+.column {
+    -webkit-flex-direction: column;
+}
 </style>
 <script>
 if (window.testRunner)
     testRunner.dumpAsText();
 </script>
 <script src="../../resources/check-layout.js"></script>
 <body onload="checkLayout('.flexbox')">
-<div class="flexbox">
-  <button data-expected-display="block" data-expected-width="100">button</button>
-  <canvas data-expected-display="block" data-expected-width="100">canvas</canvas>
-  <iframe data-expected-display="block" data-expected-width="100" src="data:text/html,<body bgcolor=#fff>iframe</body>"></iframe>
-  <object data-expected-display="block" data-expected-width="100">object</object>
-  <select data-expected-display="block" data-expected-width="100">
+<div class="flexbox" style="height:200px">
+  <button data-expected-display="block" data-expected-width="100" data-expected-height="200">button</button>
+  <canvas data-expected-display="block" data-expected-width="100" data-expected-height="200">canvas</canvas>
+  <iframe data-expected-display="block" data-expected-width="100" data-expected-height="200" src="data:text/html,<body bgcolor=#fff>iframe</body>"></iframe>
+  <object data-expected-display="block" data-expected-width="100" data-expected-height="200">object</object>
+  <select data-expected-display="block" data-expected-width="100" data-expected-height="200">
     <option>select</option>
   </select>
-  <textarea data-expected-display="block" data-expected-width="100">textarea</textarea>
+  <textarea data-expected-display="block" data-expected-width="100" data-expected-height="200">textarea</textarea>
 </div>
 
 
@@ -48,13 +51,11 @@
 
 <div class="flexbox">
   <img data-expected-display="block" data-expected-width="345" style="-webkit-flex: 1 0 auto;" src="../images/resources/blue-100.png">
-  <!-- FIXME: The image should stretch in the cross direction. -->
   <img data-expected-display="block" data-expected-width="255" data-expected-height="100" style="-webkit-flex: 1 0 auto;" src="../images/resources/green-10.png">
 </div>
 
 <div class="flexbox">
   <img data-expected-display="block" data-expected-width="200" style="-webkit-flex: 1 0 auto;" src="../images/resources/blue-100.png">
-  <!-- FIXME: The missing image placeholders should stretch in the cross direction. -->
   <img data-expected-display="block" data-expected-width="200" data-expected-height="100" style="-webkit-flex: 2 0 0;" src="doesnotexist.png">
   <img data-expected-display="block" data-expected-width="200" data-expected-height="100" style="-webkit-flex: 2 0 0;" src="doesnotexist.png" alt="placeholder text">
 </div>
@@ -91,4 +92,37 @@
   <div data-expected-display="table" data-expected-width="600" style="display: inline-table"></div>
 </div>
 
+<div class="flexbox" style="height:200px">
+  <iframe data-expected-display="block" data-expected-height="200" src="data:text/html,<body bgcolor=#fff>iframe</body>"></iframe>
+  <iframe seamless data-expected-display="block" data-expected-height="200" src="data:text/html,<body bgcolor=#fff>iframe</body>"></iframe>
+</div>
+
+<div class="flexbox" style="height:100px">
+  <iframe data-expected-display="block" data-expected-height="100" src="data:text/html,<body bgcolor=#fff>iframe</body>"></iframe>
+  <iframe seamless data-expected-display="block" data-expected-height="100" src="data:text/html,<body bgcolor=#fff>iframe</body>"></iframe>
+</div>
+
+<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,<body bgcolor=#fff>iframe</body>"></iframe>
+  <iframe seamless data-expected-display="block" data-expected-width="100px" src="data:text/html,<body bgcolor=#fff>iframe</body>"></iframe>
+</div>
+
+<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,<body bgcolor=#fff>iframe</body>"></iframe>
+  <iframe seamless data-expected-display="block" data-expected-width="600" data-expected-height="30" style="height:30px" src="data:text/html,<body bgcolor=#fff>iframe</body>"></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>
+
 </html>
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,37 @@
+2012-08-21  Shezan Baig  <sbaig1@bloomberg.net>
+
+        Fix cross-direction stretch for replaced elements in row flexbox
+        https://bugs.webkit.org/show_bug.cgi?id=94237
+
+        Reviewed by Ojan Vafai.
+
+        When stretching, don't take into account the instrinsic size of child
+        replaced elements. Only the fixed size, min size, and max size of the
+        child should be taken into account. The logic that computed this was
+        moved from RenderBox::computeLogicalHeight to a new helper method
+        called logicalHeightConstrainedByMinMax.  This helper method is now
+        used from RenderFlexibleBox::applyStretchAlignmentToChild, instead of
+        using RenderBox::computeLogicalHeight.
+
+        A similar change will need to be made for column-flowing flexboxes.
+        This will be addressed in https://webkit.org/b/94604.
+
+        No new tests.  The existing css3/flexbox/flexitem.html test was
+        extended to cover this case.
+
+        * rendering/RenderBox.cpp:
+        (WebCore::RenderBox::logicalHeightConstrainedByMinMax): New helper
+        method that is used by RenderBox::computeLogicalHeight and also by
+        RenderFlexibleBox::applyStretchAlignmentToChild.
+        (WebCore):
+        (WebCore::RenderBox::computeLogicalHeight): Updated to use the new
+        logicalHeightConstrainedByMinMax helper method.
+        * rendering/RenderBox.h:
+        (RenderBox):
+        * rendering/RenderFlexibleBox.cpp:
+        (WebCore::RenderFlexibleBox::applyStretchAlignmentToChild): Use
+        logicalHeightConstrainedByMinMax instead of computeLogicalHeight.
+
 2012-08-21  Hayato Ito  <hayato@chromium.org>
 
         Make an event object clonable to support an event propagation across seamless iframes.

diff --git a/Source/WebCore/rendering/RenderBox.cpp b/Source/WebCore/rendering/RenderBox.cpp
@@ -433,6 +433,21 @@ void RenderBox::updateLayerTransform()
         layer()->updateTransform();
 }
 
+LayoutUnit RenderBox::logicalHeightConstrainedByMinMax(LayoutUnit availableHeight)
+{
+    RenderStyle* styleToUse = style();
+    LayoutUnit result = computeLogicalHeightUsing(MainOrPreferredSize, styleToUse->logicalHeight());
+    if (result == -1)
+        result = availableHeight;
+    LayoutUnit minH = computeLogicalHeightUsing(MinSize, styleToUse->logicalMinHeight()); // Leave as -1 if unset.
+    LayoutUnit maxH = styleToUse->logicalMaxHeight().isUndefined() ? result : computeLogicalHeightUsing(MaxSize, styleToUse->logicalMaxHeight());
+    if (maxH == -1)
+        maxH = result;
+    result = min(maxH, result);
+    result = max(minH, result);
+    return result;
+}
+
 IntRect RenderBox::absoluteContentBox() const
 {
     // This is wrong with transforms and flipped writing modes.
@@ -1979,13 +1994,12 @@ void RenderBox::computeLogicalHeight()
         // grab our cached flexible height.
         // FIXME: Account for block-flow in flexible boxes.
         // https://bugs.webkit.org/show_bug.cgi?id=46418
-        RenderStyle* styleToUse = style();
         if (hasOverrideHeight() && parent()->isFlexibleBoxIncludingDeprecated())
             h = Length(overrideLogicalContentHeight(), Fixed);
         else if (treatAsReplaced)
             h = Length(computeReplacedLogicalHeight(), Fixed);
         else {
-            h = styleToUse->logicalHeight();
+            h = style()->logicalHeight();
             checkMinMaxHeight = true;
         }
 
@@ -1999,17 +2013,9 @@ void RenderBox::computeLogicalHeight()
         }
 
         LayoutUnit heightResult;
-        if (checkMinMaxHeight) {
-            heightResult = computeLogicalHeightUsing(MainOrPreferredSize, styleToUse->logicalHeight());
-            if (heightResult == -1)
-                heightResult = logicalHeight();
-            LayoutUnit minH = computeLogicalHeightUsing(MinSize, styleToUse->logicalMinHeight()); // Leave as -1 if unset.
-            LayoutUnit maxH = styleToUse->logicalMaxHeight().isUndefined() ? heightResult : computeLogicalHeightUsing(MaxSize, styleToUse->logicalMaxHeight());
-            if (maxH == -1)
-                maxH = heightResult;
-            heightResult = min(maxH, heightResult);
-            heightResult = max(minH, heightResult);
-        } else {
+        if (checkMinMaxHeight)
+            heightResult = logicalHeightConstrainedByMinMax(logicalHeight());
+        else {
             // The only times we don't check min/max height are when a fixed length has
             // been given as an override.  Just use that.  The value has already been adjusted
             // for box-sizing.

diff --git a/Source/WebCore/rendering/RenderBox.h b/Source/WebCore/rendering/RenderBox.h
@@ -75,6 +75,8 @@ class RenderBox : public RenderBoxModelObject {
     LayoutUnit logicalWidth() const { return style()->isHorizontalWritingMode() ? width() : height(); }
     LayoutUnit logicalHeight() const { return style()->isHorizontalWritingMode() ? height() : width(); }
 
+    LayoutUnit logicalHeightConstrainedByMinMax(LayoutUnit);
+
     int pixelSnappedLogicalHeight() const { return style()->isHorizontalWritingMode() ? pixelSnappedHeight() : pixelSnappedWidth(); }
     int pixelSnappedLogicalWidth() const { return style()->isHorizontalWritingMode() ? pixelSnappedWidth() : pixelSnappedHeight(); }
 

diff --git a/Source/WebCore/rendering/RenderFlexibleBox.cpp b/Source/WebCore/rendering/RenderFlexibleBox.cpp
@@ -1233,21 +1233,23 @@ void RenderFlexibleBox::alignChildren(OrderIterator& iterator, const WTF::Vector
 void RenderFlexibleBox::applyStretchAlignmentToChild(RenderBox* child, LayoutUnit lineCrossAxisExtent)
 {
     if (!isColumnFlow() && child->style()->logicalHeight().isAuto()) {
-        LayoutUnit logicalHeightBefore = child->logicalHeight();
-        LayoutUnit stretchedLogicalHeight = child->logicalHeight() + availableAlignmentSpaceForChild(lineCrossAxisExtent, child);
-
-        child->setLogicalHeight(stretchedLogicalHeight);
-        child->computeLogicalHeight();
-
-        // FIXME: Can avoid laying out here in some cases. See https://webkit.org/b/87905.
-        if (child->logicalHeight() != logicalHeightBefore) {
-            child->setOverrideLogicalContentHeight(child->logicalHeight() - child->borderAndPaddingLogicalHeight());
-            child->setLogicalHeight(0);
-            child->setChildNeedsLayout(true, MarkOnlyThis);
-            child->layoutIfNeeded();
+        // FIXME: If the child has orthogonal flow, then it already has an override height set. How do we stretch?
+        if (!hasOrthogonalFlow(child)) {
+            LayoutUnit stretchedLogicalHeight = child->logicalHeight() + availableAlignmentSpaceForChild(lineCrossAxisExtent, child);
+            LayoutUnit desiredLogicalHeight = child->logicalHeightConstrainedByMinMax(stretchedLogicalHeight);
+
+            // FIXME: Can avoid laying out here in some cases. See https://webkit.org/b/87905.
+            if (desiredLogicalHeight != child->logicalHeight()) {
+                child->setOverrideLogicalContentHeight(desiredLogicalHeight - child->borderAndPaddingLogicalHeight());
+                child->setLogicalHeight(0);
+                child->setChildNeedsLayout(true, MarkOnlyThis);
+                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);