- Revision
- 154731
- Author
- [email protected]
- Date
- 2013-08-28 03:26:10 -0700 (Wed, 28 Aug 2013)
Log Message
[CSS Grid Layout] Fix grid position resolution
https://bugs.webkit.org/show_bug.cgi?id=119801
Reviewed by Andreas Kling.
>From Blink r148833, r148878, r150403 by <[email protected]>
Source/WebCore:
Both grid-{column|row}-end and negative positions were not
properly handled in our grid position resolution code. We were
using the same code to resolve all the grid positions without
considering the edges of the grid.
Also refactored the grid size estimation in
resolveGridPositionsFromStyle() so we can use it for the grid size
estimation. The code no longer requires the grid to be filled at
that moment as the specs changed to use the "explicit grid" which
is independent of grid items (only depends on style).
Test: fast/css-grid-layout/grid-item-negative-position-resolution.html
* rendering/RenderGrid.cpp:
(WebCore::RenderGrid::maximumIndexInDirection):
(WebCore::RenderGrid::resolveGridPositionsFromStyle):
(WebCore::adjustGridPositionForSide):
(WebCore::RenderGrid::resolveGridPositionFromStyle):
* rendering/RenderGrid.h:
LayoutTests:
Added a new test to check negative position resolution. Also added
several new test cases to check that we properly resolve grid
positions in the grid edges.
* fast/css-grid-layout/grid-item-negative-position-resolution-expected.txt: Added.
* fast/css-grid-layout/grid-item-negative-position-resolution.html: Added.
* fast/css-grid-layout/grid-item-spanning-resolution-expected.txt:
* fast/css-grid-layout/grid-item-spanning-resolution.html:
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (154730 => 154731)
--- trunk/LayoutTests/ChangeLog 2013-08-28 09:58:33 UTC (rev 154730)
+++ trunk/LayoutTests/ChangeLog 2013-08-28 10:26:10 UTC (rev 154731)
@@ -1,5 +1,23 @@
2013-08-28 Sergio Villar Senin <[email protected]>
+ [CSS Grid Layout] Fix grid position resolution
+ https://bugs.webkit.org/show_bug.cgi?id=119801
+
+ Reviewed by Andreas Kling.
+
+ From Blink r148833, r148878, r150403 by <[email protected]>
+
+ Added a new test to check negative position resolution. Also added
+ several new test cases to check that we properly resolve grid
+ positions in the grid edges.
+
+ * fast/css-grid-layout/grid-item-negative-position-resolution-expected.txt: Added.
+ * fast/css-grid-layout/grid-item-negative-position-resolution.html: Added.
+ * fast/css-grid-layout/grid-item-spanning-resolution-expected.txt:
+ * fast/css-grid-layout/grid-item-spanning-resolution.html:
+
+2013-08-28 Sergio Villar Senin <[email protected]>
+
[CSS Grid Layout] infinity should be defined as a negative value
https://bugs.webkit.org/show_bug.cgi?id=107053
Added: trunk/LayoutTests/fast/css-grid-layout/grid-item-negative-position-resolution-expected.txt (0 => 154731)
--- trunk/LayoutTests/fast/css-grid-layout/grid-item-negative-position-resolution-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/css-grid-layout/grid-item-negative-position-resolution-expected.txt 2013-08-28 10:26:10 UTC (rev 154731)
@@ -0,0 +1,6 @@
+Test that negative grid positions are correctly resolved.
+
+PASS
+PASS
+PASS
+PASS
Property changes on: trunk/LayoutTests/fast/css-grid-layout/grid-item-negative-position-resolution-expected.txt
___________________________________________________________________
Added: svn:eol-style
Added: trunk/LayoutTests/fast/css-grid-layout/grid-item-negative-position-resolution.html (0 => 154731)
--- trunk/LayoutTests/fast/css-grid-layout/grid-item-negative-position-resolution.html (rev 0)
+++ trunk/LayoutTests/fast/css-grid-layout/grid-item-negative-position-resolution.html 2013-08-28 10:26:10 UTC (rev 154731)
@@ -0,0 +1,71 @@
+<!DOCTYPE html>
+<html>
+<script>
+if (window.testRunner)
+ testRunner.overridePreference("WebKitCSSGridLayoutEnabled", 1);
+</script>
+<link href="" rel="stylesheet">
+<style>
+.grid {
+ -webkit-grid-definition-columns: 50px 100px;
+ -webkit-grid-definition-rows: 50px 100px;
+ /* To detect how much we extend the grid. */
+ -webkit-grid-auto-columns: 200px;
+ -webkit-grid-auto-rows: 200px;
+
+ /* Make the grid shrink-to-fit. */
+ position: absolute;
+}
+
+.negativeStartPositionGrowGridInColumnDirection {
+ -webkit-grid-column: -1 / auto;
+ -webkit-grid-row: 1;
+}
+
+.negativeStartPositionGrowGridInRowDirection {
+ -webkit-grid-column: 1;
+ -webkit-grid-row: -1 / auto;
+}
+
+.negativeEndPositionStartNegativeInColumnDirection {
+ -webkit-grid-column: -3 / -1;
+ -webkit-grid-row: 1;
+}
+
+.negativeEndPositionStartNegativeInRowDirection {
+ -webkit-grid-column: -5 / -2;
+ -webkit-grid-row: 1;
+}
+</style>
+<script src=""
+<body _onload_="checkLayout('.grid');">
+
+<p>Test that negative grid positions are correctly resolved.</p>
+
+<div style="position: relative">
+<div class="grid" data-expected-width="350" data-expected-height="150">
+ <div class="sizedToGridArea negativeStartPositionGrowGridInColumnDirection" data-offset-x="150" data-offset-y="0" data-expected-width="200" data-expected-height="50"></div>
+</div>
+</div>
+
+<div style="position: relative">
+<div class="grid" data-expected-width="150" data-expected-height="350">
+ <div class="sizedToGridArea negativeStartPositionGrowGridInRowDirection" data-offset-x="0" data-offset-y="150" data-expected-width="50" data-expected-height="200"></div>
+</div>
+</div>
+
+<div style="position: relative">
+<div class="grid" data-expected-width="150" data-expected-height="150">
+ <div class="sizedToGridArea negativeEndPositionStartNegativeInColumnDirection" data-offset-x="0" data-offset-y="0" data-expected-width="150" data-expected-height="50"></div>
+</div>
+</div>
+
+<div style="position: relative">
+<div class="grid" data-expected-width="150" data-expected-height="150">
+ <div class="sizedToGridArea negativeEndPositionStartNegativeInRowDirection" data-offset-x="0" data-offset-y="0" data-expected-width="50" data-expected-height="50"></div>
+</div>
+</div>
+
+
+</body>
+</html>
Property changes on: trunk/LayoutTests/fast/css-grid-layout/grid-item-negative-position-resolution.html
___________________________________________________________________
Added: svn:mime-type
Added: svn:eol-style
Modified: trunk/LayoutTests/fast/css-grid-layout/grid-item-spanning-resolution-expected.txt (154730 => 154731)
--- trunk/LayoutTests/fast/css-grid-layout/grid-item-spanning-resolution-expected.txt 2013-08-28 09:58:33 UTC (rev 154730)
+++ trunk/LayoutTests/fast/css-grid-layout/grid-item-spanning-resolution-expected.txt 2013-08-28 10:26:10 UTC (rev 154731)
@@ -11,3 +11,4 @@
PASS
PASS
PASS
+PASS
Modified: trunk/LayoutTests/fast/css-grid-layout/grid-item-spanning-resolution.html (154730 => 154731)
--- trunk/LayoutTests/fast/css-grid-layout/grid-item-spanning-resolution.html 2013-08-28 09:58:33 UTC (rev 154730)
+++ trunk/LayoutTests/fast/css-grid-layout/grid-item-spanning-resolution.html 2013-08-28 10:26:10 UTC (rev 154731)
@@ -13,6 +13,13 @@
height: 300px;
}
+#bigGrid {
+ -webkit-grid-definition-columns: 25% 25% 25% 25%;
+ -webkit-grid-definition-rows: 25% 25% 25% 25%;
+ height: 100px;
+ width: 200px;
+}
+
.negativeOverflowRowFirstColumn {
-webkit-grid-row: 1 / -5;
-webkit-grid-column: 1;
@@ -32,6 +39,16 @@
-webkit-grid-row: 1;
-webkit-grid-column: 1 / 5;
}
+
+.secondRowSecondColumnNoSpan {
+ -webkit-grid-column: 2 / 3;
+ -webkit-grid-row: 2 / 3;
+}
+
+.thirdRowThirdColumnNoSpan {
+ -webkit-grid-column: 3 / 4;
+ -webkit-grid-row: 3 / 4;
+}
</style>
<script src=""
<body _onload_="checkLayout('.grid');">
@@ -100,9 +117,14 @@
<div style="position: relative">
<div class="grid" data-expected-width="400" data-expected-height="300">
- <div class="sizedToGridArea autoSecondRowAutoFirstColumn" data-offset-x="0" data-offset-y="90" data-expected-width="160" data-expected-height="210"></div>
+ <div class="sizedToGridArea autoSecondRowAutoFirstColumn" data-offset-x="0" data-offset-y="0" data-expected-width="160" data-expected-height="90"></div>
</div>
</div>
+<div style="position: relative">
+<div class="grid" id="bigGrid" data-expected-width="200" data-expected-height="100">
+ <div class="sizedToGridArea secondRowSecondColumnNoSpan" data-offset-x="50" data-offset-y="25" data-expected-width="50" data-expected-height="25"></div>
+ <div class="sizedToGridArea thirdRowThirdColumnNoSpan" data-offset-x="100" data-offset-y="50" data-expected-width="50" data-expected-height="25"></div>
+</div>
</body>
</html>
Modified: trunk/Source/WebCore/ChangeLog (154730 => 154731)
--- trunk/Source/WebCore/ChangeLog 2013-08-28 09:58:33 UTC (rev 154730)
+++ trunk/Source/WebCore/ChangeLog 2013-08-28 10:26:10 UTC (rev 154731)
@@ -1,5 +1,34 @@
2013-08-28 Sergio Villar Senin <[email protected]>
+ [CSS Grid Layout] Fix grid position resolution
+ https://bugs.webkit.org/show_bug.cgi?id=119801
+
+ Reviewed by Andreas Kling.
+
+ From Blink r148833, r148878, r150403 by <[email protected]>
+
+ Both grid-{column|row}-end and negative positions were not
+ properly handled in our grid position resolution code. We were
+ using the same code to resolve all the grid positions without
+ considering the edges of the grid.
+
+ Also refactored the grid size estimation in
+ resolveGridPositionsFromStyle() so we can use it for the grid size
+ estimation. The code no longer requires the grid to be filled at
+ that moment as the specs changed to use the "explicit grid" which
+ is independent of grid items (only depends on style).
+
+ Test: fast/css-grid-layout/grid-item-negative-position-resolution.html
+
+ * rendering/RenderGrid.cpp:
+ (WebCore::RenderGrid::maximumIndexInDirection):
+ (WebCore::RenderGrid::resolveGridPositionsFromStyle):
+ (WebCore::adjustGridPositionForSide):
+ (WebCore::RenderGrid::resolveGridPositionFromStyle):
+ * rendering/RenderGrid.h:
+
+2013-08-28 Sergio Villar Senin <[email protected]>
+
[CSS Grid Layout] infinity should be defined as a negative value
https://bugs.webkit.org/show_bug.cgi?id=107053
Modified: trunk/Source/WebCore/rendering/RenderGrid.cpp (154730 => 154731)
--- trunk/Source/WebCore/rendering/RenderGrid.cpp 2013-08-28 09:58:33 UTC (rev 154730)
+++ trunk/Source/WebCore/rendering/RenderGrid.cpp 2013-08-28 10:26:10 UTC (rev 154731)
@@ -327,16 +327,6 @@
return trackStyles[i];
}
-static size_t estimatedGridSizeForPosition(const GridPosition& position)
-{
- if (position.isAuto())
- return 1;
-
- // Negative explicit values never grow the grid as they are clamped against
- // the explicit grid's size. Thus we don't special case them here.
- return std::max(position.integerPosition(), 1);
-}
-
size_t RenderGrid::explicitGridColumnCount() const
{
return style()->gridColumns().size();
@@ -352,19 +342,14 @@
size_t maximumIndex = std::max<size_t>(1, (direction == ForColumns) ? explicitGridColumnCount() : explicitGridRowCount());
for (RenderBox* child = firstChildBox(); child; child = child->nextSiblingBox()) {
- // This function bypasses the cache (cachedGridCoordinate()) as it is used to build it.
- // Also we can't call resolveGridPositionsFromStyle here as it assumes that the grid is build and we are in
- // the middle of building it. However we should be able to share more code with the previous logic (FIXME).
- const GridPosition& initialPosition = (direction == ForColumns) ? child->style()->gridItemColumnStart() : child->style()->gridItemRowStart();
- const GridPosition& finalPosition = (direction == ForColumns) ? child->style()->gridItemColumnEnd() : child->style()->gridItemRowEnd();
+ OwnPtr<GridSpan> positions = resolveGridPositionsFromStyle(child, direction);
- size_t estimatedSizeForInitialPosition = estimatedGridSizeForPosition(initialPosition);
- size_t estimatedSizeForFinalPosition = estimatedGridSizeForPosition(finalPosition);
- ASSERT(estimatedSizeForInitialPosition);
- ASSERT(estimatedSizeForFinalPosition);
+ // |positions| is null if we need to run the auto-placement algorithm. Our estimation ignores
+ // this case as the auto-placement algorithm will grow the grid as needed.
+ if (!positions)
+ continue;
- maximumIndex = std::max(maximumIndex, estimatedSizeForInitialPosition);
- maximumIndex = std::max(maximumIndex, estimatedSizeForFinalPosition);
+ maximumIndex = std::max(maximumIndex, positions->finalPositionIndex + 1);
}
return maximumIndex;
@@ -730,8 +715,6 @@
PassOwnPtr<RenderGrid::GridSpan> RenderGrid::resolveGridPositionsFromStyle(const RenderBox* gridItem, TrackSizingDirection direction) const
{
- ASSERT(gridWasPopulated());
-
const GridPosition& initialPosition = (direction == ForColumns) ? gridItem->style()->gridItemColumnStart() : gridItem->style()->gridItemRowStart();
const GridPositionSide initialPositionSide = (direction == ForColumns) ? ColumnStartSide : RowStartSide;
const GridPosition& finalPosition = (direction == ForColumns) ? gridItem->style()->gridItemColumnEnd() : gridItem->style()->gridItemRowEnd();
@@ -767,26 +750,32 @@
return adoptPtr(new GridSpan(resolvedInitialPosition, resolvedFinalPosition));
}
-size_t RenderGrid::resolveGridPositionFromStyle(const GridPosition& position, GridPositionSide side) const
+static size_t adjustGridPositionForSide(size_t resolvedPosition, RenderGrid::GridPositionSide side)
{
- ASSERT(gridWasPopulated());
+ // An item finishing on the N-th line belongs to the N-1-th cell.
+ if (side == RenderGrid::ColumnEndSide || side == RenderGrid::RowEndSide)
+ return resolvedPosition ? resolvedPosition - 1 : 0;
+ return resolvedPosition;
+}
+
+size_t RenderGrid::resolveGridPositionFromStyle(const GridPosition& position, GridPositionSide side) const
+{
// FIXME: Handle other values for grid-{row,column} like ranges or line names.
switch (position.type()) {
case IntegerPosition: {
+ ASSERT(position.integerPosition());
if (position.isPositive())
- return position.integerPosition() - 1;
+ return adjustGridPositionForSide(position.integerPosition() - 1, side);
- size_t resolvedPosition = abs(position.integerPosition());
- // FIXME: This returns one less than the expected result for side == ColumnStartSide or RowStartSide as we don't properly convert
- // the grid line to its grid track. However this avoids the issue of growing the grid when inserting the item (e.g. -1 / auto).
+ size_t resolvedPosition = abs(position.integerPosition()) - 1;
const size_t endOfTrack = (side == ColumnStartSide || side == ColumnEndSide) ? explicitGridColumnCount() : explicitGridRowCount();
// Per http://lists.w3.org/Archives/Public/www-style/2013Mar/0589.html, we clamp negative value to the first line.
if (endOfTrack < resolvedPosition)
return 0;
- return endOfTrack - resolvedPosition;
+ return adjustGridPositionForSide(endOfTrack - resolvedPosition, side);
}
case AutoPosition:
// 'auto' depends on the opposite position for resolution (e.g. grid-row: auto / 1).
Modified: trunk/Source/WebCore/rendering/RenderGrid.h (154730 => 154731)
--- trunk/Source/WebCore/rendering/RenderGrid.h 2013-08-28 09:58:33 UTC (rev 154730)
+++ trunk/Source/WebCore/rendering/RenderGrid.h 2013-08-28 10:26:10 UTC (rev 154731)
@@ -44,6 +44,13 @@
virtual bool avoidsFloats() const OVERRIDE { return true; }
virtual bool canCollapseAnonymousBlockChild() const OVERRIDE { return false; }
+ enum GridPositionSide {
+ ColumnStartSide,
+ ColumnEndSide,
+ RowStartSide,
+ RowEndSide
+ };
+
private:
virtual bool isRenderGrid() const OVERRIDE { return true; }
virtual void computeIntrinsicLogicalWidths(LayoutUnit& minLogicalWidth, LayoutUnit& maxLogicalWidth) const OVERRIDE;
@@ -122,12 +129,6 @@
GridSpan resolveGridPositionsFromAutoPlacementPosition(const RenderBox*, TrackSizingDirection, size_t) const;
PassOwnPtr<GridSpan> resolveGridPositionsFromStyle(const RenderBox*, TrackSizingDirection) const;
- enum GridPositionSide {
- ColumnStartSide,
- ColumnEndSide,
- RowStartSide,
- RowEndSide
- };
size_t resolveGridPositionFromStyle(const GridPosition&, GridPositionSide) const;
LayoutUnit gridAreaBreadthForChild(const RenderBox* child, TrackSizingDirection, const Vector<GridTrack>&) const;
