Web Inspector: Debugger buttons positioned incorrectly, align-content default value...
authorjfernandez@igalia.com <jfernandez@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 19 Oct 2016 13:42:43 +0000 (13:42 +0000)
committerjfernandez@igalia.com <jfernandez@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 19 Oct 2016 13:42:43 +0000 (13:42 +0000)
https://bugs.webkit.org/show_bug.cgi?id=163572

Reviewed by Sergio Villar Senin.

Source/WebCore:

We only allow the new CSS Box Alignment syntax when the Grid Layout
feature is enabled. Due to flexbox backward compatibility we have
implemented a different code path for the style initial/default values
assignment. However, we have incorrectly resolved both align-content
and justify-content to 'flex-start' when grid layout is disabled.

This patch changes the approach, so we set 'normal' (the value specified
by the new syntax) for both properties, but using the values defined in
the old syntax (Flexbox specification) at computed style resolution.

Since 'stretch' is the default value for the align-content property, this
issue implies that any flexbox line with an undefined height will be
laid out incorrectly, if not explicitly set via CSS, because flex items
can't use the available height, even though they use 'stretch' for their
'align-self' properties.

Test: css3/flexbox/flexbox-lines-must-be-stretched-by-default.html

* css/CSSComputedStyleDeclaration.cpp:
(WebCore::valueForContentPositionAndDistributionWithOverflowAlignment):
(WebCore::ComputedStyleExtractor::propertyValue):
* rendering/style/RenderStyle.h:
(WebCore::RenderStyle::initialContentAlignment):

LayoutTests:

Modified test cases for initial values.
Added regression test for the align-content issue.

* css3/flexbox/flexbox-lines-must-be-stretched-by-default-expected.txt: Added.
* css3/flexbox/flexbox-lines-must-be-stretched-by-default.html: Added.
* fast/css/ensure-flexbox-compatibility-with-initial-values-expected.txt:

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

LayoutTests/ChangeLog
LayoutTests/css3/flexbox/flexbox-lines-must-be-stretched-by-default-expected.txt [new file with mode: 0644]
LayoutTests/css3/flexbox/flexbox-lines-must-be-stretched-by-default.html [new file with mode: 0644]
LayoutTests/fast/css/ensure-flexbox-compatibility-with-initial-values-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/css/CSSComputedStyleDeclaration.cpp
Source/WebCore/rendering/style/RenderStyle.h

index fca34cf..6692065 100644 (file)
@@ -1,3 +1,17 @@
+2016-10-19  Javier Fernandez  <jfernandez@igalia.com>
+
+        Web Inspector: Debugger buttons positioned incorrectly, align-content default value is unexpected
+        https://bugs.webkit.org/show_bug.cgi?id=163572
+
+        Reviewed by Sergio Villar Senin.
+
+        Modified test cases for initial values.
+        Added regression test for the align-content issue.
+
+        * css3/flexbox/flexbox-lines-must-be-stretched-by-default-expected.txt: Added.
+        * css3/flexbox/flexbox-lines-must-be-stretched-by-default.html: Added.
+        * fast/css/ensure-flexbox-compatibility-with-initial-values-expected.txt:
+
 2016-10-19  Jer Noble  <jer.noble@apple.com>
 
         [Mac][MSE] Movies with a 'mvex' box have a zero-duration
diff --git a/LayoutTests/css3/flexbox/flexbox-lines-must-be-stretched-by-default-expected.txt b/LayoutTests/css3/flexbox/flexbox-lines-must-be-stretched-by-default-expected.txt
new file mode 100644 (file)
index 0000000..7a29d9f
--- /dev/null
@@ -0,0 +1,7 @@
+'Test for BUG=647694 - align-content "stretch" is not applied by default when grid is disabled.'
+
+PASS
+
+PASS
+
+
diff --git a/LayoutTests/css3/flexbox/flexbox-lines-must-be-stretched-by-default.html b/LayoutTests/css3/flexbox/flexbox-lines-must-be-stretched-by-default.html
new file mode 100644 (file)
index 0000000..c0ae6ac
--- /dev/null
@@ -0,0 +1,63 @@
+<!DOCTYPE html>
+<style>
+.flex-container {
+    display: flex;
+    height:100px;
+    width:200px;
+    background-color:pink;
+    flex-wrap: wrap;
+}
+.flex-item1 {
+    width: 100%;
+    background-color:blue;
+    border:1px solid;
+    border-color:red;
+}
+.flex-item2 {
+    width: 100%;
+    background-color:blue;
+}
+</style>
+<script src="../../resources/check-layout.js"></script>
+<body>
+<p>'Test for BUG=647694 - align-content "stretch" is not applied by default when grid is disabled.'</p>
+<script>
+function runTest(gridEnabled)
+{
+    if (window.internals)
+        window.internals.setCSSGridLayoutEnabled(gridEnabled);
+
+    var flexContainer = document.createElement("div");
+    if (gridEnabled)
+        flexContainer.className += "gridEnabled flex-container";
+    else
+        flexContainer.className += "gridDisabled flex-container";
+    document.body.appendChild(flexContainer);
+
+    var flexItem1 = document.createElement("div");
+    flexItem1.id = "flexItem1";
+    flexItem1.className += "flex-item1";
+    flexItem1.setAttribute("data-expected-height", "51");
+    flexContainer.appendChild(flexItem1);
+
+    var flexItem2 = document.createElement("div");
+    flexItem2.id = "flexItem2";
+    flexItem2.className += "flex-item2";
+    flexItem2.setAttribute("data-expected-height", "49");
+    flexContainer.appendChild(flexItem2);
+
+    var br = document.createElement("br");
+    document.body.appendChild(br);
+
+    flexContainer.style.alignContent = "initial";
+
+    if (gridEnabled)
+        checkLayout('.gridEnabled');
+    else
+        checkLayout('.gridDisabled');
+}
+
+runTest(false);
+runTest(true);
+</script>
+</body>
index 74e9fd8..b1a3b03 100644 (file)
@@ -25,19 +25,19 @@ PASS CSS.supports('justify-content', 'normal') is true
 Verifying initial values are supported when grid is DISABLED.
 PASS CSS.supports('align-items', 'stretch') is true
 PASS CSS.supports('align-self', 'stretch') is true
-PASS CSS.supports('align-content', 'flex-start') is true
+PASS CSS.supports('align-content', 'stretch') is true
 PASS CSS.supports('justify-content', 'flex-start') is true
 PASS CSS.supports('align-items', 'stretch') is true
 PASS CSS.supports('align-self', 'stretch') is true
-PASS CSS.supports('align-content', 'flex-start') is true
+PASS CSS.supports('align-content', 'stretch') is true
 PASS CSS.supports('justify-content', 'flex-start') is true
 PASS CSS.supports('align-items', 'stretch') is true
 PASS CSS.supports('align-self', 'stretch') is true
-PASS CSS.supports('align-content', 'flex-start') is true
+PASS CSS.supports('align-content', 'stretch') is true
 PASS CSS.supports('justify-content', 'flex-start') is true
 PASS CSS.supports('align-items', 'stretch') is true
 PASS CSS.supports('align-self', 'stretch') is true
-PASS CSS.supports('align-content', 'flex-start') is true
+PASS CSS.supports('align-content', 'stretch') is true
 PASS CSS.supports('justify-content', 'flex-start') is true
 PASS successfullyParsed is true
 
index 9638948..cb5c1e7 100644 (file)
@@ -1,3 +1,34 @@
+2016-10-19  Javier Fernandez  <jfernandez@igalia.com>
+
+        Web Inspector: Debugger buttons positioned incorrectly, align-content default value is unexpected
+        https://bugs.webkit.org/show_bug.cgi?id=163572
+
+        Reviewed by Sergio Villar Senin.
+
+        We only allow the new CSS Box Alignment syntax when the Grid Layout
+        feature is enabled. Due to flexbox backward compatibility we have
+        implemented a different code path for the style initial/default values
+        assignment. However, we have incorrectly resolved both align-content
+        and justify-content to 'flex-start' when grid layout is disabled.
+
+        This patch changes the approach, so we set 'normal' (the value specified
+        by the new syntax) for both properties, but using the values defined in
+        the old syntax (Flexbox specification) at computed style resolution.
+
+        Since 'stretch' is the default value for the align-content property, this
+        issue implies that any flexbox line with an undefined height will be
+        laid out incorrectly, if not explicitly set via CSS, because flex items
+        can't use the available height, even though they use 'stretch' for their
+        'align-self' properties.
+
+        Test: css3/flexbox/flexbox-lines-must-be-stretched-by-default.html
+
+        * css/CSSComputedStyleDeclaration.cpp:
+        (WebCore::valueForContentPositionAndDistributionWithOverflowAlignment):
+        (WebCore::ComputedStyleExtractor::propertyValue):
+        * rendering/style/RenderStyle.h:
+        (WebCore::RenderStyle::initialContentAlignment):
+
 2016-10-19  Carlos Alberto Lopez Perez  <clopez@igalia.com>
 
         [GTK] REGRESSION(r207396) Build broken with Clang.
index 40f1e18..a70a871 100644 (file)
@@ -62,6 +62,7 @@
 #include "RenderBlock.h"
 #include "RenderBox.h"
 #include "RenderStyle.h"
+#include "RuntimeEnabledFeatures.h"
 #include "SVGElement.h"
 #include "Settings.h"
 #include "ShapeValue.h"
@@ -2414,14 +2415,22 @@ static Ref<CSSValueList> valueForItemPositionWithOverflowAlignment(const StyleSe
     return result;
 }
 
-static Ref<CSSValueList> valueForContentPositionAndDistributionWithOverflowAlignment(const StyleContentAlignmentData& data)
+static Ref<CSSValueList> valueForContentPositionAndDistributionWithOverflowAlignment(const StyleContentAlignmentData& data, CSSValueID normalBehaviorValueID)
 {
     auto& cssValuePool = CSSValuePool::singleton();
     auto result = CSSValueList::createSpaceSeparated();
     if (data.distribution() != ContentDistributionDefault)
         result.get().append(cssValuePool.createValue(data.distribution()));
-    if (data.distribution() == ContentDistributionDefault || data.position() != ContentPositionNormal)
-        result.get().append(cssValuePool.createValue(data.position()));
+    if (data.distribution() == ContentDistributionDefault || data.position() != ContentPositionNormal) {
+        bool gridEnabled = false;
+#if ENABLE(CSS_GRID_LAYOUT)
+        gridEnabled = RuntimeEnabledFeatures::sharedFeatures().isCSSGridLayoutEnabled();
+#endif
+        if (data.position() != ContentPositionNormal || gridEnabled)
+            result.get().append(cssValuePool.createValue(data.position()));
+        else
+            result.get().append(cssValuePool.createIdentifierValue(normalBehaviorValueID));
+    }
     if ((data.position() >= ContentPositionCenter || data.distribution() != ContentDistributionDefault) && data.overflow() != OverflowAlignmentDefault)
         result.get().append(cssValuePool.createValue(data.overflow()));
     ASSERT(result.get().length() > 0);
@@ -2803,7 +2812,7 @@ RefPtr<CSSValue> ComputedStyleExtractor::propertyValue(CSSPropertyID propertyID,
         case CSSPropertyEmptyCells:
             return cssValuePool.createValue(style->emptyCells());
         case CSSPropertyAlignContent:
-            return valueForContentPositionAndDistributionWithOverflowAlignment(style->alignContent());
+            return valueForContentPositionAndDistributionWithOverflowAlignment(style->alignContent(), CSSValueStretch);
         case CSSPropertyAlignItems:
             return valueForItemPositionWithOverflowAlignment(style->alignItems());
         case CSSPropertyAlignSelf:
@@ -2823,7 +2832,7 @@ RefPtr<CSSValue> ComputedStyleExtractor::propertyValue(CSSPropertyID propertyID,
         case CSSPropertyFlexWrap:
             return cssValuePool.createValue(style->flexWrap());
         case CSSPropertyJustifyContent:
-            return valueForContentPositionAndDistributionWithOverflowAlignment(style->justifyContent());
+            return valueForContentPositionAndDistributionWithOverflowAlignment(style->justifyContent(), CSSValueFlexStart);
 #if ENABLE(CSS_GRID_LAYOUT)
         case CSSPropertyJustifyItems:
             return valueForItemPositionWithOverflowAlignment(resolveJustifyItemsAuto(style->justifyItems(), styledNode->parentNode()));
index 39fe327..87fa746 100644 (file)
@@ -2004,7 +2004,7 @@ public:
     static int initialOrder() { return 0; }
     static StyleSelfAlignmentData initialSelfAlignment() { return StyleSelfAlignmentData(ItemPositionAuto, OverflowAlignmentDefault); }
     static StyleSelfAlignmentData initialDefaultAlignment() { return StyleSelfAlignmentData(isCSSGridLayoutEnabled() ? ItemPositionNormal : ItemPositionStretch, OverflowAlignmentDefault); }
-    static StyleContentAlignmentData initialContentAlignment() { return StyleContentAlignmentData(isCSSGridLayoutEnabled() ? ContentPositionNormal : ContentPositionFlexStart, ContentDistributionDefault, OverflowAlignmentDefault); }
+    static StyleContentAlignmentData initialContentAlignment() { return StyleContentAlignmentData(ContentPositionNormal, ContentDistributionDefault, OverflowAlignmentDefault); }
     static EFlexDirection initialFlexDirection() { return FlowRow; }
     static EFlexWrap initialFlexWrap() { return FlexNoWrap; }
     static int initialMarqueeLoopCount() { return -1; }