Changeset 205102 in webkit

Timestamp:

Aug 28, 2016 7:45:43 AM (8 years ago)

Author:

jfernandez@igalia.com

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.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/css3/flexbox/new-alignment-values-invalid-if-grid-not-enabled-expected.txt (added)
• LayoutTests/css3/flexbox/new-alignment-values-invalid-if-grid-not-enabled.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/css/parser/CSSParser.cpp (modified) (4 diffs)
• Source/WebCore/rendering/RenderFlexibleBox.cpp (modified) (3 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2016-08-28  Javier Fernandez  <jfernandez@igalia.com>
+
+        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  <fwang@igalia.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2016-08-28  Javier Fernandez  <jfernandez@igalia.com>
+
+        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  <fwang@igalia.com>
 

trunk/Source/WebCore/css/parser/CSSParser.cpp

@@ -818 +818 @@
     case CSSPropertyColumnFill:
         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;
@@ -1144 +1156 @@
     case CSSPropertyFontVariantAlternates:
         return true;
+    case CSSPropertyAlignItems:
+    case CSSPropertyAlignSelf:
+        return !RuntimeEnabledFeatures::sharedFeatures().isCSSGridLayoutEnabled();
     default:
         return false;
@@ -2724 +2739 @@
         break;
     case CSSPropertyJustifySelf:
+        ASSERT(RuntimeEnabledFeatures::sharedFeatures().isCSSGridLayoutEnabled());
         return parseItemPositionOverflowPosition(propId, important);
     case CSSPropertyJustifyItems:
+        ASSERT(RuntimeEnabledFeatures::sharedFeatures().isCSSGridLayoutEnabled());
         if (parseLegacyPosition(propId, important))
             return true;
@@ -3144 +3161 @@
         break;
     case CSSPropertyAlignSelf:
+        ASSERT(RuntimeEnabledFeatures::sharedFeatures().isCSSGridLayoutEnabled());
         return parseItemPositionOverflowPosition(propId, important);
 
     case CSSPropertyAlignItems:
+        ASSERT(RuntimeEnabledFeatures::sharedFeatures().isCSSGridLayoutEnabled());
         return parseItemPositionOverflowPosition(propId, important);
     case CSSPropertyBorderBottomStyle:

trunk/Source/WebCore/rendering/RenderFlexibleBox.cpp

@@ -35 +35 @@
 #include "RenderLayer.h"
 #include "RenderView.h"
+#include "RuntimeEnabledFeatures.h"
 #include <limits>
 #include <wtf/MathExtras.h>
@@ -1398 +1399 @@
                 // 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);
@@ -1432 +1435 @@
                 // FIXME: https://webkit.org/b/135460 - The extended grammar is not supported
                 // yet for FlexibleBox.
+                ASSERT(RuntimeEnabledFeatures::sharedFeatures().isCSSGridLayoutEnabled());
+                break;
             default:
                 ASSERT_NOT_REACHED();