Title: [126257] trunk
Revision
126257
Author
[email protected]
Date
2012-08-21 20:35:38 -0700 (Tue, 21 Aug 2012)

Log Message

Fix cross-direction stretch for replaced elements in row flexbox
https://bugs.webkit.org/show_bug.cgi?id=94237

Patch by Shezan Baig <[email protected]> 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:

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (126256 => 126257)


--- trunk/LayoutTests/ChangeLog	2012-08-22 03:11:50 UTC (rev 126256)
+++ trunk/LayoutTests/ChangeLog	2012-08-22 03:35:38 UTC (rev 126257)
@@ -1,3 +1,17 @@
+2012-08-21  Shezan Baig  <[email protected]>
+
+        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  <[email protected]>
 
         Unreviewed Chromium gardening. Rebaseline tests after http://trac.webkit.org/changeset/126225 .

Modified: trunk/LayoutTests/css3/flexbox/flexitem-expected.txt (126256 => 126257)


--- trunk/LayoutTests/css3/flexbox/flexitem-expected.txt	2012-08-22 03:11:50 UTC (rev 126256)
+++ trunk/LayoutTests/css3/flexbox/flexitem-expected.txt	2012-08-22 03:35:38 UTC (rev 126257)
@@ -6,26 +6,10 @@
 
 PASS
 
-FAIL:
-Expected 100 for height, but got 10. 
+PASS
 
-<div class="flexbox">
-  <img data-expected-display="block" data-expected-width="345" style="-webkit-flex: 1 0 auto;" src=""
-  <!-- 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=""
-</div>
-
-FAIL:
-Expected 100 for height, but got 20. 
-Expected 100 for height, but got 20. 
-
-<div class="flexbox">
-  <img data-expected-display="block" data-expected-width="200" style="-webkit-flex: 1 0 auto;" src=""
-  <!-- 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=""
-  <img data-expected-display="block" data-expected-width="200" data-expected-height="100" style="-webkit-flex: 2 0 0;" src="" alt="placeholder text">
-</div>
 PASS
+PASS
 button
 PASS
 PASS
@@ -36,3 +20,43 @@
   <!-- 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="" bgcolor=#fff&gt;iframe&lt;/body&gt;"></iframe>
+  <iframe seamless="" data-expected-display="block" data-expected-width="100px" src="" 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="" 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="" 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>

Modified: trunk/LayoutTests/css3/flexbox/flexitem.html (126256 => 126257)


--- trunk/LayoutTests/css3/flexbox/flexitem.html	2012-08-22 03:11:50 UTC (rev 126256)
+++ trunk/LayoutTests/css3/flexbox/flexitem.html	2012-08-22 03:35:38 UTC (rev 126257)
@@ -16,6 +16,9 @@
     font-size: 12px;
     min-width: 0;
 }
+.column {
+    -webkit-flex-direction: column;
+}
 </style>
 <script>
 if (window.testRunner)
@@ -23,15 +26,15 @@
 </script>
 <script src=""
 <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="" 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="" 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=""
-  <!-- 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=""
 </div>
 
 <div class="flexbox">
   <img data-expected-display="block" data-expected-width="200" style="-webkit-flex: 1 0 auto;" src=""
-  <!-- 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=""
   <img data-expected-display="block" data-expected-width="200" data-expected-height="100" style="-webkit-flex: 2 0 0;" src="" 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="" bgcolor=#fff>iframe</body>"></iframe>
+  <iframe seamless data-expected-display="block" data-expected-height="200" src="" bgcolor=#fff>iframe</body>"></iframe>
+</div>
+
+<div class="flexbox" style="height:100px">
+  <iframe data-expected-display="block" data-expected-height="100" src="" bgcolor=#fff>iframe</body>"></iframe>
+  <iframe seamless data-expected-display="block" data-expected-height="100" src="" 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="" bgcolor=#fff>iframe</body>"></iframe>
+  <iframe seamless data-expected-display="block" data-expected-width="100px" src="" 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="" bgcolor=#fff>iframe</body>"></iframe>
+  <iframe seamless data-expected-display="block" data-expected-width="600" data-expected-height="30" style="height:30px" src="" 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>

Modified: trunk/Source/WebCore/ChangeLog (126256 => 126257)


--- trunk/Source/WebCore/ChangeLog	2012-08-22 03:11:50 UTC (rev 126256)
+++ trunk/Source/WebCore/ChangeLog	2012-08-22 03:35:38 UTC (rev 126257)
@@ -1,3 +1,37 @@
+2012-08-21  Shezan Baig  <[email protected]>
+
+        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  <[email protected]>
 
         Make an event object clonable to support an event propagation across seamless iframes.

Modified: trunk/Source/WebCore/rendering/RenderBox.cpp (126256 => 126257)


--- trunk/Source/WebCore/rendering/RenderBox.cpp	2012-08-22 03:11:50 UTC (rev 126256)
+++ trunk/Source/WebCore/rendering/RenderBox.cpp	2012-08-22 03:35:38 UTC (rev 126257)
@@ -433,6 +433,21 @@
         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 @@
         // 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 @@
         }
 
         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.

Modified: trunk/Source/WebCore/rendering/RenderBox.h (126256 => 126257)


--- trunk/Source/WebCore/rendering/RenderBox.h	2012-08-22 03:11:50 UTC (rev 126256)
+++ trunk/Source/WebCore/rendering/RenderBox.h	2012-08-22 03:35:38 UTC (rev 126257)
@@ -75,6 +75,8 @@
     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(); }
 

Modified: trunk/Source/WebCore/rendering/RenderFlexibleBox.cpp (126256 => 126257)


--- trunk/Source/WebCore/rendering/RenderFlexibleBox.cpp	2012-08-22 03:11:50 UTC (rev 126256)
+++ trunk/Source/WebCore/rendering/RenderFlexibleBox.cpp	2012-08-22 03:35:38 UTC (rev 126257)
@@ -1233,21 +1233,23 @@
 void RenderFlexibleBox::applyStretchAlignmentToChild(RenderBox* child, LayoutUnit lineCrossAxisExtent)
 {
     if (!isColumnFlow() && child->style()->logicalHeight().isAuto()) {
-        LayoutUnit logicalHeightBefore = child->logicalHeight();
-        LayoutUnit stretchedLogicalHeight = child->logicalHeight() + availableAlignmentSpaceForChild(lineCrossAxisExtent, child);
+        // 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);
 
-        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: 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);
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to