flexbox scrollbars don't take flex-direction into account
authortony@chromium.org <tony@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 27 Jan 2012 18:12:05 +0000 (18:12 +0000)
committertony@chromium.org <tony@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 27 Jan 2012 18:12:05 +0000 (18:12 +0000)
https://bugs.webkit.org/show_bug.cgi?id=70772

Reviewed by Darin Adler.

Source/WebCore:

This fixes a bug where we always used the logicalScrollbarHeight.
For column flow, this was incorrect.

Also fix a bug where we didn't include the trailing border+padding+scrollbar when computing the
height of a column flow.

Tests: css3/flexbox/cross-axis-scrollbar-expected.html
       css3/flexbox/cross-axis-scrollbar.html

* rendering/RenderFlexibleBox.cpp:
(WebCore::RenderFlexibleBox::crossAxisScrollbarExtent): Add a direction aware method for getting the scrollbar size.
(WebCore):
(WebCore::RenderFlexibleBox::layoutAndPlaceChildren): Use crossAxisScrollbarExtent.
* rendering/RenderFlexibleBox.h:
(RenderFlexibleBox):

LayoutTests:

* css3/flexbox/cross-axis-scrollbar-expected.html: Added.
* css3/flexbox/cross-axis-scrollbar.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/css3/flexbox/cross-axis-scrollbar-expected.html [new file with mode: 0644]
LayoutTests/css3/flexbox/cross-axis-scrollbar.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderFlexibleBox.cpp
Source/WebCore/rendering/RenderFlexibleBox.h

index 966f563..37861e2 100644 (file)
@@ -1,3 +1,13 @@
+2012-01-27  Tony Chang  <tony@chromium.org>
+
+        flexbox scrollbars don't take flex-direction into account
+        https://bugs.webkit.org/show_bug.cgi?id=70772
+
+        Reviewed by Darin Adler.
+
+        * css3/flexbox/cross-axis-scrollbar-expected.html: Added.
+        * css3/flexbox/cross-axis-scrollbar.html: Added.
+
 2012-01-27  Zan Dobersek  <zandobersek@gmail.com>
 
         REGRESSION (r106036-r106050): 12 tests failing on GTK Linux 64-bit Debug
diff --git a/LayoutTests/css3/flexbox/cross-axis-scrollbar-expected.html b/LayoutTests/css3/flexbox/cross-axis-scrollbar-expected.html
new file mode 100644 (file)
index 0000000..7ad411f
--- /dev/null
@@ -0,0 +1,71 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+body {
+    margin: 0;
+}
+
+.testcase {
+    position: absolute;
+}
+
+.testcase div {
+    background-color: green;
+}
+</style>
+</head>
+<body>
+This test passes if no red is showing.
+
+<div style="position: relative; background-color: transparent;">
+
+<div class="testcase" style="top:0; left: 0">
+    <div style="width: 100px; height: 50px"></div>
+    <div style="width: 100px; overflow-x: scroll"></div>
+    <div style="width: 100px; height: 10px"></div>
+</div>
+
+<div class="testcase" style="height: 50px; -webkit-writing-mode: vertical-lr; top: 0; left: 150px">
+    <div style="width: 100px"></div>
+    <div style="overflow-y: scroll"></div>
+    <div style="width: 10px"></div>
+</div>
+
+<div class="testcase" style="height: 50px; -webkit-writing-mode: vertical-lr; top: 0; left: 300px">
+    <div style="width: 100px"></div>
+    <div style="overflow-y: scroll"></div>
+    <div style="width: 10px"></div>
+</div>
+
+<div class="testcase" style="top: 0; left: 450px">
+    <div style="width: 100px; height: 50px"></div>
+    <div style="width: 100px; overflow-x: scroll"></div>
+    <div style="width: 100px; height: 10px"></div>
+</div>
+
+<div class="testcase" style="top: 100px; left: 0">
+    <div style="width: 100px; height: 50px; "></div>
+    <div style="width: 100px; overflow-x: scroll"></div>
+    <div style="width: 100px; height: 10px"></div>
+</div>
+
+<div class="testcase" style="height: 50px; -webkit-writing-mode: vertical-lr; top: 100px; left: 150px">
+    <div style="width: 90px; overflow-y: scroll"></div>
+    <div style="width: 10px"></div>
+</div>
+
+<div class="testcase" style="top: 100px; left: 300px">
+    <div style="width: 100px; height: 40px; overflow-x: scroll"></div>
+    <div style="width: 100px; height: 10px"></div>
+</div>
+
+<div class="testcase" style="height: 50px; -webkit-writing-mode: vertical-lr; top: 100px; left: 450px">
+    <div style="width: 100px"></div>
+    <div style="overflow-y: scroll"></div>
+    <div style="width: 10px"></div>
+</div>
+
+
+</body>
+</html>
diff --git a/LayoutTests/css3/flexbox/cross-axis-scrollbar.html b/LayoutTests/css3/flexbox/cross-axis-scrollbar.html
new file mode 100644 (file)
index 0000000..1ea1bcb
--- /dev/null
@@ -0,0 +1,141 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+body {
+    margin: 0;
+}
+
+.flexbox {
+    display: -webkit-flexbox;
+    background-color: red;
+}
+
+.vertical-rl {
+    -webkit-writing-mode: vertical-rl;
+}
+
+.vertical-lr {
+    -webkit-writing-mode: vertical-lr;
+}
+
+.column {
+    -webkit-flex-direction: column;
+}
+
+.column > div {
+    background-color: green;
+    width: 100px;
+    height: 10px;
+}
+
+.row > div {
+    background-color: green;
+    width: -webkit-flex(1);
+    height: 50px;
+}
+
+.vertical-lr > .column > div {
+    height: 50px;
+    width: 20px;
+}
+
+.vertical-lr > .row > div {
+    height: -webkit-flex(1);
+    width: 100px;
+}
+
+</style>
+</head>
+<body>
+
+This test passes if no red is showing.
+
+<div style="position: relative;">
+
+<div style="width: 50px; position: absolute; top: 0; left: 0;">
+<div class="flexbox column" style="overflow-x: scroll; overflow-y: hidden; border-bottom: 10px solid green;">
+    <div style="-webkit-flex-item-align: start"></div>
+    <div style="-webkit-flex-item-align: center"></div>
+    <div style="-webkit-flex-item-align: end"></div>
+    <div style="-webkit-flex-item-align: baseline"></div>
+    <div style="-webkit-flex-item-align: stretch"></div>
+</div>
+</div>
+
+<div style="width: 50px; position: absolute; top: 0; left: 150px;">
+<div class="flexbox column" style="overflow-y: scroll; overflow-x: hidden; border-right: 10px solid green;">
+    <div style="-webkit-flex-item-align: start"></div>
+    <div style="-webkit-flex-item-align: center"></div>
+    <div style="-webkit-flex-item-align: end"></div>
+    <div style="-webkit-flex-item-align: baseline"></div>
+    <div style="-webkit-flex-item-align: stretch"></div>
+</div>
+</div>
+
+<div style="height: 20px; position: absolute; top:0; left: 300px" class="vertical-lr">
+<div class="flexbox column" style="overflow-y: scroll; overflow-x: hidden; border-right: 10px solid green;">
+    <div style="-webkit-flex-item-align: start"></div>
+    <div style="-webkit-flex-item-align: center"></div>
+    <div style="-webkit-flex-item-align: end"></div>
+    <div style="-webkit-flex-item-align: baseline"></div>
+    <div style="-webkit-flex-item-align: start"></div>
+</div>
+</div>
+
+<div style="height: 20px; position: absolute; top:0; left: 450px;" class="vertical-lr">
+<div class="flexbox column" style="overflow-x: scroll; overflow-y: hidden; border-bottom: 10px solid green;">
+    <div style="-webkit-flex-item-align: start"></div>
+    <div style="-webkit-flex-item-align: center"></div>
+    <div style="-webkit-flex-item-align: end"></div>
+    <div style="-webkit-flex-item-align: baseline"></div>
+    <div style="-webkit-flex-item-align: start"></div>
+</div>
+</div>
+
+<div style="height: 20px; width: 100px; position: absolute; top: 100px; left: 0">
+<div class="flexbox row" style="overflow-x: scroll; overflow-y: hidden; border-bottom: 10px solid green;">
+    <div style="-webkit-flex-item-align: start"></div>
+    <div style="-webkit-flex-item-align: center"></div>
+    <div style="-webkit-flex-item-align: end"></div>
+    <div style="-webkit-flex-item-align: baseline"></div>
+    <div style="-webkit-flex-item-align: stretch"></div>
+</div>
+</div>
+
+<div style="height: 20px; width: 100px; position: absolute; top: 100px; left: 150px">
+<div class="flexbox row" style="overflow-y: scroll; overflow-x: hidden; border-right: 10px solid green;">
+    <div style="-webkit-flex-item-align: start"></div>
+    <div style="-webkit-flex-item-align: center"></div>
+    <div style="-webkit-flex-item-align: end"></div>
+    <div style="-webkit-flex-item-align: baseline"></div>
+    <div style="-webkit-flex-item-align: stretch"></div>
+</div>
+</div>
+
+<div style="height: 50px; width: 50px; position: absolute; top: 100px; left: 300px" class="vertical-lr">
+<div class="flexbox row" style="overflow-x: scroll; overflow-y: hidden; border-bottom: 10px solid green;">
+    <div style="-webkit-flex-item-align: start"></div>
+    <div style="-webkit-flex-item-align: center"></div>
+    <div style="-webkit-flex-item-align: end"></div>
+    <div style="-webkit-flex-item-align: baseline"></div>
+    <div style="-webkit-flex-item-align: stretch"></div>
+</div>
+</div>
+
+<div style="height: 50px; width: 50px; position: absolute; top: 100px; left: 450px" class="vertical-lr">
+<div class="flexbox row" style="overflow-y: scroll; overflow-x: hidden; border-right: 10px solid green;">
+    <div style="-webkit-flex-item-align: start"></div>
+    <div style="-webkit-flex-item-align: center"></div>
+    <div style="-webkit-flex-item-align: end"></div>
+    <div style="-webkit-flex-item-align: baseline"></div>
+    <div style="-webkit-flex-item-align: stretch"></div>
+</div>
+</div>
+
+</div>
+<!-- FIXME: Add tests for horizontal-bt and vertical-rl.  Setting the logical
+bottom border on these tests cause additional scrollbars to appear. -->
+
+</body>
+</html>
index 7d4a05e..77ef63b 100644 (file)
@@ -1,3 +1,26 @@
+2012-01-27  Tony Chang  <tony@chromium.org>
+
+        flexbox scrollbars don't take flex-direction into account
+        https://bugs.webkit.org/show_bug.cgi?id=70772
+
+        Reviewed by Darin Adler.
+
+        This fixes a bug where we always used the logicalScrollbarHeight.
+        For column flow, this was incorrect.
+
+        Also fix a bug where we didn't include the trailing border+padding+scrollbar when computing the
+        height of a column flow.
+
+        Tests: css3/flexbox/cross-axis-scrollbar-expected.html
+               css3/flexbox/cross-axis-scrollbar.html
+
+        * rendering/RenderFlexibleBox.cpp:
+        (WebCore::RenderFlexibleBox::crossAxisScrollbarExtent): Add a direction aware method for getting the scrollbar size.
+        (WebCore):
+        (WebCore::RenderFlexibleBox::layoutAndPlaceChildren): Use crossAxisScrollbarExtent.
+        * rendering/RenderFlexibleBox.h:
+        (RenderFlexibleBox):
+
 2012-01-27  Kentaro Hara  <haraken@chromium.org>
 
         Unreviewed. Rebasedlined run-bindings-tests results.
index 13a15d6..45543fe 100644 (file)
@@ -416,6 +416,11 @@ LayoutUnit RenderFlexibleBox::crossAxisMarginExtentForChild(RenderBox* child) co
     return isHorizontalFlow() ? child->marginTop() + child->marginBottom() : child->marginLeft() + child->marginRight();
 }
 
+LayoutUnit RenderFlexibleBox::crossAxisScrollbarExtent() const
+{
+    return isHorizontalFlow() ? horizontalScrollbarHeight() : verticalScrollbarWidth();
+}
+
 LayoutPoint RenderFlexibleBox::flowAwareLocationForChild(RenderBox* child) const
 {
     return isHorizontalFlow() ? child->location() : child->location().transposedPoint();
@@ -675,11 +680,10 @@ void RenderFlexibleBox::layoutAndPlaceChildren(FlexOrderIterator& iterator, cons
             maxAscent = std::max(maxAscent, ascent);
             maxDescent = std::max(maxDescent, descent);
 
-            // FIXME: add flowAwareScrollbarLogicalHeight.
             if (crossAxisLength().isAuto())
-                setCrossAxisExtent(std::max(crossAxisExtent(), crossAxisBorderAndPaddingExtent() + crossAxisMarginExtentForChild(child) + maxAscent + maxDescent + scrollbarLogicalHeight()));
+                setCrossAxisExtent(std::max(crossAxisExtent(), crossAxisBorderAndPaddingExtent() + crossAxisMarginExtentForChild(child) + maxAscent + maxDescent + crossAxisScrollbarExtent()));
         } else if (crossAxisLength().isAuto())
-            setCrossAxisExtent(std::max(crossAxisExtent(), crossAxisBorderAndPaddingExtent() + crossAxisMarginExtentForChild(child) + crossAxisExtentForChild(child) + scrollbarLogicalHeight()));
+            setCrossAxisExtent(std::max(crossAxisExtent(), crossAxisBorderAndPaddingExtent() + crossAxisMarginExtentForChild(child) + crossAxisExtentForChild(child) + crossAxisScrollbarExtent()));
 
         mainAxisOffset += flowAwareMarginStartForChild(child);
 
@@ -694,7 +698,7 @@ void RenderFlexibleBox::layoutAndPlaceChildren(FlexOrderIterator& iterator, cons
         mainAxisOffset += packingSpaceBetweenChildren(availableFreeSpace, totalPositiveFlexibility, style()->flexPack(), childSizes.size());
 
         if (isColumnFlow())
-            setLogicalHeight(mainAxisOffset);
+            setLogicalHeight(mainAxisOffset + flowAwareBorderEnd() + flowAwarePaddingEnd() + scrollbarLogicalHeight());
     }
 
     if (style()->flexDirection() == FlowColumnReverse) {
index f73ded7..3ec4c2a 100644 (file)
@@ -78,6 +78,7 @@ private:
     LayoutUnit flowAwareMarginBeforeForChild(RenderBox* child) const;
     LayoutUnit flowAwareMarginAfterForChild(RenderBox* child) const;
     LayoutUnit crossAxisMarginExtentForChild(RenderBox* child) const;
+    LayoutUnit crossAxisScrollbarExtent() const;
     LayoutPoint flowAwareLocationForChild(RenderBox* child) const;
     // FIXME: Supporting layout deltas.
     void setFlowAwareLocationForChild(RenderBox* child, const LayoutPoint&);