REGRESSION: flexbox content-size fails to exclude scrollbar
authortony@chromium.org <tony@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 1 Aug 2012 00:54:19 +0000 (00:54 +0000)
committertony@chromium.org <tony@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 1 Aug 2012 00:54:19 +0000 (00:54 +0000)
https://bugs.webkit.org/show_bug.cgi?id=92667

Reviewed by Ojan Vafai.

Source/WebCore:

In r123909, we switched to computing the height using computeContentLogicalHeightUsing().
Unfortunately, this includes the scrollbar when we want the content height. Add a helper
method for computing the value needed by flexbox.

Test: css3/flexbox/content-height-with-scrollbars.html

* rendering/RenderFlexibleBox.cpp:
(WebCore::RenderFlexibleBox::mainAxisContentExtent): Use computeLogicalClientHeight
(WebCore::RenderFlexibleBox::computeLogicalClientHeight): Add new method for taking scrollbar into consideration.
(WebCore::RenderFlexibleBox::computeAvailableFreeSpace): Use computeLogicalClientHeight
(WebCore::RenderFlexibleBox::lineBreakLength): Use computeLogicalClientHeight
* rendering/RenderFlexibleBox.h:

LayoutTests:

Add column flow test cases with scrollbars forced on.

* css3/flexbox/content-height-with-scrollbars-expected.html: Added.
* css3/flexbox/content-height-with-scrollbars.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/css3/flexbox/content-height-with-scrollbars-expected.html [new file with mode: 0644]
LayoutTests/css3/flexbox/content-height-with-scrollbars.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderFlexibleBox.cpp
Source/WebCore/rendering/RenderFlexibleBox.h

index d45af47..099d135 100644 (file)
@@ -1,5 +1,17 @@
 2012-07-31  Tony Chang  <tony@chromium.org>
 
+        REGRESSION: flexbox content-size fails to exclude scrollbar
+        https://bugs.webkit.org/show_bug.cgi?id=92667
+
+        Reviewed by Ojan Vafai.
+
+        Add column flow test cases with scrollbars forced on.
+
+        * css3/flexbox/content-height-with-scrollbars-expected.html: Added.
+        * css3/flexbox/content-height-with-scrollbars.html: Added.
+
+2012-07-31  Tony Chang  <tony@chromium.org>
+
         -webkit-order should take an integer, not a number
         https://bugs.webkit.org/show_bug.cgi?id=92688
 
diff --git a/LayoutTests/css3/flexbox/content-height-with-scrollbars-expected.html b/LayoutTests/css3/flexbox/content-height-with-scrollbars-expected.html
new file mode 100644 (file)
index 0000000..780028d
--- /dev/null
@@ -0,0 +1,46 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+.outer {
+    height: 100px;
+    overflow:scroll;
+}
+.outer > div, td:nth-child(1) {
+    background-color: lightgreen;
+}
+td:nth-child(2) {
+    background-color: lightblue;
+}
+table, td {
+    padding: 0;
+    border-spacing: 0;
+}
+</style>
+</head>
+<body>
+<p>This tests that when setting the height of a flex item to a percentage
+height, we use the content height with scrollbars. The content should not be
+scrollable in any of the test cases below.</p>
+
+<div class="outer">
+  <div style="height: 100%"></div>
+</div>
+
+<div class="outer">
+  <div style="height: 100%; box-sizing: border-box; border: 5px solid green"></div>
+</div>
+
+<div class="outer">
+  <div style="display:inline-block; height: 50px; width: 50%;"></div><div
+      style="display:inline-block; height: 50px; width: 50%; background-color: lightblue"></div>
+</div>
+<div class="outer" style="padding: 10px; height: auto">
+  <table style="margin: 0; padding: 0; width: 100%">
+  <tr>
+    <td style="width: 50%; height: 50px"></td>
+    <td style="width: 50%; height: 50px"></td>
+  </tr>
+  </table>
+</body>
+</html>
diff --git a/LayoutTests/css3/flexbox/content-height-with-scrollbars.html b/LayoutTests/css3/flexbox/content-height-with-scrollbars.html
new file mode 100644 (file)
index 0000000..ef1584c
--- /dev/null
@@ -0,0 +1,42 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+.outer {
+    display: -webkit-flex;
+    -webkit-flex-flow: column;
+    height: 100px;
+    overflow: scroll;
+}
+.outer > :nth-child(1) {
+    background-color: lightgreen;
+}
+.outer > :nth-child(2) {
+    background-color: lightblue;
+}
+</style>
+</head>
+<body>
+<p>This tests that when setting the height of a flex item to a percentage
+height, we use the content height with scrollbars. The content should not be
+scrollable in any of the test cases below.</p>
+
+<div class="outer">
+  <div style="height: 100%"></div>
+</div>
+
+<div class="outer">
+  <div style="height: 100%; box-sizing: border-box; border: 5px solid green"></div>
+</div>
+
+<div class="outer" style="-webkit-flex-wrap: wrap">
+  <div style="height: 50px; width: 50%;"></div>
+  <div style="height: 50px; width: 50%; background-color: lightblue"></div>
+</div>
+
+<div class="outer" style="-webkit-flex-wrap: wrap; height: auto; max-height: 100px; padding: 10px;">
+  <div style="height: 50px; width: 50%;"></div>
+  <div style="height: 50px; width: 50%; background-color: lightblue"></div>
+</div>
+</body>
+</html>
index f3916e1..bd08b20 100644 (file)
@@ -1,3 +1,23 @@
+2012-07-31  Tony Chang  <tony@chromium.org>
+
+        REGRESSION: flexbox content-size fails to exclude scrollbar
+        https://bugs.webkit.org/show_bug.cgi?id=92667
+
+        Reviewed by Ojan Vafai.
+
+        In r123909, we switched to computing the height using computeContentLogicalHeightUsing().
+        Unfortunately, this includes the scrollbar when we want the content height. Add a helper
+        method for computing the value needed by flexbox.
+
+        Test: css3/flexbox/content-height-with-scrollbars.html
+
+        * rendering/RenderFlexibleBox.cpp:
+        (WebCore::RenderFlexibleBox::mainAxisContentExtent): Use computeLogicalClientHeight
+        (WebCore::RenderFlexibleBox::computeLogicalClientHeight): Add new method for taking scrollbar into consideration.
+        (WebCore::RenderFlexibleBox::computeAvailableFreeSpace): Use computeLogicalClientHeight
+        (WebCore::RenderFlexibleBox::lineBreakLength): Use computeLogicalClientHeight
+        * rendering/RenderFlexibleBox.h:
+
 2012-07-31  Kwang Yul Seo  <skyul@company100.net>
 
         Remove unused method HTMLElementStack::bottom()
index c50b411..8955fde 100644 (file)
@@ -401,10 +401,18 @@ LayoutUnit RenderFlexibleBox::crossAxisContentExtent() const
 LayoutUnit RenderFlexibleBox::mainAxisContentExtent()
 {
     if (isColumnFlow())
-        return std::max(LayoutUnit(0), computeContentLogicalHeightUsing(MainOrPreferredSize, style()->logicalHeight()));
+        return std::max(LayoutUnit(0), computeLogicalClientHeight(MainOrPreferredSize, style()->logicalHeight()));
     return contentLogicalWidth();
 }
 
+LayoutUnit RenderFlexibleBox::computeLogicalClientHeight(SizeType heightType, const Length& height)
+{
+    LayoutUnit heightIncludingScrollbar = computeContentLogicalHeightUsing(heightType, height);
+    if (heightIncludingScrollbar == -1)
+        return -1;
+    return std::max(LayoutUnit(0), heightIncludingScrollbar - scrollbarLogicalHeight());
+}
+
 WritingMode RenderFlexibleBox::transformedWritingMode() const
 {
     WritingMode mode = style()->writingMode();
@@ -612,11 +620,11 @@ LayoutUnit RenderFlexibleBox::computeAvailableFreeSpace(LayoutUnit preferredMain
     else if (hasOverrideHeight())
         contentExtent = overrideLogicalContentHeight();
     else {
-        LayoutUnit heightResult = computeContentLogicalHeightUsing(MainOrPreferredSize, style()->logicalHeight());
+        LayoutUnit heightResult = computeLogicalClientHeight(MainOrPreferredSize, style()->logicalHeight());
         if (heightResult == -1)
             heightResult = preferredMainAxisExtent;
-        LayoutUnit minHeight = computeContentLogicalHeightUsing(MinSize, style()->logicalMinHeight()); // Leave as -1 if unset.
-        LayoutUnit maxHeight = style()->logicalMaxHeight().isUndefined() ? heightResult : computeContentLogicalHeightUsing(MaxSize, style()->logicalMaxHeight());
+        LayoutUnit minHeight = computeLogicalClientHeight(MinSize, style()->logicalMinHeight()); // Leave as -1 if unset.
+        LayoutUnit maxHeight = style()->logicalMaxHeight().isUndefined() ? heightResult : computeLogicalClientHeight(MaxSize, style()->logicalMaxHeight());
         if (maxHeight == -1)
             maxHeight = heightResult;
         heightResult = std::min(maxHeight, heightResult);
@@ -789,10 +797,10 @@ LayoutUnit RenderFlexibleBox::lineBreakLength()
     if (!isColumnFlow())
         return mainAxisContentExtent();
 
-    LayoutUnit height = computeContentLogicalHeightUsing(MainOrPreferredSize, style()->logicalHeight());
+    LayoutUnit height = computeLogicalClientHeight(MainOrPreferredSize, style()->logicalHeight());
     if (height == -1)
         height = MAX_LAYOUT_UNIT;
-    LayoutUnit maxHeight = computeContentLogicalHeightUsing(MaxSize, style()->logicalMaxHeight());
+    LayoutUnit maxHeight = computeLogicalClientHeight(MaxSize, style()->logicalMaxHeight());
     if (maxHeight != -1)
         height = std::min(height, maxHeight);
     return height;
index 3a9e52a..108b8d6 100644 (file)
@@ -85,6 +85,7 @@ private:
     LayoutUnit mainAxisExtent() const;
     LayoutUnit crossAxisContentExtent() const;
     LayoutUnit mainAxisContentExtent();
+    LayoutUnit computeLogicalClientHeight(SizeType, const Length& height);
     WritingMode transformedWritingMode() const;
     LayoutUnit flowAwareBorderStart() const;
     LayoutUnit flowAwareBorderEnd() const;