Changeset 205807 in webkit


Ignore:
Timestamp:
Sep 12, 2016, 4:46:57 AM (8 years ago)
Author:
jfernandez@igalia.com
Message:

[css-align] Initial values are parsed as invalid for some Alignment properties
​https://bugs.webkit.org/show_bug.cgi?id=161303

Reviewed by Darin Adler.

Source/WebCore:

Due to the implementation of the new CSS Box Alignment specification,
some properties have now new values allowed, which are not valid
according to the Flexible Box Layout specification.

In r205102 we have get back the keywordID parsing, originally implemented for
the Flexbible Box Layout specification. Even though the new valued would be
parsed as invalid when they are set, the 'initial' values will be assigned
in any case.

This patch verifies that the 'initial' values depend on whether the Grid
Layout is enabled or not and verifying such values are parsed as valid.

Additionally, it gets back as well they keywordID parsing for the Content
Alignment properties (align-content and justify-content). This required to
touch a bit the StyleBuilderConverter logic, since we will have to deal with
either the complex CSSContentDistributionValue complex or the simpler
CSSPrimitiveValue.

Test: fast/css/ensure-flexbox-compatibility-with-initial-values.html

  • css/StyleBuilderConverter.h:

(WebCore::StyleBuilderConverter::convertContentAlignmentData): Handling a primitive value if Grid Layout is not enabled.

  • css/parser/CSSParser.cpp:

(WebCore::isValidKeywordPropertyAndValue): Simpler parsing of alignment properties if Grid Layout is not enabled.
(WebCore::isKeywordPropertyID): Alignment properties are defined as keyword if Grid Layout is no enabled.
(WebCore::CSSParser::parseValue): Assert Grid Layout is enabled when using the complex parsing.

  • rendering/style/RenderStyle.cpp:

(WebCore::RenderStyle::isCSSGridLayoutEnabled): Checking out the Grid Layout runtime flags.

  • rendering/style/RenderStyle.h:

(WebCore::RenderStyle::initialDefaultAlignment): Initial value will depend on whether Grid Layout is enabled or not.
(WebCore::RenderStyle::initialContentAlignment): Initial value will depend on whether Grid Layout is enabled or not.

LayoutTests:

Test to verify the "initial" values of the CSS Box Alignment properties
are parsed as valid independently of whether Grid Layout is enabled or not.

  • fast/css/ensure-flexbox-compatibility-with-initial-values-expected.txt: Added.
  • fast/css/ensure-flexbox-compatibility-with-initial-values.html: Added.
  • fast/css/resources/alignment-parsing-utils.js:

(checkSupportedValues):

Location:
trunk
Files:
2 added
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r205791 r205807  
     12016-09-12  Javier Fernandez  <jfernandez@igalia.com>
     2
     3        [css-align] Initial values are parsed as invalid for some Alignment properties
     4        https://bugs.webkit.org/show_bug.cgi?id=161303
     5
     6        Reviewed by Darin Adler.
     7
     8        Test to verify the "initial" values of the CSS Box Alignment properties
     9        are parsed as valid independently of whether Grid Layout is enabled or not.
     10
     11        * fast/css/ensure-flexbox-compatibility-with-initial-values-expected.txt: Added.
     12        * fast/css/ensure-flexbox-compatibility-with-initial-values.html: Added.
     13        * fast/css/resources/alignment-parsing-utils.js:
     14        (checkSupportedValues):
     15
    1162016-09-11  Chris Dumez  <cdumez@apple.com>
    217

  • trunk/LayoutTests/fast/css/resources/alignment-parsing-utils.js

    r201498 r205807  
    4747    checkValues(element, property, propertyID, "", value);
    4848}
     49
     50function checkSupportedValues(elementID, property)
     51{
     52    var value = eval("window.getComputedStyle(" + elementID + " , '').getPropertyValue('" + property + "')");
     53    shouldBeTrue("CSS.supports('" + property + "', '" + value + "')");
     54}

  • trunk/Source/WebCore/ChangeLog

    r205806 r205807  
     12016-09-12  Javier Fernandez  <jfernandez@igalia.com>
     2
     3        [css-align] Initial values are parsed as invalid for some Alignment properties
     4        https://bugs.webkit.org/show_bug.cgi?id=161303
     5
     6        Reviewed by Darin Adler.
     7
     8        Due to the implementation of the new CSS Box Alignment specification,
     9        some properties have now new values allowed, which are not valid
     10        according to the Flexible Box Layout specification.
     11
     12        In r205102 we have get back the keywordID parsing, originally implemented for
     13        the Flexbible Box Layout specification. Even though the new valued would be
     14        parsed as invalid when they are set, the 'initial' values will be assigned
     15        in any case.
     16
     17        This patch verifies that the 'initial' values depend on whether the Grid
     18        Layout is enabled or not and verifying such values are parsed as valid.
     19
     20        Additionally, it gets back as well they keywordID parsing for the Content
     21        Alignment properties (align-content and justify-content). This required to
     22        touch a bit the StyleBuilderConverter logic, since we will have to deal with
     23        either the complex CSSContentDistributionValue complex or the  simpler
     24        CSSPrimitiveValue.
     25
     26        Test: fast/css/ensure-flexbox-compatibility-with-initial-values.html
     27
     28        * css/StyleBuilderConverter.h:
     29        (WebCore::StyleBuilderConverter::convertContentAlignmentData): Handling a primitive value if Grid Layout is not enabled.
     30        * css/parser/CSSParser.cpp:
     31        (WebCore::isValidKeywordPropertyAndValue): Simpler parsing of alignment properties if Grid Layout is not enabled.
     32        (WebCore::isKeywordPropertyID): Alignment properties are defined as keyword if Grid Layout is no enabled.
     33        (WebCore::CSSParser::parseValue): Assert Grid Layout is enabled when using the complex parsing.
     34        * rendering/style/RenderStyle.cpp:
     35        (WebCore::RenderStyle::isCSSGridLayoutEnabled): Checking out the Grid Layout runtime flags.
     36        * rendering/style/RenderStyle.h:
     37        (WebCore::RenderStyle::initialDefaultAlignment): Initial value will depend on whether Grid Layout is enabled or not.
     38        (WebCore::RenderStyle::initialContentAlignment): Initial value will depend on whether Grid Layout is enabled or not.
     39
    1402016-09-12  Chris Dumez  <cdumez@apple.com>
    241

  • trunk/Source/WebCore/css/StyleBuilderConverter.h

    r205421 r205807  
    4747#include "Pair.h"
    4848#include "QuotesData.h"
     49#include "RuntimeEnabledFeatures.h"
    4950#include "SVGURIReference.h"
    5051#include "Settings.h"
    … …  
    12541255{
    12551256    StyleContentAlignmentData alignmentData = RenderStyle::initialContentAlignment();
    1256     auto& contentValue = downcast<CSSContentDistributionValue>(value);
    1257     if (contentValue.distribution()->getValueID() != CSSValueInvalid)
    1258         alignmentData.setDistribution(contentValue.distribution().get());
    1259     if (contentValue.position()->getValueID() != CSSValueInvalid)
    1260         alignmentData.setPosition(contentValue.position().get());
    1261     if (contentValue.overflow()->getValueID() != CSSValueInvalid)
    1262         alignmentData.setOverflow(contentValue.overflow().get());
     1257#if ENABLE(CSS_GRID_LAYOUT)
     1258    if (RuntimeEnabledFeatures::sharedFeatures().isCSSGridLayoutEnabled()) {
     1259        auto& contentValue = downcast<CSSContentDistributionValue>(value);
     1260        if (contentValue.distribution()->getValueID() != CSSValueInvalid)
     1261            alignmentData.setDistribution(contentValue.distribution().get());
     1262        if (contentValue.position()->getValueID() != CSSValueInvalid)
     1263            alignmentData.setPosition(contentValue.position().get());
     1264        if (contentValue.overflow()->getValueID() != CSSValueInvalid)
     1265            alignmentData.setOverflow(contentValue.overflow().get());
     1266        return alignmentData;
     1267    }
     1268#endif
     1269    auto& primitiveValue = downcast<CSSPrimitiveValue>(value);
     1270    switch (primitiveValue.getValueID()) {
     1271    case CSSValueStretch:
     1272    case CSSValueSpaceBetween:
     1273    case CSSValueSpaceAround:
     1274        alignmentData.setDistribution(primitiveValue);
     1275        break;
     1276    case CSSValueFlexStart:
     1277    case CSSValueFlexEnd:
     1278    case CSSValueCenter:
     1279        alignmentData.setPosition(primitiveValue);
     1280        break;
     1281    default:
     1282        ASSERT_NOT_REACHED();
     1283    }
    12631284    return alignmentData;
    12641285}

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

    r205787 r205807  
    820820            return true;
    821821        break;
     822    case CSSPropertyAlignContent:
     823        // FIXME: Per CSS alignment, this property should accept an optional <overflow-position>. We should share this parsing code with 'justify-self'.
     824        // FIXME: For now, we will do it behind the GRID_LAYOUT compile flag.
     825        return valueID == CSSValueFlexStart || valueID == CSSValueFlexEnd || valueID == CSSValueCenter || valueID == CSSValueSpaceBetween || valueID == CSSValueSpaceAround || valueID == CSSValueStretch;
    822826    case CSSPropertyAlignItems:
    823827        // FIXME: Per CSS alignment, this property should accept the same arguments as 'justify-self' so we should share its parsing code.
    … …  
    840844             return true;
    841845        break;
     846    case CSSPropertyJustifyContent:
     847        // FIXME: Per CSS alignment, this property should accept an optional <overflow-position>. We should share this parsing code with 'justify-self'.
     848        // FIXME: For now, we will do it behind the GRID_LAYOUT compile flag.
     849        return valueID == CSSValueFlexStart || valueID == CSSValueFlexEnd || valueID == CSSValueCenter || valueID == CSSValueSpaceBetween || valueID == CSSValueSpaceAround;
    842850    case CSSPropertyWebkitFontKerning:
    843851        if (valueID == CSSValueAuto || valueID == CSSValueNormal || valueID == CSSValueNone)
    … …  
    11561164    case CSSPropertyFontVariantAlternates:
    11571165        return true;
     1166    case CSSPropertyJustifyContent:
     1167    case CSSPropertyAlignContent:
    11581168    case CSSPropertyAlignItems:
    11591169    case CSSPropertyAlignSelf:
    … …  
    27392749        return false;
    27402750    }
     2751#if ENABLE(CSS_GRID_LAYOUT)
    27412752    case CSSPropertyJustifyContent:
     2753        ASSERT(RuntimeEnabledFeatures::sharedFeatures().isCSSGridLayoutEnabled());
    27422754        parsedValue = parseContentDistributionOverflowPosition();
    27432755        break;
    2744 #if ENABLE(CSS_GRID_LAYOUT)
    27452756    case CSSPropertyJustifySelf:
    27462757        if (!isCSSGridLayoutEnabled())
    … …  
    31633174        break;
    31643175#endif
     3176#if ENABLE(CSS_GRID_LAYOUT)
    31653177    case CSSPropertyAlignContent:
     3178        ASSERT(RuntimeEnabledFeatures::sharedFeatures().isCSSGridLayoutEnabled());
    31663179        parsedValue = parseContentDistributionOverflowPosition();
    31673180        break;
    3168 #if ENABLE(CSS_GRID_LAYOUT)
    31693181    case CSSPropertyAlignSelf:
    31703182        ASSERT(RuntimeEnabledFeatures::sharedFeatures().isCSSGridLayoutEnabled());

  • trunk/Source/WebCore/rendering/RenderFlexibleBox.cpp

    r205102 r205807  
    13991399                // Defaulting to Stretch for now, as it what most of FlexBox based renders
    14001400                // expect as default.
     1401#if ENABLE(CSS_GRID_LAYOUT)
    14011402                ASSERT(RuntimeEnabledFeatures::sharedFeatures().isCSSGridLayoutEnabled());
     1403#else
     1404                ASSERT_NOT_REACHED();
     1405#endif
    14021406                FALLTHROUGH;
    14031407            case ItemPositionStretch: {
    … …  
    14351439                // FIXME: https://webkit.org/b/135460 - The extended grammar is not supported
    14361440                // yet for FlexibleBox.
     1441#if ENABLE(CSS_GRID_LAYOUT)
    14371442                ASSERT(RuntimeEnabledFeatures::sharedFeatures().isCSSGridLayoutEnabled());
     1443#else
     1444                ASSERT_NOT_REACHED();
     1445#endif
    14381446                break;
    14391447            default:

  • trunk/Source/WebCore/rendering/style/RenderStyle.cpp

    r203250 r205807  
    3838#include "RenderObject.h"
    3939#include "RenderTheme.h"
     40#include "RuntimeEnabledFeatures.h"
    4041#include "ScaleTransformOperation.h"
    4142#include "ShadowData.h"
    … …  
    188189    ASSERT_WITH_SECURITY_IMPLICATION(!m_deletionHasBegun);
    189190    m_deletionHasBegun = true;
     191#endif
     192}
     193
     194bool RenderStyle::isCSSGridLayoutEnabled()
     195{
     196#if ENABLE(CSS_GRID_LAYOUT)
     197    return RuntimeEnabledFeatures::sharedFeatures().isCSSGridLayoutEnabled();
     198#else
     199    return false;
    190200#endif
    191201}

  • trunk/Source/WebCore/rendering/style/RenderStyle.h

    r204717 r205807  
    20022002    static int initialOrder() { return 0; }
    20032003    static StyleSelfAlignmentData initialSelfAlignment() { return StyleSelfAlignmentData(ItemPositionAuto, OverflowAlignmentDefault); }
    2004     static StyleSelfAlignmentData initialDefaultAlignment() { return StyleSelfAlignmentData(ItemPositionNormal, OverflowAlignmentDefault); }
    2005     static StyleContentAlignmentData initialContentAlignment() { return StyleContentAlignmentData(ContentPositionNormal, ContentDistributionDefault, OverflowAlignmentDefault); }
     2004    static StyleSelfAlignmentData initialDefaultAlignment() { return StyleSelfAlignmentData(isCSSGridLayoutEnabled() ? ItemPositionNormal : ItemPositionStretch, OverflowAlignmentDefault); }
     2005    static StyleContentAlignmentData initialContentAlignment() { return StyleContentAlignmentData(isCSSGridLayoutEnabled() ? ContentPositionNormal : ContentPositionFlexStart, ContentDistributionDefault, OverflowAlignmentDefault); }
    20062006    static EFlexDirection initialFlexDirection() { return FlowRow; }
    20072007    static EFlexWrap initialFlexWrap() { return FlexNoWrap; }
    … …  
    20672067    static const AtomicString& initialContentAltText() { return emptyAtom; }
    20682068
     2069    static bool isCSSGridLayoutEnabled();
     2070
    20692071    static WillChangeData* initialWillChange() { return nullptr; }
    20702072
Note: See TracChangeset for help on using the changeset viewer.