Changeset 164869 in webkit


Ignore:
Timestamp:
Feb 28, 2014 10:34:48 AM (10 years ago)
Author:
svillar@igalia.com
Message:

[CSS Grid Layout] Fix positioning grid items using named grid lines/areas
​https://bugs.webkit.org/show_bug.cgi?id=129372

Reviewed by Darin Adler.

Source/WebCore:

Our code was assuming that a <custom-ident> in
-webkit-grid-{column|row}-{start|end} and
-webkit-grid-{column|row} was always a grid area name. That's
wrong because the <custom-ident> could be also a explicitly named
grid line or the an implicitly named grid line created by a grid
area definition.

The style resolution code was not correct either. This patch fixes
it so it now matches the spec, which means that:

  • first we try to match any existing grid area.
  • then if there is a named grid line with the name

<custom-ident>-{start|end} for -webkit-grid-{column|row}-{start|end}
defined before the grid area then we use it instead of the grid
area.

  • otherwise if there is a named grid line we resolve to the first such line.
  • otherwise we treat it as 'auto'.

Fixing this uncovered a bug in GridPosition, we were not using the
name of the stored grid area to check if two GridPositions were
the same.

Tests: fast/css-grid-layout/grid-item-position-changed-dynamic.html

fast/css-grid-layout/named-grid-lines-with-named-grid-areas-resolution.html

  • css/StyleResolver.cpp:

(WebCore::gridLineDefinedBeforeGridArea): New function to check if
a given named grid line was defined before an implicit named grid
line created by a grid area definition.
(WebCore::StyleResolver::adjustNamedGridItemPosition): New
function that adjusts the position of a GridPosition parsed as a
grid area.
(WebCore::StyleResolver::adjustGridItemPosition): Use the new
function adjustNamedGridItemPosition to adjust the positions of
named grid lines.

  • css/StyleResolver.h:
  • rendering/RenderGrid.cpp:

(WebCore::RenderGrid::resolveNamedGridLinePositionFromStyle): Use GridPosition:: namespace.
(WebCore::RenderGrid::resolveGridPositionFromStyle): Ditto.
(WebCore::RenderGrid::resolveRowEndColumnEndNamedGridLinePositionAgainstOppositePosition): Ditto.

  • rendering/RenderGrid.h:
  • rendering/style/GridPosition.h:

(WebCore::GridPosition::adjustGridPositionForRowEndColumnEndSide): Moved from RenderGrid.cpp.
(WebCore::GridPosition::adjustGridPositionForSide): Ditto.
(WebCore::GridPosition::operator==): Use the named grid line to check equality.

LayoutTests:

Added a new test that checks that we correctly position grid items
using named grid lines, grid areas and also with the implicit
named grid lines created by grid areas.

I'm also importing a test from Blink that checks that we can
dynamically change the position of a grid item by changing the
name of the grid lines used to position it.

  • fast/css-grid-layout/grid-item-position-changed-dynamic-expected.txt:

Merged from Blink r153913 by <​jchaffraix@chromium.org>.

  • fast/css-grid-layout/grid-item-position-changed-dynamic.html: Ditto.
  • fast/css-grid-layout/named-grid-lines-with-named-grid-areas-resolution-expected.txt: Added.
  • fast/css-grid-layout/named-grid-lines-with-named-grid-areas-resolution.html: Added.
Location:
trunk
Files:
4 added
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r164867 r164869  
     12014-02-27  Sergio Villar Senin  <svillar@igalia.com>
     2
     3        [CSS Grid Layout] Fix positioning grid items using named grid lines/areas
     4        https://bugs.webkit.org/show_bug.cgi?id=129372
     5
     6        Reviewed by Darin Adler.
     7
     8        Added a new test that checks that we correctly position grid items
     9        using named grid lines, grid areas and also with the implicit
     10        named grid lines created by grid areas.
     11
     12        I'm also importing a test from Blink that checks that we can
     13        dynamically change the position of a grid item by changing the
     14        name of the grid lines used to position it.
     15
     16        * fast/css-grid-layout/grid-item-position-changed-dynamic-expected.txt:
     17        Merged from Blink r153913 by <jchaffraix@chromium.org>.
     18        * fast/css-grid-layout/grid-item-position-changed-dynamic.html: Ditto.
     19        * fast/css-grid-layout/named-grid-lines-with-named-grid-areas-resolution-expected.txt: Added.
     20        * fast/css-grid-layout/named-grid-lines-with-named-grid-areas-resolution.html: Added.
     21
    1222014-02-28  Mario Sanchez Prada  <mario.prada@samsung.com>
    223

  • trunk/Source/WebCore/ChangeLog

    r164868 r164869  
     12014-02-27  Sergio Villar Senin  <svillar@igalia.com>
     2
     3        [CSS Grid Layout] Fix positioning grid items using named grid lines/areas
     4        https://bugs.webkit.org/show_bug.cgi?id=129372
     5
     6        Reviewed by Darin Adler.
     7
     8        Our code was assuming that a <custom-ident> in
     9        -webkit-grid-{column|row}-{start|end} and
     10        -webkit-grid-{column|row} was always a grid area name. That's
     11        wrong because the <custom-ident> could be also a explicitly named
     12        grid line or the an implicitly named grid line created by a grid
     13        area definition.
     14
     15        The style resolution code was not correct either. This patch fixes
     16        it so it now matches the spec, which means that:
     17        - first we try to match any existing grid area.
     18        - then if there is a named grid line with the name
     19        <custom-ident>-{start|end} for -webkit-grid-{column|row}-{start|end}
     20        defined before the grid area then we use it instead of the grid
     21        area.
     22        - otherwise if there is a named grid line we resolve to the first such line.
     23        - otherwise we treat it as 'auto'.
     24
     25        Fixing this uncovered a bug in GridPosition, we were not using the
     26        name of the stored grid area to check if two GridPositions were
     27        the same.
     28
     29        Tests: fast/css-grid-layout/grid-item-position-changed-dynamic.html
     30               fast/css-grid-layout/named-grid-lines-with-named-grid-areas-resolution.html
     31
     32        * css/StyleResolver.cpp:
     33        (WebCore::gridLineDefinedBeforeGridArea): New function to check if
     34        a given named grid line was defined before an implicit named grid
     35        line created by a grid area definition.
     36        (WebCore::StyleResolver::adjustNamedGridItemPosition): New
     37        function that adjusts the position of a GridPosition parsed as a
     38        grid area.
     39        (WebCore::StyleResolver::adjustGridItemPosition): Use the new
     40        function adjustNamedGridItemPosition to adjust the positions of
     41        named grid lines.
     42        * css/StyleResolver.h:
     43        * rendering/RenderGrid.cpp:
     44        (WebCore::RenderGrid::resolveNamedGridLinePositionFromStyle): Use GridPosition:: namespace.
     45        (WebCore::RenderGrid::resolveGridPositionFromStyle): Ditto.
     46        (WebCore::RenderGrid::resolveRowEndColumnEndNamedGridLinePositionAgainstOppositePosition): Ditto.
     47        * rendering/RenderGrid.h:
     48        * rendering/style/GridPosition.h:
     49        (WebCore::GridPosition::adjustGridPositionForRowEndColumnEndSide): Moved from RenderGrid.cpp.
     50        (WebCore::GridPosition::adjustGridPositionForSide): Ditto.
     51        (WebCore::GridPosition::operator==): Use the named grid line to check equality.
     52
    1532014-02-28  Zoltan Horvath  <zoltan@webkit.org>
    254

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

    r164795 r164869  
    1010 * Copyright (C) Research In Motion Limited 2011. All rights reserved.
    1111 * Copyright (C) 2012 Google Inc. All rights reserved.
     12 * Copyright (C) 2014 Igalia S.L.
    1213 *
    1314 * This library is free software; you can redistribute it and/or
    … …  
    13771378
    13781379#if ENABLE(CSS_GRID_LAYOUT)
     1380static inline bool gridLineDefinedBeforeGridArea(const String& gridLineName, const String& gridAreaName, const NamedGridAreaMap& gridAreaMap, const NamedGridLinesMap& namedLinesMap, GridPositionSide side)
     1381{
     1382    ASSERT(namedLinesMap.contains(gridLineName));
     1383    // Grid line indexes are inserted in order.
     1384    size_t namedGridLineFirstDefinition = GridPosition::adjustGridPositionForSide(namedLinesMap.get(gridLineName)[0], side);
     1385
     1386    ASSERT(gridAreaMap.contains(gridAreaName));
     1387    const GridCoordinate& gridAreaCoordinates = gridAreaMap.get(gridAreaName);
     1388
     1389    // GridCoordinate refers to tracks while the indexes in namedLinesMap refer to lines, that's why we need to add 1 to
     1390    // the grid coordinate to get the end line index.
     1391    switch (side) {
     1392    case ColumnStartSide:
     1393        return namedGridLineFirstDefinition < gridAreaCoordinates.columns.initialPositionIndex;
     1394    case ColumnEndSide:
     1395        return namedGridLineFirstDefinition < gridAreaCoordinates.columns.finalPositionIndex;
     1396    case RowStartSide:
     1397        return namedGridLineFirstDefinition < gridAreaCoordinates.rows.initialPositionIndex;
     1398    case RowEndSide:
     1399        return namedGridLineFirstDefinition < gridAreaCoordinates.rows.finalPositionIndex;
     1400    }
     1401    ASSERT_NOT_REACHED();
     1402    return false;
     1403}
     1404
     1405std::unique_ptr<GridPosition> StyleResolver::adjustNamedGridItemPosition(const NamedGridAreaMap& gridAreaMap, const NamedGridLinesMap& namedLinesMap, const GridPosition& position, GridPositionSide side) const
     1406{
     1407    ASSERT(position.isNamedGridArea());
     1408    // The StyleBuilder always treats <custom-ident> as a named grid area. We must decide here if they are going to be
     1409    // resolved to either a grid area or a grid line.
     1410
     1411    String namedGridAreaOrGridLine = position.namedGridLine();
     1412    bool hasStartSuffix = namedGridAreaOrGridLine.endsWith("-start");
     1413    bool hasEndSuffix = namedGridAreaOrGridLine.endsWith("-end");
     1414    bool isStartSide = side == ColumnStartSide || side == RowStartSide;
     1415    bool hasStartSuffixForStartSide = hasStartSuffix && isStartSide;
     1416    bool hasEndSuffixForEndSide = hasEndSuffix && !isStartSide;
     1417    size_t suffixLength = hasStartSuffix ? strlen("-start") : strlen("-end");
     1418    String gridAreaName = hasStartSuffixForStartSide || hasEndSuffixForEndSide ? namedGridAreaOrGridLine.substring(0, namedGridAreaOrGridLine.length() - suffixLength) : namedGridAreaOrGridLine;
     1419
     1420    if (gridAreaMap.contains(gridAreaName)) {
     1421        String gridLineName;
     1422        if (isStartSide && !hasStartSuffix)
     1423            gridLineName = namedGridAreaOrGridLine + "-start";
     1424        else if (!isStartSide && !hasEndSuffix)
     1425            gridLineName = namedGridAreaOrGridLine + "-end";
     1426        else
     1427            gridLineName = namedGridAreaOrGridLine;
     1428
     1429        if (namedLinesMap.contains(gridLineName) && gridLineDefinedBeforeGridArea(gridLineName, gridAreaName, gridAreaMap, namedLinesMap, side)) {
     1430            // Use the explicitly defined grid line defined before the grid area instead of the grid area.
     1431            auto adjustedPosition = std::make_unique<GridPosition>();
     1432            adjustedPosition->setExplicitPosition(1, gridLineName);
     1433            return adjustedPosition;
     1434        }
     1435
     1436        if (hasStartSuffixForStartSide || hasEndSuffixForEndSide) {
     1437            // Renderer expects the grid area name instead of the implicit grid line name.
     1438            auto adjustedPosition = std::make_unique<GridPosition>();
     1439            adjustedPosition->setNamedGridArea(gridAreaName);
     1440            return adjustedPosition;
     1441        }
     1442
     1443        return nullptr;
     1444    }
     1445
     1446    if (namedLinesMap.contains(namedGridAreaOrGridLine)) {
     1447        auto adjustedPosition = std::make_unique<GridPosition>();
     1448        adjustedPosition->setExplicitPosition(1, namedGridAreaOrGridLine);
     1449        return adjustedPosition;
     1450    }
     1451
     1452    // We must clear unknown named grid areas
     1453    return std::make_unique<GridPosition>();
     1454}
     1455
    13791456void StyleResolver::adjustGridItemPosition(RenderStyle& style, const RenderStyle& parentStyle) const
    13801457{
    … …  
    13951472    }
    13961473
    1397     // Unknown named grid area compute to 'auto'.
    1398     const NamedGridAreaMap& map = parentStyle.namedGridArea();
    1399 
    1400 #define CLEAR_UNKNOWN_NAMED_AREA(prop, Prop) \
    1401     if (prop.isNamedGridArea() && !map.contains(prop.namedGridLine())) \
    1402         style.setGridItem##Prop(GridPosition());
    1403 
    1404     CLEAR_UNKNOWN_NAMED_AREA(columnStartPosition, ColumnStart);
    1405     CLEAR_UNKNOWN_NAMED_AREA(columnEndPosition, ColumnEnd);
    1406     CLEAR_UNKNOWN_NAMED_AREA(rowStartPosition, RowStart);
    1407     CLEAR_UNKNOWN_NAMED_AREA(rowEndPosition, RowEnd);
     1474    // If the grid position is a single <custom-ident> then the spec mandates us to resolve it following this steps:
     1475    //     * If there is a named grid area called <custom-ident> resolve the position to the area's corresponding edge.
     1476    //         * If a grid area was found with that name, check that there is no <custom-ident>-start or <custom-ident>-end (depending
     1477    // on the css property being defined) specified before the grid area. If that's the case resolve to that grid line.
     1478    //     * Otherwise check if there is a grid line named <custom-ident>.
     1479    //     * Otherwise treat it as auto.
     1480    const NamedGridLinesMap& namedGridColumnLines = parentStyle.namedGridColumnLines();
     1481    const NamedGridLinesMap& namedGridRowLines = parentStyle.namedGridRowLines();
     1482    const NamedGridAreaMap& gridAreaMap = parentStyle.namedGridArea();
     1483
     1484#define ADJUST_GRID_POSITION_MAYBE(position, Prop, namedGridLines, side) \
     1485    if (position.isNamedGridArea()) {                                   \
     1486        std::unique_ptr<GridPosition> adjustedPosition = adjustNamedGridItemPosition(gridAreaMap, namedGridLines, position, side); \
     1487        if (adjustedPosition)                                           \
     1488            style.setGridItem##Prop(*adjustedPosition);                 \
     1489    }
     1490
     1491    ADJUST_GRID_POSITION_MAYBE(columnStartPosition, ColumnStart, namedGridColumnLines, ColumnStartSide);
     1492    ADJUST_GRID_POSITION_MAYBE(columnEndPosition, ColumnEnd, namedGridColumnLines, ColumnEndSide);
     1493    ADJUST_GRID_POSITION_MAYBE(rowStartPosition, RowStart, namedGridRowLines, RowStartSide);
     1494    ADJUST_GRID_POSITION_MAYBE(rowEndPosition, RowEnd, namedGridRowLines, RowEndSide);
    14081495}
    14091496#endif /* ENABLE(CSS_GRID_LAYOUT) */

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

    r164659 r164869  
    297297#if ENABLE(CSS_GRID_LAYOUT)
    298298    void adjustGridItemPosition(RenderStyle& styleToAdjust, const RenderStyle& parentStyle) const;
     299    std::unique_ptr<GridPosition> adjustNamedGridItemPosition(const NamedGridAreaMap&, const NamedGridLinesMap&, const GridPosition&, GridPositionSide) const;
    299300#endif
    300301

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

    r164659 r164869  
    894894}
    895895
    896 inline static size_t adjustGridPositionForRowEndColumnEndSide(size_t resolvedPosition)
    897 {
    898     return resolvedPosition ? resolvedPosition - 1 : 0;
    899 }
    900 
    901 static size_t adjustGridPositionForSide(size_t resolvedPosition, RenderGrid::GridPositionSide side)
    902 {
    903     // An item finishing on the N-th line belongs to the N-1-th cell.
    904     if (side == RenderGrid::ColumnEndSide || side == RenderGrid::RowEndSide)
    905         return adjustGridPositionForRowEndColumnEndSide(resolvedPosition);
    906 
    907     return resolvedPosition;
    908 }
    909 
    910896size_t RenderGrid::resolveNamedGridLinePositionFromStyle(const GridPosition& position, GridPositionSide side) const
    911897{
    … …  
    918904            return 0;
    919905        const size_t lastLine = explicitGridSizeForSide(side);
    920         return adjustGridPositionForSide(lastLine, side);
     906        return GridPosition::adjustGridPositionForSide(lastLine, side);
    921907    }
    922908
    … …  
    926912    else
    927913        namedGridLineIndex = std::max<int>(it->value.size() - abs(position.integerPosition()), 0);
    928     return adjustGridPositionForSide(it->value[namedGridLineIndex], side);
     914    return GridPosition::adjustGridPositionForSide(it->value[namedGridLineIndex], side);
    929915}
    930916
    … …  
    940926        // Handle <integer> explicit position.
    941927        if (position.isPositive())
    942             return adjustGridPositionForSide(position.integerPosition() - 1, side);
     928            return GridPosition::adjustGridPositionForSide(position.integerPosition() - 1, side);
    943929
    944930        size_t resolvedPosition = abs(position.integerPosition()) - 1;
    … …  
    949935            return 0;
    950936
    951         return adjustGridPositionForSide(endOfTrack - resolvedPosition, side);
     937        return GridPosition::adjustGridPositionForSide(endOfTrack - resolvedPosition, side);
    952938    }
    953939    case NamedGridAreaPosition:
    … …  
    10491035
    10501036    size_t gridLineIndex = std::min(gridLines.size() - 1, firstLineAfterOppositePositionIndex + position.spanPosition() - 1);
    1051     size_t resolvedGridLinePosition = adjustGridPositionForRowEndColumnEndSide(gridLines[gridLineIndex]);
     1037    size_t resolvedGridLinePosition = GridPosition::adjustGridPositionForRowEndColumnEndSide(gridLines[gridLineIndex]);
    10521038    if (resolvedGridLinePosition < resolvedOppositePosition)
    10531039        resolvedGridLinePosition = resolvedOppositePosition;

  • trunk/Source/WebCore/rendering/RenderGrid.h

    r164659 r164869  
    5555    virtual bool avoidsFloats() const override { return true; }
    5656    virtual bool canCollapseAnonymousBlockChild() const override { return false; }
    57 
    58     enum GridPositionSide {
    59         ColumnStartSide,
    60         ColumnEndSide,
    61         RowStartSide,
    62         RowEndSide
    63     };
    6457
    6558    const Vector<LayoutUnit>& columnPositions() const { return m_columnPositions; }

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

    r164659 r164869  
    4646};
    4747
     48enum GridPositionSide {
     49    ColumnStartSide,
     50    ColumnEndSide,
     51    RowStartSide,
     52    RowEndSide
     53};
     54
    4855class GridPosition {
    4956public:
    … …  
    5259        , m_integerPosition(0)
    5360    {
     61    }
     62
     63    static inline size_t adjustGridPositionForRowEndColumnEndSide(size_t resolvedPosition)
     64    {
     65        return resolvedPosition ? resolvedPosition - 1 : 0;
     66    }
     67
     68    static size_t adjustGridPositionForSide(size_t resolvedPosition, GridPositionSide side)
     69    {
     70        // An item finishing on the N-th line belongs to the N-1-th cell.
     71        if (side == ColumnEndSide || side == RowEndSide)
     72            return adjustGridPositionForRowEndColumnEndSide(resolvedPosition);
     73
     74        return resolvedPosition;
    5475    }
    5576
    … …  
    104125    bool operator==(const GridPosition& other) const
    105126    {
    106         return m_type == other.m_type && m_integerPosition == other.m_integerPosition;
     127        return m_type == other.m_type && m_integerPosition == other.m_integerPosition && m_namedGridLine == other.m_namedGridLine;
    107128    }
    108129
Note: See TracChangeset for help on using the changeset viewer.