Changeset 207535 in webkit


Ignore:
Timestamp:
Oct 19, 2016 6:42:43 AM (8 years ago)
Author:
jfernandez@igalia.com
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:
Location:
trunk
Files:
2 added
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r207523 r207535  
     12016-10-19  Javier Fernandez  <jfernandez@igalia.com>
     2
     3        Web Inspector: Debugger buttons positioned incorrectly, align-content default value is unexpected
     4        https://bugs.webkit.org/show_bug.cgi?id=163572
     5
     6        Reviewed by Sergio Villar Senin.
     7
     8        Modified test cases for initial values.
     9        Added regression test for the align-content issue.
     10
     11        * css3/flexbox/flexbox-lines-must-be-stretched-by-default-expected.txt: Added.
     12        * css3/flexbox/flexbox-lines-must-be-stretched-by-default.html: Added.
     13        * fast/css/ensure-flexbox-compatibility-with-initial-values-expected.txt:
     14
    1152016-10-19  Jer Noble  <jer.noble@apple.com>
    216

  • trunk/LayoutTests/fast/css/ensure-flexbox-compatibility-with-initial-values-expected.txt

    r205807 r207535  
    2626PASS CSS.supports('align-items', 'stretch') is true
    2727PASS CSS.supports('align-self', 'stretch') is true
    28 PASS CSS.supports('align-content', 'flex-start') is true
     28PASS CSS.supports('align-content', 'stretch') is true
    2929PASS CSS.supports('justify-content', 'flex-start') is true
    3030PASS CSS.supports('align-items', 'stretch') is true
    3131PASS CSS.supports('align-self', 'stretch') is true
    32 PASS CSS.supports('align-content', 'flex-start') is true
     32PASS CSS.supports('align-content', 'stretch') is true
    3333PASS CSS.supports('justify-content', 'flex-start') is true
    3434PASS CSS.supports('align-items', 'stretch') is true
    3535PASS CSS.supports('align-self', 'stretch') is true
    36 PASS CSS.supports('align-content', 'flex-start') is true
     36PASS CSS.supports('align-content', 'stretch') is true
    3737PASS CSS.supports('justify-content', 'flex-start') is true
    3838PASS CSS.supports('align-items', 'stretch') is true
    3939PASS CSS.supports('align-self', 'stretch') is true
    40 PASS CSS.supports('align-content', 'flex-start') is true
     40PASS CSS.supports('align-content', 'stretch') is true
    4141PASS CSS.supports('justify-content', 'flex-start') is true
    4242PASS successfullyParsed is true

  • trunk/Source/WebCore/ChangeLog

    r207533 r207535  
     12016-10-19  Javier Fernandez  <jfernandez@igalia.com>
     2
     3        Web Inspector: Debugger buttons positioned incorrectly, align-content default value is unexpected
     4        https://bugs.webkit.org/show_bug.cgi?id=163572
     5
     6        Reviewed by Sergio Villar Senin.
     7
     8        We only allow the new CSS Box Alignment syntax when the Grid Layout
     9        feature is enabled. Due to flexbox backward compatibility we have
     10        implemented a different code path for the style initial/default values
     11        assignment. However, we have incorrectly resolved both align-content
     12        and justify-content to 'flex-start' when grid layout is disabled.
     13
     14        This patch changes the approach, so we set 'normal' (the value specified
     15        by the new syntax) for both properties, but using the values defined in
     16        the old syntax (Flexbox specification) at computed style resolution.
     17
     18        Since 'stretch' is the default value for the align-content property, this
     19        issue implies that any flexbox line with an undefined height will be
     20        laid out incorrectly, if not explicitly set via CSS, because flex items
     21        can't use the available height, even though they use 'stretch' for their
     22        'align-self' properties.
     23
     24        Test: css3/flexbox/flexbox-lines-must-be-stretched-by-default.html
     25
     26        * css/CSSComputedStyleDeclaration.cpp:
     27        (WebCore::valueForContentPositionAndDistributionWithOverflowAlignment):
     28        (WebCore::ComputedStyleExtractor::propertyValue):
     29        * rendering/style/RenderStyle.h:
     30        (WebCore::RenderStyle::initialContentAlignment):
     31
    1322016-10-19  Carlos Alberto Lopez Perez  <clopez@igalia.com>
    233

  • trunk/Source/WebCore/css/CSSComputedStyleDeclaration.cpp

    r207499 r207535  
    6363#include "RenderBox.h"
    6464#include "RenderStyle.h"
     65#include "RuntimeEnabledFeatures.h"
    6566#include "SVGElement.h"
    6667#include "Settings.h"
    … …  
    24152416}
    24162417
    2417 static Ref<CSSValueList> valueForContentPositionAndDistributionWithOverflowAlignment(const StyleContentAlignmentData& data)
     2418static Ref<CSSValueList> valueForContentPositionAndDistributionWithOverflowAlignment(const StyleContentAlignmentData& data, CSSValueID normalBehaviorValueID)
    24182419{
    24192420    auto& cssValuePool = CSSValuePool::singleton();
    … …  
    24212422    if (data.distribution() != ContentDistributionDefault)
    24222423        result.get().append(cssValuePool.createValue(data.distribution()));
    2423     if (data.distribution() == ContentDistributionDefault || data.position() != ContentPositionNormal)
    2424         result.get().append(cssValuePool.createValue(data.position()));
     2424    if (data.distribution() == ContentDistributionDefault || data.position() != ContentPositionNormal) {
     2425        bool gridEnabled = false;
     2426#if ENABLE(CSS_GRID_LAYOUT)
     2427        gridEnabled = RuntimeEnabledFeatures::sharedFeatures().isCSSGridLayoutEnabled();
     2428#endif
     2429        if (data.position() != ContentPositionNormal || gridEnabled)
     2430            result.get().append(cssValuePool.createValue(data.position()));
     2431        else
     2432            result.get().append(cssValuePool.createIdentifierValue(normalBehaviorValueID));
     2433    }
    24252434    if ((data.position() >= ContentPositionCenter || data.distribution() != ContentDistributionDefault) && data.overflow() != OverflowAlignmentDefault)
    24262435        result.get().append(cssValuePool.createValue(data.overflow()));
    … …  
    28042813            return cssValuePool.createValue(style->emptyCells());
    28052814        case CSSPropertyAlignContent:
    2806             return valueForContentPositionAndDistributionWithOverflowAlignment(style->alignContent());
     2815            return valueForContentPositionAndDistributionWithOverflowAlignment(style->alignContent(), CSSValueStretch);
    28072816        case CSSPropertyAlignItems:
    28082817            return valueForItemPositionWithOverflowAlignment(style->alignItems());
    … …  
    28242833            return cssValuePool.createValue(style->flexWrap());
    28252834        case CSSPropertyJustifyContent:
    2826             return valueForContentPositionAndDistributionWithOverflowAlignment(style->justifyContent());
     2835            return valueForContentPositionAndDistributionWithOverflowAlignment(style->justifyContent(), CSSValueFlexStart);
    28272836#if ENABLE(CSS_GRID_LAYOUT)
    28282837        case CSSPropertyJustifyItems:

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

    r207499 r207535  
    20052005    static StyleSelfAlignmentData initialSelfAlignment() { return StyleSelfAlignmentData(ItemPositionAuto, OverflowAlignmentDefault); }
    20062006    static StyleSelfAlignmentData initialDefaultAlignment() { return StyleSelfAlignmentData(isCSSGridLayoutEnabled() ? ItemPositionNormal : ItemPositionStretch, OverflowAlignmentDefault); }
    2007     static StyleContentAlignmentData initialContentAlignment() { return StyleContentAlignmentData(isCSSGridLayoutEnabled() ? ContentPositionNormal : ContentPositionFlexStart, ContentDistributionDefault, OverflowAlignmentDefault); }
     2007    static StyleContentAlignmentData initialContentAlignment() { return StyleContentAlignmentData(ContentPositionNormal, ContentDistributionDefault, OverflowAlignmentDefault); }
    20082008    static EFlexDirection initialFlexDirection() { return FlowRow; }
    20092009    static EFlexWrap initialFlexWrap() { return FlexNoWrap; }
Note: See TracChangeset for help on using the changeset viewer.