The specs[1] mandate us to return the used value instead of the specified value which was the one we were using. This requires to force a layout to compute the used values. This means that we'll have to update a lot of test results. [1] http://dev.w3.org/csswg/css-grid/#resolved-track-list
Created attachment 222577 [details] Patch
Created attachment 222580 [details] Patch With fix for the mac EWS
Comment on attachment 222580 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=222580&action=review > Source/WebCore/ChangeLog:17 > + According to the specs getComputedStyle() should return the used > + values instead of the resolved values for compatibility with early > + implementations. This means that grid-definition-{columns|rows} > + are now layout dependent as we need to compute the used values for > + grid track sizes. I suppose this is consistent with other implementations of grid layout? > Source/WebCore/rendering/RenderGrid.cpp:1020 > + m_columnPositions.resize(sizingData.columnTracks.size() + 1); If we don't plan to extend past this size, let's use resizeToFit() to avoid padding the vector for growth. > Source/WebCore/rendering/RenderGrid.cpp:1025 > + m_rowPositions.resize(sizingData.rowTracks.size() + 1); Same here. > Source/WebCore/rendering/RenderGrid.cpp:-1022 > - // FIXME: |columnTrack| and |rowTrack| should be smaller than our column / row count. Does this FIXME no longer apply? Should we take it with us to populateGridPositions()?
Comment on attachment 222580 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=222580&action=review >> Source/WebCore/ChangeLog:17 >> + grid track sizes. > > I suppose this is consistent with other implementations of grid layout? Yes, actually it is not consistent with the rest of the CSS specs but the WG decided to add that exception to grid layout to be consistent mainly with IE's implementation. Regarding Blink, well I landed a similar fix there some months ago so yeah pretty consistent :). >> Source/WebCore/rendering/RenderGrid.cpp:1020 >> + m_columnPositions.resize(sizingData.columnTracks.size() + 1); > > If we don't plan to extend past this size, let's use resizeToFit() to avoid padding the vector for growth. Fair enough >> Source/WebCore/rendering/RenderGrid.cpp:-1022 >> - // FIXME: |columnTrack| and |rowTrack| should be smaller than our column / row count. > > Does this FIXME no longer apply? Should we take it with us to populateGridPositions()? It's an old comment that does no longer apply, even without this patch. Note that we're actually checking what the FIXME mentions in the two ASSERTs above. The cached grid coordinate is guaranteed to respect those limits because the grid grows if needed when the GridCoordinate is stored.
Comment on attachment 222580 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=222580&action=review > Source/WebCore/rendering/RenderGrid.cpp:1035 > + ASSERT_UNUSED(sizingData, coordinate.rows.initialPositionIndex < rowTracks.size()); Actually this is wrong, the right parts of the conditions should be sizingData.columnTracks.size() and sizingData.rowTracks.size(). Don't know how I could upload a patch that does not build...
Created attachment 223139 [details] Patch
Comment on attachment 223139 [details] Patch OK, r=me
Committed r163547: <http://trac.webkit.org/changeset/163547>
*** Bug 77566 has been marked as a duplicate of this bug. ***