- Revision
- 207535
- Author
- jfernan...@igalia.com
- Date
- 2016-10-19 06:42:43 -0700 (Wed, 19 Oct 2016)
Log Message
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.
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:
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (207534 => 207535)
--- trunk/LayoutTests/ChangeLog 2016-10-19 11:39:34 UTC (rev 207534)
+++ trunk/LayoutTests/ChangeLog 2016-10-19 13:42:43 UTC (rev 207535)
@@ -1,3 +1,17 @@
+2016-10-19 Javier Fernandez <jfernan...@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.no...@apple.com>
[Mac][MSE] Movies with a 'mvex' box have a zero-duration
Added: trunk/LayoutTests/css3/flexbox/flexbox-lines-must-be-stretched-by-default-expected.txt (0 => 207535)
--- trunk/LayoutTests/css3/flexbox/flexbox-lines-must-be-stretched-by-default-expected.txt (rev 0)
+++ trunk/LayoutTests/css3/flexbox/flexbox-lines-must-be-stretched-by-default-expected.txt 2016-10-19 13:42:43 UTC (rev 207535)
@@ -0,0 +1,7 @@
+'Test for BUG=647694 - align-content "stretch" is not applied by default when grid is disabled.'
+
+PASS
+
+PASS
+
+
Added: trunk/LayoutTests/css3/flexbox/flexbox-lines-must-be-stretched-by-default.html (0 => 207535)
--- trunk/LayoutTests/css3/flexbox/flexbox-lines-must-be-stretched-by-default.html (rev 0)
+++ trunk/LayoutTests/css3/flexbox/flexbox-lines-must-be-stretched-by-default.html 2016-10-19 13:42:43 UTC (rev 207535)
@@ -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=""
+<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>
Modified: trunk/LayoutTests/fast/css/ensure-flexbox-compatibility-with-initial-values-expected.txt (207534 => 207535)
--- trunk/LayoutTests/fast/css/ensure-flexbox-compatibility-with-initial-values-expected.txt 2016-10-19 11:39:34 UTC (rev 207534)
+++ trunk/LayoutTests/fast/css/ensure-flexbox-compatibility-with-initial-values-expected.txt 2016-10-19 13:42:43 UTC (rev 207535)
@@ -25,19 +25,19 @@
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
Modified: trunk/Source/WebCore/ChangeLog (207534 => 207535)
--- trunk/Source/WebCore/ChangeLog 2016-10-19 11:39:34 UTC (rev 207534)
+++ trunk/Source/WebCore/ChangeLog 2016-10-19 13:42:43 UTC (rev 207535)
@@ -1,3 +1,34 @@
+2016-10-19 Javier Fernandez <jfernan...@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 <clo...@igalia.com>
[GTK] REGRESSION(r207396) Build broken with Clang.
Modified: trunk/Source/WebCore/css/CSSComputedStyleDeclaration.cpp (207534 => 207535)
--- trunk/Source/WebCore/css/CSSComputedStyleDeclaration.cpp 2016-10-19 11:39:34 UTC (rev 207534)
+++ trunk/Source/WebCore/css/CSSComputedStyleDeclaration.cpp 2016-10-19 13:42:43 UTC (rev 207535)
@@ -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 @@
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 @@
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 @@
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()));
Modified: trunk/Source/WebCore/rendering/style/RenderStyle.h (207534 => 207535)
--- trunk/Source/WebCore/rendering/style/RenderStyle.h 2016-10-19 11:39:34 UTC (rev 207534)
+++ trunk/Source/WebCore/rendering/style/RenderStyle.h 2016-10-19 13:42:43 UTC (rev 207535)
@@ -2004,7 +2004,7 @@
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; }
