Changeset 178701 in webkit

Timestamp:

Jan 20, 2015 1:19:48 AM (9 years ago)

Author:

svillar@igalia.com

Message:

[CSS Grid Layout] Wrong arguments passed to computeNormalizedFractionBreadth
​https://bugs.webkit.org/show_bug.cgi?id=139058

Reviewed by Andreas Kling.

Source/WebCore:

The method RenderGrid::computeNormalizedFractionBreadth() is
supposed to get as fourth argument either the grid element's
content box size or the max-size of a particular grid item. For
the former we were incorrectly passing the free space after
distributing the available space to tracks.

Apart from that, that method has a bug in its implementation. As
we were using the free space instead of the grid element's content
box size, we didn't have to subtract the usedBreadth of grid
tracks from the passed in value. That did not work for the case of
the caller passing the max-size of a particular item.

• rendering/RenderGrid.cpp:

(WebCore::RenderGrid::computeUsedBreadthOfGridTracks):
(WebCore::RenderGrid::computeNormalizedFractionBreadth):

LayoutTests:

• TestExpectations: Removed the test added in r148643 (missing

from r148642) as the test works as expected now.

• fast/css-grid-layout/flex-and-minmax-content-resolution-rows.html:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/TestExpectations (modified) (1 diff)
• LayoutTests/fast/css-grid-layout/flex-and-minmax-content-resolution-rows.html (modified) (3 diffs)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/RenderGrid.cpp (modified) (5 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2015-01-19  Sergio Villar Senin  <svillar@igalia.com>
+
+        [CSS Grid Layout] Wrong arguments passed to computeNormalizedFractionBreadth
+        https://bugs.webkit.org/show_bug.cgi?id=139058
+
+        Reviewed by Andreas Kling.
+
+        * TestExpectations: Removed the test added in r148643 (missing
+        from r148642) as the test works as expected now.
+        * fast/css-grid-layout/flex-and-minmax-content-resolution-rows.html:
+
 2015-01-19  Michael Saboff  <msaboff@apple.com>
 

trunk/LayoutTests/TestExpectations

@@ -458 +458 @@
 
 webkit.org/b/140309 css3/background/background-repeat-space-content.html [ ImageOnlyFailure ]
-
-webkit.org/b/139058 fast/css-grid-layout/flex-and-minmax-content-resolution-rows.html [ Failure ]

trunk/LayoutTests/fast/css-grid-layout/flex-and-minmax-content-resolution-rows.html

@@ -87 +87 @@
 <div class="constrainedContainer">
     <div class="grid gridMinMinContent">
-        <div class="sizedToGridArea firstRowFirstColumn" data-expected-width="50" data-expected-height="30">XXXXX XXXX</div>
+        <div class="sizedToGridArea firstRowFirstColumn" data-expected-width="50" data-expected-height="20">XXXXX XXXX</div>
         <div class="sizedToGridArea secondRowFirstColumn" data-expected-width="50" data-expected-height="50">XXXXX XXXX</div>
     </div>
@@ -102 +102 @@
 <div style="width: 10px; height: 40px">
     <div class="grid gridMinMinContent">
-        <div class="sizedToGridArea firstRowFirstColumn" data-expected-width="50" data-expected-height="30">XXXXX XXXX</div>
+        <div class="sizedToGridArea firstRowFirstColumn" data-expected-width="50" data-expected-height="20">XXXXX XXXX</div>
         <div class="sizedToGridArea secondRowFirstColumn" data-expected-width="50" data-expected-height="50">XXXXX XXXX</div>
     </div>
@@ -116 +116 @@
 <div style="width: 10px; height: 110px;">
     <div class="grid gridMinMinContent">
-        <div class="sizedToGridArea firstRowFirstColumn" data-expected-width="50" data-expected-height="30">XXXXX XXXX</div>
+        <div class="sizedToGridArea firstRowFirstColumn" data-expected-width="50" data-expected-height="20">XXXXX XXXX</div>
         <div class="sizedToGridArea secondRowFirstColumn" data-expected-width="50" data-expected-height="50">XXXXX XXXX</div>
     </div>

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2015-01-19  Sergio Villar Senin  <svillar@igalia.com>
+
+        [CSS Grid Layout] Wrong arguments passed to computeNormalizedFractionBreadth
+        https://bugs.webkit.org/show_bug.cgi?id=139058
+
+        Reviewed by Andreas Kling.
+
+        The method RenderGrid::computeNormalizedFractionBreadth() is
+        supposed to get as fourth argument either the grid element's
+        content box size or the max-size of a particular grid item. For
+        the former we were incorrectly passing the free space after
+        distributing the available space to tracks.
+
+        Apart from that, that method has a bug in its implementation. As
+        we were using the free space instead of the grid element's content
+        box size, we didn't have to subtract the usedBreadth of grid
+        tracks from the passed in value. That did not work for the case of
+        the caller passing the max-size of a particular item.
+
+        * rendering/RenderGrid.cpp:
+        (WebCore::RenderGrid::computeUsedBreadthOfGridTracks):
+        (WebCore::RenderGrid::computeNormalizedFractionBreadth):
+
 2015-01-19  Myles C. Maxfield  <mmaxfield@apple.com>
 

trunk/Source/WebCore/rendering/RenderGrid.cpp

@@ -298 +298 @@
 void RenderGrid::computeUsedBreadthOfGridTracks(GridTrackSizingDirection direction, GridSizingData& sizingData, LayoutUnit& availableLogicalSpace)
 {
+    const LayoutUnit initialAvailableLogicalSpace = availableLogicalSpace;
     Vector<GridTrack>& tracks = (direction == ForColumns) ? sizingData.columnTracks : sizingData.rowTracks;
     Vector<unsigned> flexibleSizedTracksIndex;
@@ -335 +336 @@
         return;
 
-    // 3. Grow all Grid tracks in GridTracks from their UsedBreadth up to their MaxBreadth value until
-    // availableLogicalSpace (RemainingSpace in the specs) is exhausted.
+    // 3. Grow all Grid tracks in GridTracks from their UsedBreadth up to their MaxBreadth value until availableLogicalSpace is exhausted.
     if (!hasUndefinedRemainingSpace) {
         const unsigned tracksSize = tracks.size();
@@ -355 +355 @@
     double normalizedFractionBreadth = 0;
     if (!hasUndefinedRemainingSpace)
-        normalizedFractionBreadth = computeNormalizedFractionBreadth(tracks, GridSpan(0, tracks.size() - 1), direction, availableLogicalSpace);
+        normalizedFractionBreadth = computeNormalizedFractionBreadth(tracks, GridSpan(0, tracks.size() - 1), direction, initialAvailableLogicalSpace);
     else {
         for (auto trackIndex : flexibleSizedTracksIndex) {
@@ -422 +422 @@
 }
 
-double RenderGrid::computeNormalizedFractionBreadth(Vector<GridTrack>& tracks, const GridSpan& tracksSpan, GridTrackSizingDirection direction, LayoutUnit availableLogicalSpace) const
-{
-    // |availableLogicalSpace| already accounts for the used breadths so no need to remove it here.
-
+double RenderGrid::computeNormalizedFractionBreadth(Vector<GridTrack>& tracks, const GridSpan& tracksSpan, GridTrackSizingDirection direction, LayoutUnit spaceToFill) const
+{
+    LayoutUnit allocatedSpace;
     Vector<GridTrackForNormalization> tracksForNormalization;
     for (auto& position : tracksSpan) {
+        GridTrack& track = tracks[position.toInt()];
+        allocatedSpace += track.m_usedBreadth;
+
         const GridTrackSize& trackSize = gridTrackSize(direction, position.toInt());
         if (!trackSize.maxTrackBreadth().isFlex())
             continue;
 
-        tracksForNormalization.append(GridTrackForNormalization(tracks[position.toInt()], trackSize.maxTrackBreadth().flex()));
+        tracksForNormalization.append(GridTrackForNormalization(track, trackSize.maxTrackBreadth().flex()));
     }
 
@@ -448 +450 @@
     double accumulatedFractions = 0;
     LayoutUnit fractionValueBasedOnGridItemsRatio = 0;
-    LayoutUnit availableLogicalSpaceIgnoringFractionTracks = availableLogicalSpace;
+    LayoutUnit availableLogicalSpaceIgnoringFractionTracks = spaceToFill - allocatedSpace;
 
     for (auto& track : tracksForNormalization) {