Title: [164214] trunk
- Revision
- 164214
- Author
- [email protected]
- Date
- 2014-02-17 02:57:43 -0800 (Mon, 17 Feb 2014)
Log Message
[CSS Grid Layout] Fix missing layout in flexible and content sized columns
https://bugs.webkit.org/show_bug.cgi?id=128672
Reviewed by Sergio Villar Senin.
Source/WebCore:
RenderGrid::logicalContentHeightForChild() is called for some items at the beginning of RenderGrid::layoutGridItems()
from RenderGrid::computeUsedBreadthOfGridTracks(). This causes that the comparison inside the for loop in
RenderGrid::layoutGridItems() does not detect width changes, so elements won't be marked as needsLayout.
So the comparison is done in RenderGrid::logicalContentHeightForChild() and the element is marked to perform a layout if
the width has changed.
The issue can be reproduced easily with a simple grid with one flexible or content sized column, all the available width
is not used. On top of that, when you resize the window the flexible or content sized columns are not updating their
size properly.
CSS Grid Layout perftest results are around 4% worse, which is expected as we're adding a missing layout.
Test: fast/css-grid-layout/flex-content-sized-column-use-available-width.html
* rendering/RenderGrid.cpp:
(WebCore::RenderGrid::logicalContentHeightForChild): Check width changes and mark element as needed layout if required.
LayoutTests:
Add test that reproduce the issue for both cases flexible and content sized columns.
* fast/css-grid-layout/flex-content-sized-column-use-available-width-expected.html: Added.
* fast/css-grid-layout/flex-content-sized-column-use-available-width.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (164213 => 164214)
--- trunk/LayoutTests/ChangeLog 2014-02-17 10:54:24 UTC (rev 164213)
+++ trunk/LayoutTests/ChangeLog 2014-02-17 10:57:43 UTC (rev 164214)
@@ -1,3 +1,15 @@
+2014-02-17 Manuel Rego Casasnovas <[email protected]>
+
+ [CSS Grid Layout] Fix missing layout in flexible and content sized columns
+ https://bugs.webkit.org/show_bug.cgi?id=128672
+
+ Reviewed by Sergio Villar Senin.
+
+ Add test that reproduce the issue for both cases flexible and content sized columns.
+
+ * fast/css-grid-layout/flex-content-sized-column-use-available-width-expected.html: Added.
+ * fast/css-grid-layout/flex-content-sized-column-use-available-width.html: Added.
+
2014-02-15 Filip Pizlo <[email protected]>
FTL should inline polymorphic heap accesses
Added: trunk/LayoutTests/fast/css-grid-layout/flex-content-sized-column-use-available-width-expected.html (0 => 164214)
--- trunk/LayoutTests/fast/css-grid-layout/flex-content-sized-column-use-available-width-expected.html (rev 0)
+++ trunk/LayoutTests/fast/css-grid-layout/flex-content-sized-column-use-available-width-expected.html 2014-02-17 10:57:43 UTC (rev 164214)
@@ -0,0 +1,27 @@
+<!DOCTYPE html>
+<html>
+<head>
+ <style type="text/css">
+ .item {
+ background-color: blue;
+ }
+ </style>
+</head>
+<body>
+ <h1>Description</h1>
+ <p>Flex and content sized column should use all the available width and you should not see the grid background (grey at the end).</p>
+ <h1>Grid 1 flex column</h1>
+ <div>
+ <div class="item">grid item</div>
+ </div>
+ <h1>Grid 1 auto column</h1>
+ <div>
+ <div class="item">
+ grid item grid item grid item grid item grid item grid item grid item grid item grid item grid item
+ grid item grid item grid item grid item grid item grid item grid item grid item grid item grid item
+ grid item grid item grid item grid item grid item grid item grid item grid item grid item grid item
+ grid item grid item grid item grid item grid item grid item grid item grid item grid item grid item
+ </div>
+ </div>
+</body>
+</html>
Added: trunk/LayoutTests/fast/css-grid-layout/flex-content-sized-column-use-available-width.html (0 => 164214)
--- trunk/LayoutTests/fast/css-grid-layout/flex-content-sized-column-use-available-width.html (rev 0)
+++ trunk/LayoutTests/fast/css-grid-layout/flex-content-sized-column-use-available-width.html 2014-02-17 10:57:43 UTC (rev 164214)
@@ -0,0 +1,36 @@
+<!DOCTYPE html>
+<html>
+<head>
+ <script>
+ if (window.testRunner)
+ testRunner.overridePreference("WebKitCSSGridLayoutEnabled", 1);
+ </script>
+ <link href="" rel="stylesheet">
+ <style type="text/css">
+ #grid-1 {
+ -webkit-grid-template-columns: 1fr;
+ }
+
+ #grid-2 {
+ -webkit-grid-template-columns: auto;
+ }
+ </style>
+</head>
+<body>
+ <h1>Description</h1>
+ <p>Flex and content sized column should use all the available width and you should not see the grid background (grey at the end).</p>
+ <h1>Grid 1 flex column</h1>
+ <div id="grid-1" class="grid">
+ <div class="firstRowFirstColumn">grid item</div>
+ </div>
+ <h1>Grid 1 auto column</h1>
+ <div id="grid-2" class="grid">
+ <div class="firstRowFirstColumn">
+ grid item grid item grid item grid item grid item grid item grid item grid item grid item grid item
+ grid item grid item grid item grid item grid item grid item grid item grid item grid item grid item
+ grid item grid item grid item grid item grid item grid item grid item grid item grid item grid item
+ grid item grid item grid item grid item grid item grid item grid item grid item grid item grid item
+ </div>
+ </div>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (164213 => 164214)
--- trunk/Source/WebCore/ChangeLog 2014-02-17 10:54:24 UTC (rev 164213)
+++ trunk/Source/WebCore/ChangeLog 2014-02-17 10:57:43 UTC (rev 164214)
@@ -1,3 +1,28 @@
+2014-02-17 Manuel Rego Casasnovas <[email protected]>
+
+ [CSS Grid Layout] Fix missing layout in flexible and content sized columns
+ https://bugs.webkit.org/show_bug.cgi?id=128672
+
+ Reviewed by Sergio Villar Senin.
+
+ RenderGrid::logicalContentHeightForChild() is called for some items at the beginning of RenderGrid::layoutGridItems()
+ from RenderGrid::computeUsedBreadthOfGridTracks(). This causes that the comparison inside the for loop in
+ RenderGrid::layoutGridItems() does not detect width changes, so elements won't be marked as needsLayout.
+
+ So the comparison is done in RenderGrid::logicalContentHeightForChild() and the element is marked to perform a layout if
+ the width has changed.
+
+ The issue can be reproduced easily with a simple grid with one flexible or content sized column, all the available width
+ is not used. On top of that, when you resize the window the flexible or content sized columns are not updating their
+ size properly.
+
+ CSS Grid Layout perftest results are around 4% worse, which is expected as we're adding a missing layout.
+
+ Test: fast/css-grid-layout/flex-content-sized-column-use-available-width.html
+
+ * rendering/RenderGrid.cpp:
+ (WebCore::RenderGrid::logicalContentHeightForChild): Check width changes and mark element as needed layout if required.
+
2014-02-16 Andreas Kling <[email protected]>
Ensure that removing an iframe from the DOM tree disconnects its Frame.
Modified: trunk/Source/WebCore/rendering/RenderGrid.cpp (164213 => 164214)
--- trunk/Source/WebCore/rendering/RenderGrid.cpp 2014-02-17 10:54:24 UTC (rev 164213)
+++ trunk/Source/WebCore/rendering/RenderGrid.cpp 2014-02-17 10:57:43 UTC (rev 164214)
@@ -424,10 +424,12 @@
LayoutUnit RenderGrid::logicalContentHeightForChild(RenderBox* child, Vector<GridTrack>& columnTracks)
{
- if (child->style().logicalHeight().isPercent())
+ LayoutUnit oldOverrideContainingBlockContentLogicalWidth = child->hasOverrideContainingBlockLogicalWidth() ? child->overrideContainingBlockContentLogicalWidth() : LayoutUnit();
+ LayoutUnit overrideContainingBlockContentLogicalWidth = gridAreaBreadthForChild(child, ForColumns, columnTracks);
+ if (child->style().logicalHeight().isPercent() || oldOverrideContainingBlockContentLogicalWidth != overrideContainingBlockContentLogicalWidth)
child->setNeedsLayout(MarkOnlyThis);
- child->setOverrideContainingBlockContentLogicalWidth(gridAreaBreadthForChild(child, ForColumns, columnTracks));
+ child->setOverrideContainingBlockContentLogicalWidth(overrideContainingBlockContentLogicalWidth);
// If |child| has a percentage logical height, we shouldn't let it override its intrinsic height, which is
// what we are interested in here. Thus we need to set the override logical height to -1 (no possible resolution).
child->setOverrideContainingBlockContentLogicalHeight(-1);
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
