[CSS Grid Layout] Fix positioned grid children position and size

Source/core/rendering/RenderGrid.cpp

 /*
  * Copyright (C) 2011 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 206 matching lines @@
     RenderBlock::addChild(newChild, beforeChild);
 
     if (gridIsDirty())
         return;
 
     if (!newChild->isBox()) {
         dirtyGrid();
         return;
     }
 
+    // Positioned items that shouldn't take up space or otherwise participate in the layout of the grid.
+    if (newChild->isOutOfFlowPositioned())
+        return;
+
     // If the new child has been inserted inside an existent anonymous block, we can simply ignore it as the anonymous
     // block is an already known grid item.
     if (newChild->parent() != this)
         return;
 
     // FIXME: Implement properly "stack" value in auto-placement algorithm.
     if (!style()->isGridAutoFlowAlgorithmStack()) {
         // The grid needs to be recomputed as it might contain auto-placed items that will change their position.
         dirtyGrid();
         return;
     }
 
     RenderBox* newChildBox = toRenderBox(newChild);
     OwnPtr<GridSpan> rowPositions = GridResolvedPosition::resolveGridPositionsFromStyle(*style(), *newChildBox, ForRows);
     OwnPtr<GridSpan> columnPositions = GridResolvedPosition::resolveGridPositionsFromStyle(*style(), *newChildBox, ForColumns);
     if (!rowPositions || !columnPositions) {
         // The new child requires the auto-placement algorithm to run so we need to recompute the grid fully.
         dirtyGrid();
         return;
     } else {
         insertItemIntoGrid(*newChildBox, GridCoordinate(*rowPositions, *columnPositions));
         addChildToIndexesMap(*newChildBox);
     }
 }
 
 void RenderGrid::addChildToIndexesMap(RenderBox& child)
 {
     ASSERT(!m_gridItemsIndexesMap.contains(&child));
-    RenderBox* sibling = child.nextSiblingBox();
+    RenderBox* sibling;
+    for (sibling = child.nextSiblingBox(); sibling; sibling = sibling->nextSiblingBox()) {

[Julien - ping for review, 2014/11/18 21:12:44]

nextSiblingBox doesn't check the type and IIRC we had quite some issues with bad
casts. Won't that make us more prone to such mistakes?

[Manuel Rego, 2014/11/19 15:18:29]

We solved those issues directly in RenderGrid::addChild().

I don't really know what we can do here as we need to skip the
positioned grid items. Besides, we were already calling nextSiblingBox()
before. Any idea?

[Julien - ping for review, 2014/11/19 18:46:43]

I didn't have a good way forward, only mentioned that as you may have had some
better idea :-)

There is not much we can do AFAICT so let's ignore that.

+        if (!sibling->isOutOfFlowPositioned())
+            break;
+    }
     bool lastSibling = !sibling;
 
-    if (lastSibling)
-        sibling = child.previousSiblingBox();
+    if (lastSibling) {
+        for (sibling = child.previousSiblingBox(); sibling; sibling = sibling->previousSiblingBox()) {
+            if (!sibling->isOutOfFlowPositioned())
+                break;
+        }
+    }
 
     size_t index = 0;
     if (sibling)
         index = lastSibling ? m_gridItemsIndexesMap.get(sibling) + 1 : m_gridItemsIndexesMap.get(sibling);
 
     if (sibling && !lastSibling) {
-        for (; sibling; sibling = sibling->nextSiblingBox())
+        for (; sibling; sibling = sibling->nextSiblingBox()) {
+            if (sibling->isOutOfFlowPositioned())
+                continue;
             m_gridItemsIndexesMap.set(sibling, m_gridItemsIndexesMap.get(sibling) + 1);
+        }
     }
 
     m_gridItemsIndexesMap.set(&child, index);
 }
 
 void RenderGrid::removeChild(RenderObject* child)
 {
     RenderBlock::removeChild(child);
 
     if (gridIsDirty())
         return;
 
     ASSERT(child->isBox());
 
     // FIXME: Implement properly "stack" value in auto-placement algorithm.
     if (!style()->isGridAutoFlowAlgorithmStack()) {
         // The grid needs to be recomputed as it might contain auto-placed items that will change their position.
         dirtyGrid();
         return;
     }
 
+    if (child->isOutOfFlowPositioned())
+        return;
+
     const RenderBox* childBox = toRenderBox(child);
     GridCoordinate coordinate = m_gridItemCoordinate.take(childBox);
 
     for (GridSpan::iterator row = coordinate.rows.begin(); row != coordinate.rows.end(); ++row) {
         for (GridSpan::iterator column = coordinate.columns.begin(); column != coordinate.columns.end(); ++column) {
             GridCell& cell = m_grid[row.toInt()][column.toInt()];
             cell.remove(cell.find(childBox));
         }
     }
 
@@ skipping 592 matching lines @@
 
     populateExplicitGridAndOrderIterator();
 
     // We clear the dirty bit here as the grid sizes have been updated, this means
     // that we can safely call gridRowCount() / gridColumnCount().
     m_gridIsDirty = false;
 
     Vector<RenderBox*> autoMajorAxisAutoGridItems;
     Vector<RenderBox*> specifiedMajorAxisAutoGridItems;
     for (RenderBox* child = m_orderIterator.first(); child; child = m_orderIterator.next()) {
+        if (child->isOutOfFlowPositioned()) {
+            continue;
+        }
+
         // FIXME: We never re-resolve positions if the grid is grown during auto-placement which may lead auto / <integer>
         // positions to not match the author's intent. The specification is unclear on what should be done in this case.
         OwnPtr<GridSpan> rowPositions = GridResolvedPosition::resolveGridPositionsFromStyle(*style(), *child, ForRows);
         OwnPtr<GridSpan> columnPositions = GridResolvedPosition::resolveGridPositionsFromStyle(*style(), *child, ForColumns);
         if (!rowPositions || !columnPositions) {
             GridSpan* majorAxisPositions = (autoPlacementMajorAxisDirection() == ForColumns) ? columnPositions.get() : rowPositions.get();
             if (!majorAxisPositions)
                 autoMajorAxisAutoGridItems.append(child);
             else
                 specifiedMajorAxisAutoGridItems.append(child);
@@ skipping 22 matching lines @@
 void RenderGrid::populateExplicitGridAndOrderIterator()
 {
     OrderIteratorPopulator populator(m_orderIterator);
 
     size_t maximumRowIndex = std::max<size_t>(1, GridResolvedPosition::explicitGridRowCount(*style()));
     size_t maximumColumnIndex = std::max<size_t>(1, GridResolvedPosition::explicitGridColumnCount(*style()));
 
     ASSERT(m_gridItemsIndexesMap.isEmpty());
     size_t childIndex = 0;
     for (RenderBox* child = firstChildBox(); child; child = child->nextSiblingBox()) {
+        if (child->isOutOfFlowPositioned()) {

[Julien - ping for review, 2014/11/18 21:12:44]

FWIW that's the kind of code that makes me advocate for some
no-out-of-flow-positioned-iterator. Most rendering traversals follow this
pattern and those who need it should be investigated.

[Manuel Rego, 2014/11/19 15:18:29]

Yeah, I understand what you mean, in several cases we don't need the
positioned items.

+            continue;
+        }
+
         populator.collectChild(child);
         m_gridItemsIndexesMap.set(child, childIndex++);
 
         // This function bypasses the cache (cachedGridCoordinate()) as it is used to build it.
         OwnPtr<GridSpan> rowPositions = GridResolvedPosition::resolveGridPositionsFromStyle(*style(), *child, ForRows);
         OwnPtr<GridSpan> columnPositions = GridResolvedPosition::resolveGridPositionsFromStyle(*style(), *child, ForColumns);
 
         // |positions| is 0 if we need to run the auto-placement algorithm.
         if (rowPositions) {
             maximumRowIndex = std::max<size_t>(maximumRowIndex, rowPositions->resolvedFinalPosition.next().toInt());
@@ skipping 138 matching lines @@
     computeUsedBreadthOfGridTracks(ForColumns, sizingData);
     ASSERT(tracksAreWiderThanMinTrackBreadth(ForColumns, sizingData.columnTracks));
     computeUsedBreadthOfGridTracks(ForRows, sizingData);
     ASSERT(tracksAreWiderThanMinTrackBreadth(ForRows, sizingData.rowTracks));
 
     populateGridPositions(sizingData);
     m_gridItemsOverflowingGridArea.resize(0);
 
     for (RenderBox* child = firstChildBox(); child; child = child->nextSiblingBox()) {
         if (child->isOutOfFlowPositioned()) {
-            // FIXME: Absolute positioned grid items should have a special
-            // behavior as described in the spec (crbug.com/273898):
-            // http://www.w3.org/TR/css-grid-1/#abspos-items
             child->containingBlock()->insertPositionedObject(child);
+            adjustPositionedGridItem(child);
+            continue;
         }
 
         // Because the grid area cannot be styled, we don't need to adjust
         // the grid breadth to account for 'box-sizing'.
         LayoutUnit oldOverrideContainingBlockContentLogicalWidth = child->hasOverrideContainingBlockLogicalWidth() ? child->overrideContainingBlockContentLogicalWidth() : LayoutUnit();
         LayoutUnit oldOverrideContainingBlockContentLogicalHeight = child->hasOverrideContainingBlockLogicalHeight() ? child->overrideContainingBlockContentLogicalHeight() : LayoutUnit();
 
         LayoutUnit overrideContainingBlockContentLogicalWidth = gridAreaBreadthForChild(*child, ForColumns, sizingData.columnTracks);
         LayoutUnit overrideContainingBlockContentLogicalHeight = gridAreaBreadthForChild(*child, ForRows, sizingData.rowTracks);
 
@@ skipping 23 matching lines @@
     }
 
     for (size_t i = 0; i < sizingData.rowTracks.size(); ++i)
         setLogicalHeight(logicalHeight() + sizingData.rowTracks[i].m_usedBreadth);
 
     // Min / max logical height is handled by the call to updateLogicalHeight in layoutBlock.
 
     setLogicalHeight(logicalHeight() + borderAndPaddingLogicalHeight());
 }
 
+void RenderGrid::adjustPositionedGridItem(RenderBox* child)
+{
+    bool containerHasHorizontalWritingMode = isHorizontalWritingMode();
+    RenderLayer* childLayer = child->layer();
+    childLayer->setStaticInlinePosition(containerHasHorizontalWritingMode ? borderLeft() + paddingLeft() : borderTop() + paddingTop());
+    childLayer->setStaticBlockPosition(containerHasHorizontalWritingMode ? borderTop() + paddingTop() : borderLeft() + paddingLeft());
+}
+
+void RenderGrid::layoutPositionedObjects(bool relayoutChildren, PositionedLayoutBehavior info)
+{
+    TrackedRendererListHashSet* positionedDescendants = positionedObjects();
+    if (!positionedDescendants)
+        return;
+
+    RenderBox* child;

[Julien - ping for review, 2014/11/18 21:12:44]

Why is child defined outside the for-loop?

[Manuel Rego, 2014/11/19 15:18:29]

A mistake, sorry.

+    TrackedRendererListHashSet::iterator end = positionedDescendants->end();
+    for (TrackedRendererListHashSet::iterator it = positionedDescendants->begin(); it != end; ++it) {
+        child = *it;
+
+        if (child->style()->position() == AbsolutePosition) {

[Julien - ping for review, 2014/11/18 21:12:44]

Fixed positioned elements are never handled here. The specification doesn't
account for them either, which is an oversight as some properties make them
containing block for fixed positioned elements (e.g 'transform')

[Manuel Rego, 2014/11/19 15:18:29]

Yeah, I think you're right. I've removed the if and notified CSS WG in
order to update the spec.
In addition, I've added some examples with fixed elements in the tests.

+            LayoutUnit columnBreadth = clientLogicalWidth();
+            LayoutUnit columnOffset = offsetForAbsolutelyPositionedChild(*child, ForColumns, columnBreadth);
+            LayoutUnit rowBreadth = clientLogicalHeight();
+            LayoutUnit rowOffset = offsetForAbsolutelyPositionedChild(*child, ForRows, rowBreadth);
+
+            bool containerHasHorizontalWritingMode = isHorizontalWritingMode();
+            RenderLayer* childLayer = child->layer();
+            childLayer->setStaticInlinePosition(childLayer->staticInlinePosition() + (containerHasHorizontalWritingMode ? columnOffset : rowOffset));
+            childLayer->setStaticBlockPosition(childLayer->staticBlockPosition() + (containerHasHorizontalWritingMode ? rowOffset : columnOffset));
+
+            child->setOverrideContainingBlockContentLogicalWidth(columnBreadth);

[Julien - ping for review, 2014/11/18 21:12:44]

columnBreadth is the client box, which is the padding box. You want to use the
content box here (as the name of the function implies).

[Manuel Rego, 2014/11/19 15:18:29]

It's the padding box of the grid container, and it seems that should be
the containing block of the positioned items. From the spec:
"the containing block will correspond to the padding edges of the grid
container"

[Julien - ping for review, 2014/11/19 18:46:43]

You're totally right about the specification. I meant that
setOverrideContainingBlockContentLogicalWidth expects the containing block's
content box and we are passing in the padding box. I am concern that this
mismatch will hurt us, especially with box-sizing.

+            child->setOverrideContainingBlockContentLogicalHeight(rowBreadth);
+        }
+    }
+
+    RenderBlock::layoutPositionedObjects(relayoutChildren, info);
+}
+
+LayoutUnit RenderGrid::offsetForAbsolutelyPositionedChild(const RenderBox& child, GridTrackSizingDirection direction, LayoutUnit& breadth)
+{
+    LayoutUnit offset = LayoutUnit(0);
+
+    OwnPtr<GridSpan> positions = GridResolvedPosition::resolveGridPositionsFromStyle(*style(), child, direction);
+    if (!positions)
+        return offset;
+
+    if ((direction == ForColumns && !child.style()->gridColumnStart().isAuto()) || (direction == ForRows && !child.style()->gridRowStart().isAuto())) {
+        breadth -= (direction == ForColumns) ? paddingLeft() : paddingTop();

[Julien - ping for review, 2014/11/18 21:12:44]

Columns / rows follow the logical order so those should be paddingLogicalLeft /
paddingLogicalTop. We should also ASSERT that we are not in an orthogonal flow
as the logic would be totally wrong in this case.

[Manuel Rego, 2014/11/19 15:18:29]

Yes, you're totally right.
I've added a new test to check that writing modes behavior is right.

+
+        GridResolvedPosition firstPosition = GridResolvedPosition(0);
+        if (positions->resolvedInitialPosition != firstPosition) {
+            for (GridResolvedPosition position = firstPosition; position < positions->resolvedInitialPosition; ++position) {
+                LayoutUnit trackBreadth = (direction == ForColumns) ? m_columnPositions[position.toInt() + 1] - m_columnPositions[position.toInt()] : m_rowPositions[position.toInt() + 1] - m_rowPositions[position.toInt()];
+                breadth -= trackBreadth;
+                offset += trackBreadth;
+            }
+        }
+    } else if ((direction == ForColumns && !child.style()->gridColumnEnd().isAuto()) || (direction == ForRows && !child.style()->gridRowEnd().isAuto())) {
+        offset -= (direction == ForColumns) ? paddingLeft() : paddingTop();

[Julien - ping for review, 2014/11/18 21:12:44]

Same comment, physical properties vs logical ones.

[Manuel Rego, 2014/11/19 15:18:29]

Done.

+    }
+
+    if ((direction == ForColumns && !child.style()->gridColumnEnd().isAuto()) || (direction == ForRows && !child.style()->gridRowEnd().isAuto())) {
+        breadth -= (direction == ForColumns) ? paddingRight() : paddingBottom();

[Julien - ping for review, 2014/11/18 21:12:44]

Ditto.

[Manuel Rego, 2014/11/19 15:18:29]

Done.

+
+        GridResolvedPosition lastPosition = GridResolvedPosition(direction == ForColumns ? gridColumnCount() : gridRowCount());
+        if (positions->resolvedFinalPosition.next() != lastPosition) {
+            for (GridResolvedPosition position = positions->resolvedFinalPosition.next(); position < lastPosition; ++position) {
+                LayoutUnit trackBreadth = (direction == ForColumns) ? m_columnPositions[position.toInt() + 1] - m_columnPositions[position.toInt()] : m_rowPositions[position.toInt() + 1] - m_rowPositions[position.toInt()];
+                breadth -= trackBreadth;
+            }
+        }
+    }
+
+    return offset;
+}
+
 GridCoordinate RenderGrid::cachedGridCoordinate(const RenderBox& gridItem) const
 {
     ASSERT(m_gridItemCoordinate.contains(&gridItem));
     return m_gridItemCoordinate.get(&gridItem);
 }
 
 LayoutUnit RenderGrid::gridAreaBreadthForChild(const RenderBox& child, GridTrackSizingDirection direction, const Vector<GridTrack>& tracks) const
 {
     const GridCoordinate& coordinate = cachedGridCoordinate(child);
     const GridSpan& span = (direction == ForColumns) ? coordinate.columns : coordinate.rows;
@@ skipping 355 matching lines @@
     if (isOutOfFlowPositioned())
         return "RenderGrid (positioned)";
     if (isAnonymous())
         return "RenderGrid (generated)";
     if (isRelPositioned())
         return "RenderGrid (relative positioned)";
     return "RenderGrid";
 }
 
 } // namespace blink