- Revision
- 205102
- Author
- [email protected]
- Date
- 2016-08-28 07:45:43 -0700 (Sun, 28 Aug 2016)
Log Message
Should never be reached failure in WebCore::RenderFlexibleBox::alignChildren
https://bugs.webkit.org/show_bug.cgi?id=151591
<rdar://problem/27711829>
Reviewed by Darin Adler.
Source/WebCore:
The align-self and align-items CSS properties were originally defined in the
Flexbible Box specification. The new CSS Box Alignment specification tries
to generalize them so they can be used by other layout models, as it's the
case of the GridLayout spec.
Since we have implemented the Grid Layout spec behind a runtime flag, we should
do the same with the new syntax of these properties.
Test: css3/flexbox/new-alignment-values-invalid-if-grid-not-enabled.html
* css/CSSParser.cpp:
(WebCore::isValidKeywordPropertyAndValue):
(WebCore::isKeywordPropertyID):
(WebCore::CSSParser::parseValue):
* rendering/RenderFlexibleBox.cpp:
(WebCore::RenderFlexibleBox::alignChildren):
LayoutTests:
Test to verify that align-self and align-items CSS properties use the old
syntax when the Grid Layout feature is not enabled.
* css3/flexbox/new-alignment-values-invalid-if-grid-not-enabled-expected.txt: Added.
* css3/flexbox/new-alignment-values-invalid-if-grid-not-enabled.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (205101 => 205102)
--- trunk/LayoutTests/ChangeLog 2016-08-28 09:35:33 UTC (rev 205101)
+++ trunk/LayoutTests/ChangeLog 2016-08-28 14:45:43 UTC (rev 205102)
@@ -1,3 +1,17 @@
+2016-08-28 Javier Fernandez <[email protected]>
+
+ Should never be reached failure in WebCore::RenderFlexibleBox::alignChildren
+ https://bugs.webkit.org/show_bug.cgi?id=151591
+ <rdar://problem/27711829>
+
+ Reviewed by Darin Adler.
+
+ Test to verify that align-self and align-items CSS properties use the old
+ syntax when the Grid Layout feature is not enabled.
+
+ * css3/flexbox/new-alignment-values-invalid-if-grid-not-enabled-expected.txt: Added.
+ * css3/flexbox/new-alignment-values-invalid-if-grid-not-enabled.html: Added.
+
2016-08-28 Frederic Wang <[email protected]>
Improve parsing of the menclose notation attribute value
Added: trunk/LayoutTests/css3/flexbox/new-alignment-values-invalid-if-grid-not-enabled-expected.txt (0 => 205102)
--- trunk/LayoutTests/css3/flexbox/new-alignment-values-invalid-if-grid-not-enabled-expected.txt (rev 0)
+++ trunk/LayoutTests/css3/flexbox/new-alignment-values-invalid-if-grid-not-enabled-expected.txt 2016-08-28 14:45:43 UTC (rev 205102)
@@ -0,0 +1,57 @@
+Test to verify that the new alignment values are parsed as invalid if Grid Layout is disabled and in any case they do not cause a crash because assertions in flexbox layout code.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+
+New alignment values should be invalid when grid layout is disabled
+
+Testing Self-Alignment values.
+PASS alignSelf is 'flex-start'
+PASS alignSelf is 'flex-start'
+PASS alignSelf is 'flex-start'
+PASS alignSelf is 'flex-start'
+PASS alignSelf is 'flex-start'
+PASS alignSelf is 'flex-start'
+
+Testing Default-Alignment values.
+PASS alignItems is 'flex-end'
+PASS alignSelf is 'flex-end'
+PASS alignItems is 'flex-end'
+PASS alignSelf is 'flex-end'
+PASS alignItems is 'flex-end'
+PASS alignSelf is 'flex-end'
+PASS alignItems is 'flex-end'
+PASS alignSelf is 'flex-end'
+PASS alignItems is 'flex-end'
+PASS alignSelf is 'flex-end'
+PASS alignItems is 'flex-end'
+PASS alignSelf is 'flex-end'
+
+Even when grid layout is enabled, new values should not violate assertions in FlexibleBox layout logic.
+
+Testing Self-Alignment values.
+PASS alignSelf is 'start unsafe'
+PASS alignSelf is 'start'
+PASS alignSelf is 'end'
+PASS alignSelf is 'flex-start safe'
+PASS alignSelf is 'self-start'
+PASS alignSelf is 'self-end'
+
+Testing Default-Alignment values.
+PASS alignItems is 'start unsafe'
+PASS alignSelf is 'start unsafe'
+PASS alignItems is 'start'
+PASS alignSelf is 'start'
+PASS alignItems is 'end'
+PASS alignSelf is 'end'
+PASS alignItems is 'flex-start safe'
+PASS alignSelf is 'flex-start safe'
+PASS alignItems is 'self-start'
+PASS alignSelf is 'self-start'
+PASS alignItems is 'self-end'
+PASS alignSelf is 'self-end'
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/css3/flexbox/new-alignment-values-invalid-if-grid-not-enabled.html (0 => 205102)
--- trunk/LayoutTests/css3/flexbox/new-alignment-values-invalid-if-grid-not-enabled.html (rev 0)
+++ trunk/LayoutTests/css3/flexbox/new-alignment-values-invalid-if-grid-not-enabled.html 2016-08-28 14:45:43 UTC (rev 205102)
@@ -0,0 +1,73 @@
+<!DOCTYPE html>
+<div id="flexContainer" style="display: flex">
+ <div id="flexItem"></div>
+</div>
+<script src=""
+<script>
+description('Test to verify that the new alignment values are parsed as invalid if Grid Layout is disabled and in any case they do not cause a crash because assertions in flexbox layout code.');
+
+function checkAlignSelfValue(value, computedValue, gridEnabled)
+{
+ item.style.webkitAlignSelf = value;
+ alignSelf = getComputedStyle(item, '').getPropertyValue('-webkit-align-self');
+ if (gridEnabled)
+ shouldBe("alignSelf", computedValue);
+ else
+ shouldBe("alignSelf", "'flex-start'");
+}
+
+function checkAlignItemsValue(value, computedValue, gridEnabled)
+{
+ container.style.webkitAlignItems = value;
+ alignItems = getComputedStyle(container, '').getPropertyValue('-webkit-align-items');
+ alignSelf = getComputedStyle(item, '').getPropertyValue('-webkit-align-self');
+ if (gridEnabled) {
+ shouldBe("alignItems", computedValue);
+ shouldBe("alignSelf", computedValue);
+ } else {
+ shouldBe("alignItems", "'flex-end'");
+ shouldBe("alignSelf", "'flex-end'");
+ }
+}
+
+function checkAlignmentValues(gridEnabled)
+{
+ if (window.internals)
+ internals.setCSSGridLayoutEnabled(gridEnabled);
+
+ debug('<br>Testing Self-Alignment values.');
+ checkAlignSelfValue("start unsafe", "'start unsafe'", gridEnabled)
+ checkAlignSelfValue("start", "'start'", gridEnabled)
+ checkAlignSelfValue("end", "'end'", gridEnabled)
+ checkAlignSelfValue("flex-start safe", "'flex-start safe'", gridEnabled)
+ checkAlignSelfValue("self-start", "'self-start'", gridEnabled)
+ checkAlignSelfValue("self-end", "'self-end'", gridEnabled)
+
+ item.style.webkitAlignSelf = "auto";
+
+ debug('<br>Testing Default-Alignment values.');
+ checkAlignItemsValue("start unsafe", "'start unsafe'", gridEnabled)
+ checkAlignItemsValue("start", "'start'", gridEnabled)
+ checkAlignItemsValue("end", "'end'", gridEnabled)
+ checkAlignItemsValue("flex-start safe", "'flex-start safe'", gridEnabled)
+ checkAlignItemsValue("self-start", "'self-start'", gridEnabled)
+ checkAlignItemsValue("self-end", "'self-end'", gridEnabled)
+
+ item.style.webkitAlignSelf = "flex-start";
+}
+
+var container = document.getElementById("flexContainer");
+var item = document.getElementById("flexItem");
+
+container.style.webkitAlignItems = "flex-end";
+item.style.webkitAlignSelf = "flex-start";
+var alignSelf = "flex-start";
+var alignItems = "flex-start";
+
+debug('<br>New alignment values should be invalid when grid layout is disabled');
+checkAlignmentValues(false);
+
+debug('<br>Even when grid layout is enabled, new values should not violate assertions in FlexibleBox layout logic.');
+checkAlignmentValues(true);
+
+</script>
Modified: trunk/Source/WebCore/ChangeLog (205101 => 205102)
--- trunk/Source/WebCore/ChangeLog 2016-08-28 09:35:33 UTC (rev 205101)
+++ trunk/Source/WebCore/ChangeLog 2016-08-28 14:45:43 UTC (rev 205102)
@@ -1,3 +1,28 @@
+2016-08-28 Javier Fernandez <[email protected]>
+
+ Should never be reached failure in WebCore::RenderFlexibleBox::alignChildren
+ https://bugs.webkit.org/show_bug.cgi?id=151591
+ <rdar://problem/27711829>
+
+ Reviewed by Darin Adler.
+
+ The align-self and align-items CSS properties were originally defined in the
+ Flexbible Box specification. The new CSS Box Alignment specification tries
+ to generalize them so they can be used by other layout models, as it's the
+ case of the GridLayout spec.
+
+ Since we have implemented the Grid Layout spec behind a runtime flag, we should
+ do the same with the new syntax of these properties.
+
+ Test: css3/flexbox/new-alignment-values-invalid-if-grid-not-enabled.html
+
+ * css/CSSParser.cpp:
+ (WebCore::isValidKeywordPropertyAndValue):
+ (WebCore::isKeywordPropertyID):
+ (WebCore::CSSParser::parseValue):
+ * rendering/RenderFlexibleBox.cpp:
+ (WebCore::RenderFlexibleBox::alignChildren):
+
2016-08-28 Frederic Wang <[email protected]>
Improve parsing of the menclose notation attribute value
Modified: trunk/Source/WebCore/css/parser/CSSParser.cpp (205101 => 205102)
--- trunk/Source/WebCore/css/parser/CSSParser.cpp 2016-08-28 09:35:33 UTC (rev 205101)
+++ trunk/Source/WebCore/css/parser/CSSParser.cpp 2016-08-28 14:45:43 UTC (rev 205102)
@@ -819,6 +819,18 @@
if (valueID == CSSValueAuto || valueID == CSSValueBalance)
return true;
break;
+ case CSSPropertyAlignItems:
+ // FIXME: Per CSS alignment, this property should accept the same arguments as 'justify-self' so we should share its parsing code.
+ // FIXME: For now, we will do it behind the GRID_LAYOUT compile flag.
+ if (valueID == CSSValueFlexStart || valueID == CSSValueFlexEnd || valueID == CSSValueCenter || valueID == CSSValueBaseline || valueID == CSSValueStretch)
+ return true;
+ break;
+ case CSSPropertyAlignSelf:
+ // FIXME: Per CSS alignment, this property should accept the same arguments as 'justify-self' so we should share its parsing code.
+ // FIXME: For now, we will do it behind the GRID_LAYOUT compile flag.
+ if (valueID == CSSValueAuto || valueID == CSSValueFlexStart || valueID == CSSValueFlexEnd || valueID == CSSValueCenter || valueID == CSSValueBaseline || valueID == CSSValueStretch)
+ return true;
+ break;
case CSSPropertyFlexDirection:
if (valueID == CSSValueRow || valueID == CSSValueRowReverse || valueID == CSSValueColumn || valueID == CSSValueColumnReverse)
return true;
@@ -1143,6 +1155,9 @@
case CSSPropertyFontVariantCaps:
case CSSPropertyFontVariantAlternates:
return true;
+ case CSSPropertyAlignItems:
+ case CSSPropertyAlignSelf:
+ return !RuntimeEnabledFeatures::sharedFeatures().isCSSGridLayoutEnabled();
default:
return false;
}
@@ -2723,8 +2738,10 @@
parsedValue = parseContentDistributionOverflowPosition();
break;
case CSSPropertyJustifySelf:
+ ASSERT(RuntimeEnabledFeatures::sharedFeatures().isCSSGridLayoutEnabled());
return parseItemPositionOverflowPosition(propId, important);
case CSSPropertyJustifyItems:
+ ASSERT(RuntimeEnabledFeatures::sharedFeatures().isCSSGridLayoutEnabled());
if (parseLegacyPosition(propId, important))
return true;
m_valueList->setCurrentIndex(0);
@@ -3143,9 +3160,11 @@
parsedValue = parseContentDistributionOverflowPosition();
break;
case CSSPropertyAlignSelf:
+ ASSERT(RuntimeEnabledFeatures::sharedFeatures().isCSSGridLayoutEnabled());
return parseItemPositionOverflowPosition(propId, important);
case CSSPropertyAlignItems:
+ ASSERT(RuntimeEnabledFeatures::sharedFeatures().isCSSGridLayoutEnabled());
return parseItemPositionOverflowPosition(propId, important);
case CSSPropertyBorderBottomStyle:
case CSSPropertyBorderCollapse:
Modified: trunk/Source/WebCore/rendering/RenderFlexibleBox.cpp (205101 => 205102)
--- trunk/Source/WebCore/rendering/RenderFlexibleBox.cpp 2016-08-28 09:35:33 UTC (rev 205101)
+++ trunk/Source/WebCore/rendering/RenderFlexibleBox.cpp 2016-08-28 14:45:43 UTC (rev 205102)
@@ -34,6 +34,7 @@
#include "LayoutRepainter.h"
#include "RenderLayer.h"
#include "RenderView.h"
+#include "RuntimeEnabledFeatures.h"
#include <limits>
#include <wtf/MathExtras.h>
@@ -1397,6 +1398,8 @@
// yet for FlexibleBox.
// Defaulting to Stretch for now, as it what most of FlexBox based renders
// expect as default.
+ ASSERT(RuntimeEnabledFeatures::sharedFeatures().isCSSGridLayoutEnabled());
+ FALLTHROUGH;
case ItemPositionStretch: {
applyStretchAlignmentToChild(*child, lineCrossAxisExtent);
// Since wrap-reverse flips cross start and cross end, strech children should be aligned with the cross end.
@@ -1431,6 +1434,8 @@
case ItemPositionRight:
// FIXME: https://webkit.org/b/135460 - The extended grammar is not supported
// yet for FlexibleBox.
+ ASSERT(RuntimeEnabledFeatures::sharedFeatures().isCSSGridLayoutEnabled());
+ break;
default:
ASSERT_NOT_REACHED();
break;
