nesting horizontal flexboxes is broken
authorojan@chromium.org <ojan@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 13 Feb 2012 23:37:55 +0000 (23:37 +0000)
committerojan@chromium.org <ojan@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 13 Feb 2012 23:37:55 +0000 (23:37 +0000)
https://bugs.webkit.org/show_bug.cgi?id=76867

Reviewed by David Hyatt.

Source/WebCore:

This is copied from RenderDeprecatedFlexibleBox and updated
for RenderFlexibleBox and to handle vertical writing mode.

Tests: css3/flexbox/preferred-widths-orthogonal.html
       css3/flexbox/preferred-widths.html

* rendering/RenderBlock.cpp:
(WebCore::RenderBlock::computePreferredLogicalWidths):
* rendering/RenderFlexibleBox.cpp:
(WebCore::marginWidthForChild):
(WebCore):
(WebCore::RenderFlexibleBox::computePreferredLogicalWidths):
* rendering/RenderFlexibleBox.h:
(RenderFlexibleBox):

LayoutTests:

* css3/flexbox/floated-flexbox-expected.txt:
* css3/flexbox/line-wrapping.html:
The old results here were wrong because they wrapped the contents of each flex item.

* css3/flexbox/preferred-widths-expected.txt: Added.
* css3/flexbox/preferred-widths-orthogonal-expected.txt: Added.
* css3/flexbox/preferred-widths-orthogonal.html: Added.
* css3/flexbox/preferred-widths.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/css3/flexbox/floated-flexbox-expected.txt
LayoutTests/css3/flexbox/line-wrapping.html
LayoutTests/css3/flexbox/preferred-widths-expected.txt [new file with mode: 0644]
LayoutTests/css3/flexbox/preferred-widths-orthogonal-expected.txt [new file with mode: 0644]
LayoutTests/css3/flexbox/preferred-widths-orthogonal.html [new file with mode: 0644]
LayoutTests/css3/flexbox/preferred-widths.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderBlock.cpp
Source/WebCore/rendering/RenderFlexibleBox.cpp
Source/WebCore/rendering/RenderFlexibleBox.h

index 49673f5..bfc04e0 100644 (file)
@@ -1,3 +1,19 @@
+2012-02-09  Ojan Vafai  <ojan@chromium.org>
+
+        nesting horizontal flexboxes is broken
+        https://bugs.webkit.org/show_bug.cgi?id=76867
+
+        Reviewed by David Hyatt.
+
+        * css3/flexbox/floated-flexbox-expected.txt:
+        * css3/flexbox/line-wrapping.html:
+        The old results here were wrong because they wrapped the contents of each flex item.
+
+        * css3/flexbox/preferred-widths-expected.txt: Added.
+        * css3/flexbox/preferred-widths-orthogonal-expected.txt: Added.
+        * css3/flexbox/preferred-widths-orthogonal.html: Added.
+        * css3/flexbox/preferred-widths.html: Added.
+
 2012-02-13  Mihnea Ovidenie  <mihnea@adobe.com>
 
         Crash in RenderFlowThread::setRegionBoxesRegionStyle
index dd05afa..7ef22e9 100644 (file)
@@ -1,7 +1 @@
-FAIL:
-Expected 130 for width, but got 110. 
-
-<div data-expected-width="130" data-expected-height="30" class="flexbox">
-    <div style="background-color:pink; width: 20px; height: 20px;"></div>
-    <div style="background-color:red; width: 100px; height: 20px;"></div>
-</div>
+PASS
index edfff45..4dda40f 100644 (file)
@@ -187,12 +187,12 @@ if (window.layoutTestController)
     </div>
 </div>
 
-<div class="flexbox auto row vertical-lr" data-expected-height=300>
-    <div data-expected-height=150 data-expected-width=100>
-        <div data-offset-x=4></div><div data-offset-x=24></div><div data-offset-x=44 data-offset-y=0></div>
+<div class="flexbox auto row vertical-lr" data-expected-height=500>
+    <div data-expected-height=250 data-expected-width=100>
+        <div data-offset-x=4></div><div data-offset-x=4></div><div data-offset-x=24 data-offset-y=0></div>
     </div>
-    <div data-expected-height=150 data-expected-width=60 style="-webkit-flex-item-align: start;">
-        <div data-offset-x=4></div><div data-offset-x=24></div><div data-offset-x=44 data-offset-y=150></div>
+    <div data-expected-height=250 data-expected-width=40 style="-webkit-flex-item-align: start;">
+        <div data-offset-x=4></div><div data-offset-x=4></div><div data-offset-x=24 data-offset-y=250></div>
     </div>
 </div>
 
@@ -233,12 +233,12 @@ if (window.layoutTestController)
     </div>
 </div>
 
-<div class="flexbox auto row vertical-rl" data-expected-height=300>
-    <div data-expected-height=150 data-expected-width=100>
-        <div data-offset-x=80></div><div data-offset-x=60></div><div data-offset-x=40 data-offset-y=0></div>
+<div class="flexbox auto row vertical-rl" data-expected-height=500>
+    <div data-expected-height=250 data-expected-width=100>
+        <div data-offset-x=80></div><div data-offset-x=80></div><div data-offset-x=60 data-offset-y=0></div>
     </div>
-    <div data-expected-height=150 data-expected-width=60 style="-webkit-flex-item-align: start;">
-        <div data-offset-x=80></div><div data-offset-x=60></div><div data-offset-x=40 data-offset-y=150></div>
+    <div data-expected-height=250 data-expected-width=40 style="-webkit-flex-item-align: start;">
+        <div data-offset-x=80></div><div data-offset-x=80></div><div data-offset-x=60 data-offset-y=250></div>
     </div>
 </div>
 
diff --git a/LayoutTests/css3/flexbox/preferred-widths-expected.txt b/LayoutTests/css3/flexbox/preferred-widths-expected.txt
new file mode 100644 (file)
index 0000000..5514010
--- /dev/null
@@ -0,0 +1,65 @@
+horizontal-tb overflowY row
+PASS
+horizontal-tb overflowX row
+PASS
+horizontal-tb overflowY column
+PASS
+horizontal-tb overflowX column
+PASS
+horizontal-tb overflowY row-reverse
+PASS
+horizontal-tb overflowX row-reverse
+PASS
+horizontal-tb overflowY column-reverse
+PASS
+horizontal-tb overflowX column-reverse
+PASS
+horizontal-bt overflowY row
+PASS
+horizontal-bt overflowX row
+PASS
+horizontal-bt overflowY column
+PASS
+horizontal-bt overflowX column
+PASS
+horizontal-bt overflowY row-reverse
+PASS
+horizontal-bt overflowX row-reverse
+PASS
+horizontal-bt overflowY column-reverse
+PASS
+horizontal-bt overflowX column-reverse
+PASS
+vertical-lr overflowY row
+PASS
+vertical-lr overflowX row
+PASS
+vertical-lr overflowY column
+PASS
+vertical-lr overflowX column
+PASS
+vertical-lr overflowY row-reverse
+PASS
+vertical-lr overflowX row-reverse
+PASS
+vertical-lr overflowY column-reverse
+PASS
+vertical-lr overflowX column-reverse
+PASS
+vertical-rl overflowY row
+PASS
+vertical-rl overflowX row
+PASS
+vertical-rl overflowY column
+PASS
+vertical-rl overflowX column
+PASS
+vertical-rl overflowY row-reverse
+PASS
+vertical-rl overflowX row-reverse
+PASS
+vertical-rl overflowY column-reverse
+PASS
+vertical-rl overflowX column-reverse
+PASS
+
diff --git a/LayoutTests/css3/flexbox/preferred-widths-orthogonal-expected.txt b/LayoutTests/css3/flexbox/preferred-widths-orthogonal-expected.txt
new file mode 100644 (file)
index 0000000..f1974f5
--- /dev/null
@@ -0,0 +1,11 @@
+PASS
+
+PASS
+
+PASS
+
+PASS
+
+PASS
+
+
diff --git a/LayoutTests/css3/flexbox/preferred-widths-orthogonal.html b/LayoutTests/css3/flexbox/preferred-widths-orthogonal.html
new file mode 100644 (file)
index 0000000..3aaf0f3
--- /dev/null
@@ -0,0 +1,117 @@
+<!DOCTYPE html>
+<html>
+<style>
+.flexbox {
+    display: -webkit-flexbox;
+    background-color: #aaa;
+    float: left;
+}
+.flexbox > div {
+    margin: 2px 13px 8px 17px;
+}
+.flexbox > div:not(.nested) {
+    width: 10px;
+    height: 15px;
+}
+.flexbox > .nested > div:not(.nested) {
+    width: 20px;
+    height: 30px;
+}
+.flexbox > .nested > .nested > div {
+    width: 30px;
+    height: 40px;
+}
+
+.flexbox > :nth-child(1) {
+    background-color: blue;
+}
+.flexbox > :nth-child(2) {
+    background-color: green;
+}
+.flexbox > div > :nth-child(1) {
+    background-color: pink;
+}
+.flexbox > div > :nth-child(2) {
+    background-color: purple;
+}
+.flexbox > div > div > :nth-child(1) {
+    background-color: red;
+}
+.flexbox > div > div > :nth-child(2) {
+    background-color: yellow;
+}
+
+.nested {
+    display: -webkit-flexbox;
+    background-color: #ddd;
+}
+.horizontal-tb {
+    -webkit-writing-mode: horizontal-tb;
+}
+.vertical-lr {
+    -webkit-writing-mode: vertical-lr;
+}
+.column {
+    -webkit-flex-flow: column;
+}
+.clear {
+    clear: both;
+}
+</style>
+<script>
+if (window.layoutTestController)
+    layoutTestController.dumpAsText();
+</script>
+<script src="resources/flexbox.js"></script>
+<body onload="checkFlexBoxen()">
+
+<div class="flexbox" data-expected-height=70  data-expect-width=90>
+    <div class="column nested" data-expected-height=60  data-expect-width=20>
+        <div></div>
+        <div></div>
+    </div>
+    <div></div>
+</div>
+<br class=clear>
+
+<div class="flexbox vertical-lr" data-expected-height=115  data-expect-width=80>
+    <div class="horizontal-tb nested" data-expected-height=80  data-expect-width=50>
+        <div class="vertical-lr nested" data-expected-height=80  data-expect-width=30>
+            <div></div>
+            <div></div>
+        </div>
+        <div></div>
+    </div>
+    <div></div>
+</div>
+<br class=clear>
+
+<div class="flexbox vertical-lr" data-expected-height=65  data-expect-width=70>
+    <div class="column nested" data-expected-height=30  data-expect-width=40>
+        <div></div>
+        <div></div>
+    </div>
+    <div></div>
+</div>
+<br class=clear>
+
+<div class="flexbox vertical-lr" data-expected-height=65  data-expect-width=70>
+    <div class="nested horizontal-tb" data-expected-height=30  data-expect-width=40>
+        <div></div>
+        <div></div>
+    </div>
+    <div></div>
+</div>
+<br class=clear>
+
+<div class="flexbox" data-expected-height=70  data-expect-width=90>
+    <div class="nested vertical-lr" data-expected-height=60  data-expect-width=20>
+        <div></div>
+        <div></div>
+    </div>
+    <div></div>
+</div>
+<br class=clear>
+
+</body>
+</html>
\ No newline at end of file
diff --git a/LayoutTests/css3/flexbox/preferred-widths.html b/LayoutTests/css3/flexbox/preferred-widths.html
new file mode 100644 (file)
index 0000000..1c25caf
--- /dev/null
@@ -0,0 +1,102 @@
+<!DOCTYPE html>
+<html>
+<style>
+.flexbox {
+    display: -webkit-flexbox;
+    background-color: grey;
+    float: left;
+}
+.flexbox > div {
+    padding: 2px 13px 8px 17px;
+    margin: 2px 13px 8px 17px;
+}
+.title {
+    margin-top: 1em;
+}
+.overflowX {
+    overflow-x: scroll;
+    overflow-y: hidden;
+}
+.overflowY {
+    overflow-x: hidden;
+    overflow-y: scroll;
+}
+.auto {
+    overflow: auto;
+}
+.horizontal-tb {
+    -webkit-writing-mode: horizontal-tb;
+}
+.horizontal-bt {
+    -webkit-writing-mode: horizontal-bt;
+}
+.vertical-rl {
+    -webkit-writing-mode: vertical-rl;
+}
+.vertical-lr {
+    -webkit-writing-mode: vertical-lr;
+}
+.row {
+    -webkit-flex-flow: row;
+}
+.row-reverse {
+    -webkit-flex-flow: row-reverse;
+}
+.column {
+    -webkit-flex-flow: column;
+}
+.column-reverse {
+    -webkit-flex-flow: column-reverse;
+}
+</style>
+<script>
+if (window.layoutTestController)
+    layoutTestController.dumpAsText();
+</script>
+<script src="resources/flexbox.js"></script>
+<body onload="checkFlexBoxen()">
+
+<script>
+var writingModes = ['horizontal-tb', 'horizontal-bt', 'vertical-lr', 'vertical-rl'];
+var flexFlows = ['row', 'column', 'row-reverse', 'column-reverse'];
+var scrollDirections = ['overflowY', 'overflowX'];
+
+var dummyNode = document.createElement('div');
+dummyNode.style.overflow = 'scroll';
+document.body.appendChild(dummyNode);
+var scrollbarWidth = dummyNode.offsetWidth - dummyNode.clientWidth;
+
+writingModes.forEach(function(writingMode) {
+    flexFlows.forEach(function(flexFlow) {
+        scrollDirections.forEach(function(scrollDirection) {
+            var flexboxClassName = writingMode + ' ' + scrollDirection + ' ' + flexFlow;
+
+            var title = document.createElement('div');
+            title.className = 'title';
+            title.innerHTML = flexboxClassName;
+            document.body.appendChild(title);
+
+            var isColumn = flexFlow.indexOf('column') != -1;
+            var isHorizontal = (writingMode.indexOf('horizontal') != -1) ? !isColumn : isColumn;
+
+            var expectedWidth = isHorizontal ? 220 : 140;
+            var expectedHeight = isHorizontal ? 60 : 110;
+            if (scrollDirection == 'overflowY')
+                expectedWidth += scrollbarWidth;
+            if (scrollDirection == 'overflowX')
+                expectedHeight += scrollbarWidth;
+
+
+            var container = document.createElement('div');
+            container.innerHTML = '<div class="flexbox ' + flexboxClassName + '" data-expected-width="' + expectedWidth + '" data-expected-height="' + expectedHeight + '">\n' +
+                '<div style="background-color:pink; width: 20px; height: 30px;"></div>\n' +
+                '<div style="background-color:red; width: 80px; height: 40px;"></div>\n' +
+            '</div><div style="clear:both;"></div>';
+
+            document.body.appendChild(container);
+        })
+    })
+})
+</script>
+</body>
+</html>
index 952c19f..cf05b35 100644 (file)
@@ -1,3 +1,25 @@
+2012-02-09  Ojan Vafai  <ojan@chromium.org>
+
+        nesting horizontal flexboxes is broken
+        https://bugs.webkit.org/show_bug.cgi?id=76867
+
+        Reviewed by David Hyatt.
+
+        This is copied from RenderDeprecatedFlexibleBox and updated
+        for RenderFlexibleBox and to handle vertical writing mode.
+
+        Tests: css3/flexbox/preferred-widths-orthogonal.html
+               css3/flexbox/preferred-widths.html
+
+        * rendering/RenderBlock.cpp:
+        (WebCore::RenderBlock::computePreferredLogicalWidths):
+        * rendering/RenderFlexibleBox.cpp:
+        (WebCore::marginWidthForChild):
+        (WebCore):
+        (WebCore::RenderFlexibleBox::computePreferredLogicalWidths):
+        * rendering/RenderFlexibleBox.h:
+        (RenderFlexibleBox):
+
 2012-02-13  Mihnea Ovidenie  <mihnea@adobe.com>
 
         Crash in RenderFlowThread::setRegionBoxesRegionStyle
index 05c9f3b..4e74ab4 100755 (executable)
@@ -5023,6 +5023,8 @@ void RenderBlock::computePreferredLogicalWidths()
         }
 
         int scrollbarWidth = 0;
+        // FIXME: This should only be done for horizontal writing mode.
+        // For vertical writing mode, this should check overflowX and use the horizontalScrollbarHeight.
         if (hasOverflowClip() && styleToUse->overflowY() == OSCROLL) {
             layer()->setHasVerticalScrollbar(true);
             scrollbarWidth = verticalScrollbarWidth();
index 36fefb3..13a950d 100644 (file)
@@ -114,6 +114,84 @@ const char* RenderFlexibleBox::renderName() const
     return "RenderFlexibleBox";
 }
 
+static LayoutUnit marginLogicalWidthForChild(RenderBox* child, RenderStyle* parentStyle)
+{
+    // A margin has three types: fixed, percentage, and auto (variable).
+    // Auto and percentage margins become 0 when computing min/max width.
+    // Fixed margins can be added in as is.
+    Length marginLeft = child->style()->marginStartUsing(parentStyle);
+    Length marginRight = child->style()->marginEndUsing(parentStyle);
+    LayoutUnit margin = 0;
+    if (marginLeft.isFixed())
+        margin += marginLeft.value();
+    if (marginRight.isFixed())
+        margin += marginRight.value();
+    return margin;
+}
+
+void RenderFlexibleBox::computePreferredLogicalWidths()
+{
+    ASSERT(preferredLogicalWidthsDirty());
+
+    RenderStyle* styleToUse = style();
+    if (styleToUse->logicalWidth().isFixed() && styleToUse->logicalWidth().value() > 0)
+        m_minPreferredLogicalWidth = m_maxPreferredLogicalWidth = computeContentBoxLogicalWidth(styleToUse->logicalWidth().value());
+    else {
+        m_minPreferredLogicalWidth = m_maxPreferredLogicalWidth = 0;
+
+        for (RenderBox* child = firstChildBox(); child; child = child->nextSiblingBox()) {
+            if (child->isPositioned())
+                continue;
+
+            LayoutUnit margin = marginLogicalWidthForChild(child, style());
+            bool hasOrthogonalWritingMode = child->isHorizontalWritingMode() != isHorizontalWritingMode();
+            LayoutUnit minPreferredLogicalWidth = hasOrthogonalWritingMode ? child->logicalHeight() : child->minPreferredLogicalWidth();
+            LayoutUnit maxPreferredLogicalWidth = hasOrthogonalWritingMode ? child->logicalHeight() : child->maxPreferredLogicalWidth();
+            minPreferredLogicalWidth += margin;
+            maxPreferredLogicalWidth += margin;
+            if (!isColumnFlow()) {
+                m_minPreferredLogicalWidth += minPreferredLogicalWidth;
+                m_maxPreferredLogicalWidth += maxPreferredLogicalWidth;
+            } else {
+                m_minPreferredLogicalWidth = std::max(minPreferredLogicalWidth, m_minPreferredLogicalWidth);
+                m_maxPreferredLogicalWidth = std::max(maxPreferredLogicalWidth, m_maxPreferredLogicalWidth);
+            }
+        }
+
+        m_maxPreferredLogicalWidth = std::max(m_minPreferredLogicalWidth, m_maxPreferredLogicalWidth);
+    }
+
+    LayoutUnit scrollbarWidth = 0;
+    if (hasOverflowClip()) {
+        if (isHorizontalWritingMode() && styleToUse->overflowY() == OSCROLL) {
+            layer()->setHasVerticalScrollbar(true);
+            scrollbarWidth = verticalScrollbarWidth();
+        } else if (!isHorizontalWritingMode() && styleToUse->overflowX() == OSCROLL) {
+            layer()->setHasHorizontalScrollbar(true);
+            scrollbarWidth = horizontalScrollbarHeight();
+        }
+    }
+
+    m_maxPreferredLogicalWidth += scrollbarWidth;
+    m_minPreferredLogicalWidth += scrollbarWidth;
+
+    if (styleToUse->logicalMinWidth().isFixed() && styleToUse->logicalMinWidth().value() > 0) {
+        m_maxPreferredLogicalWidth = std::max(m_maxPreferredLogicalWidth, computeContentBoxLogicalWidth(styleToUse->logicalMinWidth().value()));
+        m_minPreferredLogicalWidth = std::max(m_minPreferredLogicalWidth, computeContentBoxLogicalWidth(styleToUse->logicalMinWidth().value()));
+    }
+
+    if (styleToUse->logicalMaxWidth().isFixed()) {
+        m_maxPreferredLogicalWidth = std::min(m_maxPreferredLogicalWidth, computeContentBoxLogicalWidth(styleToUse->logicalMaxWidth().value()));
+        m_minPreferredLogicalWidth = std::min(m_minPreferredLogicalWidth, computeContentBoxLogicalWidth(styleToUse->logicalMaxWidth().value()));
+    }
+
+    LayoutUnit borderAndPadding = borderAndPaddingLogicalWidth();
+    m_minPreferredLogicalWidth += borderAndPadding;
+    m_maxPreferredLogicalWidth += borderAndPadding;
+
+    setPreferredLogicalWidthsDirty(false);
+}
+
 void RenderFlexibleBox::layoutBlock(bool relayoutChildren, int, BlockLayoutPass)
 {
     ASSERT(needsLayout());
index 0ca255c..0ee715f 100644 (file)
@@ -43,7 +43,7 @@ public:
     virtual const char* renderName() const;
 
     virtual bool isFlexibleBox() const { return true; }
-
+    virtual void computePreferredLogicalWidths();
     virtual void layoutBlock(bool relayoutChildren, int pageLogicalHeight = 0, BlockLayoutPass = NormalLayoutPass);
 
     bool isHorizontalFlow() const;