flexbox does wrong baseline item alignment in columns
authortony@chromium.org <tony@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 2 Oct 2012 01:51:22 +0000 (01:51 +0000)
committertony@chromium.org <tony@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 2 Oct 2012 01:51:22 +0000 (01:51 +0000)
https://bugs.webkit.org/show_bug.cgi?id=97948

Reviewed by Ojan Vafai.

Source/WebCore:

For columns, baseline alignment should just be flex-start.  We were previously
moving the logical left edge by the ascent.

Test: css3/flexbox/align-baseline.html

* rendering/RenderFlexibleBox.cpp:
(WebCore::RenderFlexibleBox::alignmentForChild): Map baseline to flex-start for orthogonal children.
(WebCore::RenderFlexibleBox::alignChildren): Add FIXME for bug in baseline alignment.
* rendering/RenderFlexibleBox.h:

LayoutTests:

Update test cases with new baseline alignment for column.

* css3/flexbox/align-baseline-expected.html: Added.
* css3/flexbox/align-baseline.html: Added.
* css3/flexbox/flex-align-baseline-expected.txt:
* css3/flexbox/flex-align-baseline.html:
* css3/flexbox/multiline-align-self.html:

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

LayoutTests/ChangeLog
LayoutTests/css3/flexbox/align-baseline-expected.html [new file with mode: 0644]
LayoutTests/css3/flexbox/align-baseline.html [new file with mode: 0644]
LayoutTests/css3/flexbox/flex-align-baseline-expected.txt
LayoutTests/css3/flexbox/flex-align-baseline.html
LayoutTests/css3/flexbox/multiline-align-self.html
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderFlexibleBox.cpp
Source/WebCore/rendering/RenderFlexibleBox.h

index 6617959..af2bd4b 100644 (file)
@@ -1,3 +1,18 @@
+2012-10-01  Tony Chang  <tony@chromium.org>
+
+        flexbox does wrong baseline item alignment in columns
+        https://bugs.webkit.org/show_bug.cgi?id=97948
+
+        Reviewed by Ojan Vafai.
+
+        Update test cases with new baseline alignment for column.
+
+        * css3/flexbox/align-baseline-expected.html: Added.
+        * css3/flexbox/align-baseline.html: Added.
+        * css3/flexbox/flex-align-baseline-expected.txt:
+        * css3/flexbox/flex-align-baseline.html:
+        * css3/flexbox/multiline-align-self.html:
+
 2012-10-01  Yoshifumi Inoue  <yosin@chromium.org>
 
         Adding appearance tests for multiple fields week input UI
diff --git a/LayoutTests/css3/flexbox/align-baseline-expected.html b/LayoutTests/css3/flexbox/align-baseline-expected.html
new file mode 100644 (file)
index 0000000..84020c5
--- /dev/null
@@ -0,0 +1,27 @@
+<style>
+body {
+    margin: 0;
+}
+.flexbox {
+    display: -webkit-flex;
+    -webkit-flex-flow: column;
+    -webkit-align-items: flex-start;
+}
+.wrap-reverse {
+    -webkit-flex-wrap: wrap-reverse;
+}
+</style>
+
+<body>
+<div class='flexbox'>
+    <h1>This text</h1>
+    <p>should be left aligned.</p>
+</div>
+
+<div class='flexbox wrap-reverse'>
+    <h1>This text</h1>
+    <p>should be right aligned.</p>
+</div>
+</body>
+
+</html>
diff --git a/LayoutTests/css3/flexbox/align-baseline.html b/LayoutTests/css3/flexbox/align-baseline.html
new file mode 100644 (file)
index 0000000..a4faa56
--- /dev/null
@@ -0,0 +1,27 @@
+<style>
+body {
+    margin: 0;
+}
+.flexbox {
+    display: -webkit-flex;
+    -webkit-flex-flow: column;
+    -webkit-align-items: baseline;
+}
+.wrap-reverse {
+    -webkit-flex-wrap: wrap-reverse;
+}
+</style>
+
+<body>
+<div class='flexbox'>
+    <h1>This text</h1>
+    <p>should be left aligned.</p>
+</div>
+
+<div class='flexbox wrap-reverse'>
+    <h1>This text</h1>
+    <p>should be right aligned.</p>
+</div>
+</body>
+
+</html>
index c509cc2..7bd45a2 100644 (file)
@@ -16,19 +16,23 @@ PASS firstChild.offsetLeft is lastChild.offsetLeft
 PASS firstChild.offsetLeft is lastChild.offsetLeft
 PASS firstChild.offsetLeft is lastChild.offsetLeft
 PASS firstChild.offsetLeft is lastChild.offsetLeft
-PASS firstChild.offsetTop is lastChild.offsetTop
+PASS firstChild.offsetTop is 0
+PASS lastChild.offsetTop is 20
 PASS firstChild.offsetTop is lastChild.offsetTop
 PASS firstChild.offsetLeft is lastChild.offsetLeft
 PASS firstChild.offsetLeft is lastChild.offsetLeft
-PASS firstChild.offsetTop is lastChild.offsetTop
+PASS firstChild.offsetTop is 0
+PASS lastChild.offsetTop is 20
 PASS firstChild.offsetTop is lastChild.offsetTop
 PASS firstChild.offsetLeft is lastChild.offsetLeft
 PASS firstChild.offsetLeft is lastChild.offsetLeft
-PASS firstChild.offsetTop is lastChild.offsetTop
+PASS firstChild.offsetTop is 0
+PASS lastChild.offsetTop is 20
 PASS firstChild.offsetTop is lastChild.offsetTop
 PASS firstChild.offsetLeft is lastChild.offsetLeft
 PASS firstChild.offsetLeft is lastChild.offsetLeft
-PASS firstChild.offsetTop is lastChild.offsetTop
+PASS firstChild.offsetTop is 0
+PASS lastChild.offsetTop is 20
 PASS firstChild.offsetTop is lastChild.offsetTop
 PASS successfullyParsed is true
 
index 734ebcc..4927f71 100644 (file)
@@ -73,7 +73,7 @@ writingModes.forEach(function(writingMode) {
             document.body.appendChild(title);
 
             var isColumn = flexFlow.indexOf('column') != -1;
-            var isHorizontal = (writingMode.indexOf('horizontal') != -1) ? !isColumn : isColumn;
+            var isHorizontalFlow = (writingMode.indexOf('horizontal') != -1) ? !isColumn : isColumn;
 
             var container = document.createElement('div');
             container.innerHTML = '<div class="flexbox ' + flexboxClassName + '" style="-webkit-align-items: baseline;">' +
@@ -81,7 +81,10 @@ writingModes.forEach(function(writingMode) {
                 '<div style="margin-top:20px;"><div style="display:inline-block;"></div></div>' +
             '</div>';
 
-            container.firstChild.isHorizontal = isHorizontal;
+            container.firstChild.isHorizontal = writingMode.indexOf('horizontal') != -1;
+            container.firstChild.isHorizontalFlow = isHorizontalFlow;
+            container.firstChild.isColumn = isColumn;
+            container.firstChild.isLTR = direction == 'ltr';
             document.body.appendChild(container);
         })
     })
@@ -93,7 +96,10 @@ for (var i = 0, len = flexboxen.length; i < len; i++) {
     var flexbox = flexboxen[i];
     firstChild = flexbox.firstChild;
     lastChild = flexbox.lastChild;
-    if (flexbox.isHorizontal)
+    if (!flexbox.isHorizontal && flexbox.isColumn && flexbox.isLTR) {
+        shouldBe('firstChild.offsetTop', "0");
+        shouldBe('lastChild.offsetTop', "20");
+    } else if (flexbox.isHorizontalFlow)
         shouldBe('firstChild.offsetTop', 'lastChild.offsetTop');
     else
         shouldBe('firstChild.offsetLeft', 'lastChild.offsetLeft');
index 268a7b7..9856890 100644 (file)
@@ -182,14 +182,14 @@ var expectations = {
                     'child1': [10, 10, 0, 0],
                     'child2': [10, 10, 10, 10],
                     'child3': [10, 10, 20, 20],
-                    'child4': [10, 10, 5, 30],
+                    'child4': [10, 10, 0, 30],
                     'child5': [10, 10, 5, 40],
                     'child6': [30, 10, 0, 50],
                     'child7': [30, 10, 0, 60],
                     'child8': [10, 10, 30, 0],
                     'child9': [10, 10, 40, 10],
                     'child10': [10, 10, 50, 20],
-                    'child11': [10, 10, 35, 30],
+                    'child11': [10, 10, 30, 30],
                     'child12': [10, 10, 35, 40],
                     'child13': [30, 10, 30, 50],
                     'child14': [30, 10, 30, 60],
@@ -218,14 +218,14 @@ var expectations = {
                     'child1': [10, 10, 590, 0],
                     'child2': [10, 10, 580, 10],
                     'child3': [10, 10, 570, 20],
-                    'child4': [10, 10, 585, 30],
+                    'child4': [10, 10, 590, 30],
                     'child5': [10, 10, 585, 40],
                     'child6': [30, 10, 570, 50],
                     'child7': [30, 10, 570, 60],
                     'child8': [10, 10, 560, 0],
                     'child9': [10, 10, 550, 10],
                     'child10': [10, 10, 540, 20],
-                    'child11': [10, 10, 555, 30],
+                    'child11': [10, 10, 560, 30],
                     'child12': [10, 10, 555, 40],
                     'child13': [30, 10, 540, 50],
                     'child14': [30, 10, 540, 60],
@@ -330,14 +330,14 @@ var expectations = {
                     'child1': [10, 10, 0, 60],
                     'child2': [10, 10, 10, 50],
                     'child3': [10, 10, 20, 40],
-                    'child4': [10, 10, 5, 30],
+                    'child4': [10, 10, 0, 30],
                     'child5': [10, 10, 5, 20],
                     'child6': [30, 10, 0, 10],
                     'child7': [30, 10, 0, 0],
                     'child8': [10, 10, 30, 60],
                     'child9': [10, 10, 40, 50],
                     'child10': [10, 10, 50, 40],
-                    'child11': [10, 10, 35, 30],
+                    'child11': [10, 10, 30, 30],
                     'child12': [10, 10, 35, 20],
                     'child13': [30, 10, 30, 10],
                     'child14': [30, 10, 30, 0],
@@ -366,14 +366,14 @@ var expectations = {
                     'child1': [10, 10, 590, 60],
                     'child2': [10, 10, 580, 50],
                     'child3': [10, 10, 570, 40],
-                    'child4': [10, 10, 585, 30],
+                    'child4': [10, 10, 590, 30],
                     'child5': [10, 10, 585, 20],
                     'child6': [30, 10, 570, 10],
                     'child7': [30, 10, 570, 0],
                     'child8': [10, 10, 560, 60],
                     'child9': [10, 10, 550, 50],
                     'child10': [10, 10, 540, 40],
-                    'child11': [10, 10, 555, 30],
+                    'child11': [10, 10, 560, 30],
                     'child12': [10, 10, 555, 20],
                     'child13': [30, 10, 540, 10],
                     'child14': [30, 10, 540, 0],
@@ -480,14 +480,14 @@ var expectations = {
                     'child1': [10, 10, 0, 60],
                     'child2': [10, 10, 10, 50],
                     'child3': [10, 10, 20, 40],
-                    'child4': [10, 10, 5, 30],
+                    'child4': [10, 10, 0, 30],
                     'child5': [10, 10, 5, 20],
                     'child6': [30, 10, 0, 10],
                     'child7': [30, 10, 0, 0],
                     'child8': [10, 10, 30, 60],
                     'child9': [10, 10, 40, 50],
                     'child10': [10, 10, 50, 40],
-                    'child11': [10, 10, 35, 30],
+                    'child11': [10, 10, 30, 30],
                     'child12': [10, 10, 35, 20],
                     'child13': [30, 10, 30, 10],
                     'child14': [30, 10, 30, 0],
@@ -516,14 +516,14 @@ var expectations = {
                     'child1': [10, 10, 590, 60],
                     'child2': [10, 10, 580, 50],
                     'child3': [10, 10, 570, 40],
-                    'child4': [10, 10, 585, 30],
+                    'child4': [10, 10, 590, 30],
                     'child5': [10, 10, 585, 20],
                     'child6': [30, 10, 570, 10],
                     'child7': [30, 10, 570, 0],
                     'child8': [10, 10, 560, 60],
                     'child9': [10, 10, 550, 50],
                     'child10': [10, 10, 540, 40],
-                    'child11': [10, 10, 555, 30],
+                    'child11': [10, 10, 560, 30],
                     'child12': [10, 10, 555, 20],
                     'child13': [30, 10, 540, 10],
                     'child14': [30, 10, 540, 0],
@@ -628,14 +628,14 @@ var expectations = {
                     'child1': [10, 10, 0, 0],
                     'child2': [10, 10, 10, 10],
                     'child3': [10, 10, 20, 20],
-                    'child4': [10, 10, 5, 30],
+                    'child4': [10, 10, 0, 30],
                     'child5': [10, 10, 5, 40],
                     'child6': [30, 10, 0, 50],
                     'child7': [30, 10, 0, 60],
                     'child8': [10, 10, 30, 0],
                     'child9': [10, 10, 40, 10],
                     'child10': [10, 10, 50, 20],
-                    'child11': [10, 10, 35, 30],
+                    'child11': [10, 10, 30, 30],
                     'child12': [10, 10, 35, 40],
                     'child13': [30, 10, 30, 50],
                     'child14': [30, 10, 30, 60],
@@ -664,14 +664,14 @@ var expectations = {
                     'child1': [10, 10, 590, 0],
                     'child2': [10, 10, 580, 10],
                     'child3': [10, 10, 570, 20],
-                    'child4': [10, 10, 585, 30],
+                    'child4': [10, 10, 590, 30],
                     'child5': [10, 10, 585, 40],
                     'child6': [30, 10, 570, 50],
                     'child7': [30, 10, 570, 60],
                     'child8': [10, 10, 560, 0],
                     'child9': [10, 10, 550, 10],
                     'child10': [10, 10, 540, 20],
-                    'child11': [10, 10, 555, 30],
+                    'child11': [10, 10, 560, 30],
                     'child12': [10, 10, 555, 40],
                     'child13': [30, 10, 540, 50],
                     'child14': [30, 10, 540, 60],
@@ -778,14 +778,14 @@ var expectations = {
                     'child1': [10, 10, 60, 0],
                     'child2': [10, 10, 50, 10],
                     'child3': [10, 10, 40, 20],
-                    'child4': [10, 10, 30, 5],
+                    'child4': [10, 10, 30, 0],
                     'child5': [10, 10, 20, 5],
                     'child6': [10, 30, 10, 0],
                     'child7': [10, 30, 0, 0],
                     'child8': [10, 10, 60, 30],
                     'child9': [10, 10, 50, 40],
                     'child10': [10, 10, 40, 50],
-                    'child11': [10, 10, 30, 35],
+                    'child11': [10, 10, 30, 30],
                     'child12': [10, 10, 20, 35],
                     'child13': [10, 30, 10, 30],
                     'child14': [10, 30, 0, 30],
@@ -814,14 +814,14 @@ var expectations = {
                     'child1': [10, 10, 60, 160],
                     'child2': [10, 10, 50, 150],
                     'child3': [10, 10, 40, 140],
-                    'child4': [10, 10, 30, 155],
+                    'child4': [10, 10, 30, 160],
                     'child5': [10, 10, 20, 155],
                     'child6': [10, 30, 10, 140],
                     'child7': [10, 30, 0, 140],
                     'child8': [10, 10, 60, 130],
                     'child9': [10, 10, 50, 120],
                     'child10': [10, 10, 40, 110],
-                    'child11': [10, 10, 30, 125],
+                    'child11': [10, 10, 30, 130],
                     'child12': [10, 10, 20, 125],
                     'child13': [10, 30, 10, 110],
                     'child14': [10, 30, 0, 110],
@@ -926,14 +926,14 @@ var expectations = {
                     'child1': [10, 10, 0, 0],
                     'child2': [10, 10, 10, 10],
                     'child3': [10, 10, 20, 20],
-                    'child4': [10, 10, 30, 5],
+                    'child4': [10, 10, 30, 0],
                     'child5': [10, 10, 40, 5],
                     'child6': [10, 30, 50, 0],
                     'child7': [10, 30, 60, 0],
                     'child8': [10, 10, 0, 30],
                     'child9': [10, 10, 10, 40],
                     'child10': [10, 10, 20, 50],
-                    'child11': [10, 10, 30, 35],
+                    'child11': [10, 10, 30, 30],
                     'child12': [10, 10, 40, 35],
                     'child13': [10, 30, 50, 30],
                     'child14': [10, 30, 60, 30],
@@ -962,14 +962,14 @@ var expectations = {
                     'child1': [10, 10, 0, 160],
                     'child2': [10, 10, 10, 150],
                     'child3': [10, 10, 20, 140],
-                    'child4': [10, 10, 30, 155],
+                    'child4': [10, 10, 30, 160],
                     'child5': [10, 10, 40, 155],
                     'child6': [10, 30, 50, 140],
                     'child7': [10, 30, 60, 140],
                     'child8': [10, 10, 0, 130],
                     'child9': [10, 10, 10, 120],
                     'child10': [10, 10, 20, 110],
-                    'child11': [10, 10, 30, 125],
+                    'child11': [10, 10, 30, 130],
                     'child12': [10, 10, 40, 125],
                     'child13': [10, 30, 50, 110],
                     'child14': [10, 30, 60, 110],
@@ -1076,14 +1076,14 @@ var expectations = {
                     'child1': [10, 10, 0, 0],
                     'child2': [10, 10, 10, 10],
                     'child3': [10, 10, 20, 20],
-                    'child4': [10, 10, 30, 5],
+                    'child4': [10, 10, 30, 0],
                     'child5': [10, 10, 40, 5],
                     'child6': [10, 30, 50, 0],
                     'child7': [10, 30, 60, 0],
                     'child8': [10, 10, 0, 30],
                     'child9': [10, 10, 10, 40],
                     'child10': [10, 10, 20, 50],
-                    'child11': [10, 10, 30, 35],
+                    'child11': [10, 10, 30, 30],
                     'child12': [10, 10, 40, 35],
                     'child13': [10, 30, 50, 30],
                     'child14': [10, 30, 60, 30],
@@ -1112,14 +1112,14 @@ var expectations = {
                     'child1': [10, 10, 0, 160],
                     'child2': [10, 10, 10, 150],
                     'child3': [10, 10, 20, 140],
-                    'child4': [10, 10, 30, 155],
+                    'child4': [10, 10, 30, 160],
                     'child5': [10, 10, 40, 155],
                     'child6': [10, 30, 50, 140],
                     'child7': [10, 30, 60, 140],
                     'child8': [10, 10, 0, 130],
                     'child9': [10, 10, 10, 120],
                     'child10': [10, 10, 20, 110],
-                    'child11': [10, 10, 30, 125],
+                    'child11': [10, 10, 30, 130],
                     'child12': [10, 10, 40, 125],
                     'child13': [10, 30, 50, 110],
                     'child14': [10, 30, 60, 110],
@@ -1224,14 +1224,14 @@ var expectations = {
                     'child1': [10, 10, 60, 0],
                     'child2': [10, 10, 50, 10],
                     'child3': [10, 10, 40, 20],
-                    'child4': [10, 10, 30, 5],
+                    'child4': [10, 10, 30, 0],
                     'child5': [10, 10, 20, 5],
                     'child6': [10, 30, 10, 0],
                     'child7': [10, 30, 0, 0],
                     'child8': [10, 10, 60, 30],
                     'child9': [10, 10, 50, 40],
                     'child10': [10, 10, 40, 50],
-                    'child11': [10, 10, 30, 35],
+                    'child11': [10, 10, 30, 30],
                     'child12': [10, 10, 20, 35],
                     'child13': [10, 30, 10, 30],
                     'child14': [10, 30, 0, 30],
@@ -1260,14 +1260,14 @@ var expectations = {
                     'child1': [10, 10, 60, 160],
                     'child2': [10, 10, 50, 150],
                     'child3': [10, 10, 40, 140],
-                    'child4': [10, 10, 30, 155],
+                    'child4': [10, 10, 30, 160],
                     'child5': [10, 10, 20, 155],
                     'child6': [10, 30, 10, 140],
                     'child7': [10, 30, 0, 140],
                     'child8': [10, 10, 60, 130],
                     'child9': [10, 10, 50, 120],
                     'child10': [10, 10, 40, 110],
-                    'child11': [10, 10, 30, 125],
+                    'child11': [10, 10, 30, 130],
                     'child12': [10, 10, 20, 125],
                     'child13': [10, 30, 10, 110],
                     'child14': [10, 30, 0, 110],
index c88d687..b49425e 100644 (file)
@@ -1,3 +1,20 @@
+2012-10-01  Tony Chang  <tony@chromium.org>
+
+        flexbox does wrong baseline item alignment in columns
+        https://bugs.webkit.org/show_bug.cgi?id=97948
+
+        Reviewed by Ojan Vafai.
+
+        For columns, baseline alignment should just be flex-start.  We were previously
+        moving the logical left edge by the ascent.
+
+        Test: css3/flexbox/align-baseline.html
+
+        * rendering/RenderFlexibleBox.cpp:
+        (WebCore::RenderFlexibleBox::alignmentForChild): Map baseline to flex-start for orthogonal children.
+        (WebCore::RenderFlexibleBox::alignChildren): Add FIXME for bug in baseline alignment.
+        * rendering/RenderFlexibleBox.h:
+
 2012-10-01  Keishi Hattori  <keishi@webkit.org>
 
         Rename CalendarPickerElement to PickerIndicatorElement
index ef4ee94..97016dd 100644 (file)
@@ -946,13 +946,16 @@ void RenderFlexibleBox::prepareChildForPositionedLayout(RenderBox* child, Layout
     }
 }
 
-static EAlignItems alignmentForChild(RenderBox* child)
+EAlignItems RenderFlexibleBox::alignmentForChild(RenderBox* child) const
 {
     EAlignItems align = child->style()->alignSelf();
     if (align == AlignAuto)
-        align = child->parent()->style()->alignItems();
+        align = style()->alignItems();
 
-    if (child->parent()->style()->flexWrap() == FlexWrapReverse) {
+    if (align == AlignBaseline && hasOrthogonalFlow(child))
+        align = AlignFlexStart;
+
+    if (style()->flexWrap() == FlexWrapReverse) {
         if (align == AlignFlexStart)
             align = AlignFlexEnd;
         else if (align == AlignFlexEnd)
@@ -1192,6 +1195,8 @@ void RenderFlexibleBox::alignChildren(OrderIterator& iterator, const WTF::Vector
                 adjustAlignmentForChild(child, availableAlignmentSpaceForChild(lineCrossAxisExtent, child) / 2);
                 break;
             case AlignBaseline: {
+                // FIXME: If we get here in columns, we want the use the descent, except we currently can't get the ascent/descent of orthogonal children.
+                // https://bugs.webkit.org/show_bug.cgi?id=98076
                 LayoutUnit ascent = marginBoxAscentForChild(child);
                 LayoutUnit startOffset = maxAscent - ascent;
                 adjustAlignmentForChild(child, startOffset);
index 2ace3c4..d2be8da 100644 (file)
@@ -106,6 +106,7 @@ private:
     // FIXME: Supporting layout deltas.
     void setFlowAwareLocationForChild(RenderBox* child, const LayoutPoint&);
     void adjustAlignmentForChild(RenderBox* child, LayoutUnit);
+    EAlignItems alignmentForChild(RenderBox* child) const;
     LayoutUnit mainAxisBorderAndPaddingExtentForChild(RenderBox* child) const;
     LayoutUnit mainAxisScrollbarExtentForChild(RenderBox* child) const;
     LayoutUnit preferredMainAxisContentExtentForChild(RenderBox* child);