WebKit Bugzilla
New
Browse
Search+
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
[patch]
Patch
bug-145604-20150608121858.patch (text/plain), 8.08 KB, created by
Javier Fernandez
on 2015-06-08 03:19:48 PDT
(
hide
)
Description:
Patch
Filename:
MIME Type:
Creator:
Javier Fernandez
Created:
2015-06-08 03:19:48 PDT
Size:
8.08 KB
patch
obsolete
>Subversion Revision: 185072 >diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog >index ddf49b73d70a1246df2bc1315314d17ae5470cd9..4bdc215a017f654d7833a3112d366bd5fbd8e0d8 100644 >--- a/Source/WebCore/ChangeLog >+++ b/Source/WebCore/ChangeLog >@@ -1,3 +1,23 @@ >+2015-06-08 Javier Fernandez <jfernandez@igalia.com> >+ >+ [CSS Grid Layout] Setting height on a grid item doesn't have any effect >+ https://bugs.webkit.org/show_bug.cgi?id=145604 >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ Box Alignment spec states that stretch is only possible when height is >+ 'auto' and no 'auto' margins are used. >+ >+ It might be the case that style changes so that stretching is not allowed, >+ hence we need to detect it and clear the override height the stretching >+ algorithm previously set. The new layout triggered by the style change >+ will then set grid item's height according to the new style rules. >+ >+ Test: fast/css-grid-layout/grid-item-should-not-be-stretched-when-height-or-margin-change.html >+ >+ * rendering/RenderGrid.cpp: >+ (WebCore::RenderGrid::applyStretchAlignmentToChildIfNeeded): >+ > 2015-06-01 Benjamin Poulain <bpoulain@apple.com> > > [CSS JIT] Fail to compile when we are out of executable memory >diff --git a/Source/WebCore/rendering/RenderGrid.cpp b/Source/WebCore/rendering/RenderGrid.cpp >index 3bd26a5e77bae0093768373675e16622ad53742d..bc378a533b8a32ad4c57463725c21507199e2377 100644 >--- a/Source/WebCore/rendering/RenderGrid.cpp >+++ b/Source/WebCore/rendering/RenderGrid.cpp >@@ -1296,25 +1296,25 @@ LayoutUnit RenderGrid::availableAlignmentSpaceForChildBeforeStretching(LayoutUni > // FIXME: This logic is shared by RenderFlexibleBox, so it should be moved to RenderBox. > void RenderGrid::applyStretchAlignmentToChildIfNeeded(RenderBox& child, LayoutUnit gridAreaBreadthForChild) > { >- if (RenderStyle::resolveAlignment(style(), child.style(), ItemPositionStretch) != ItemPositionStretch) >+ if (!allowedToStretchLogicalHeightForChild(child) || RenderStyle::resolveAlignment(style(), child.style(), ItemPositionStretch) != ItemPositionStretch) { >+ child.clearOverrideLogicalContentHeight(); > return; >+ } > > bool hasOrthogonalWritingMode = child.isHorizontalWritingMode() != isHorizontalWritingMode(); >- if (allowedToStretchLogicalHeightForChild(child)) { >- // FIXME: If the child has orthogonal flow, then it already has an override height set, so use it. >- // FIXME: grid track sizing and positioning do not support orthogonal modes yet. >- if (!hasOrthogonalWritingMode) { >- LayoutUnit stretchedLogicalHeight = availableAlignmentSpaceForChildBeforeStretching(gridAreaBreadthForChild, child); >- LayoutUnit desiredLogicalHeight = child.constrainLogicalHeightByMinMax(stretchedLogicalHeight); >- >- // FIXME: Can avoid laying out here in some cases. See https://webkit.org/b/87905. >- bool childNeedsRelayout = desiredLogicalHeight != child.logicalHeight(); >- if (childNeedsRelayout || !child.hasOverrideLogicalContentHeight()) >- child.setOverrideLogicalContentHeight(desiredLogicalHeight - child.borderAndPaddingLogicalHeight()); >- if (childNeedsRelayout) { >- child.setLogicalHeight(0); >- child.setNeedsLayout(); >- } >+ // FIXME: If the child has orthogonal flow, then it already has an override height set, so use it. >+ // FIXME: grid track sizing and positioning do not support orthogonal modes yet. >+ if (!hasOrthogonalWritingMode) { >+ LayoutUnit stretchedLogicalHeight = availableAlignmentSpaceForChildBeforeStretching(gridAreaBreadthForChild, child); >+ LayoutUnit desiredLogicalHeight = child.constrainLogicalHeightByMinMax(stretchedLogicalHeight); >+ >+ // FIXME: Can avoid laying out here in some cases. See https://webkit.org/b/87905. >+ bool childNeedsRelayout = desiredLogicalHeight != child.logicalHeight(); >+ if (childNeedsRelayout || !child.hasOverrideLogicalContentHeight()) >+ child.setOverrideLogicalContentHeight(desiredLogicalHeight - child.borderAndPaddingLogicalHeight()); >+ if (childNeedsRelayout) { >+ child.setLogicalHeight(0); >+ child.setNeedsLayout(); > } > } > } >diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog >index 792e2152457286b723d1db5b447e601bfd5756a2..539b460a966e841fe3fa8567ca92b4dce763827f 100644 >--- a/LayoutTests/ChangeLog >+++ b/LayoutTests/ChangeLog >@@ -1,3 +1,16 @@ >+2015-06-08 Javier Fernandez <jfernandez@igalia.com> >+ >+ [CSS Grid Layout] Setting height on a grid item doesn't have any effect >+ https://bugs.webkit.org/show_bug.cgi?id=145604 >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ Tests to verify that we clear the override height set by the stretching logic >+ whenever height or margin change in a way they don't allow stretching anymore. >+ >+ * fast/css-grid-layout/grid-item-should-not-be-stretched-when-height-or-margin-change-expected.txt: Added. >+ * fast/css-grid-layout/grid-item-should-not-be-stretched-when-height-or-margin-change.html: Added. >+ > 2015-06-01 Brady Eidson <beidson@apple.com> > > Add WKTR support for "should open external URLs". >diff --git a/LayoutTests/fast/css-grid-layout/grid-item-should-not-be-stretched-when-height-or-margin-change-expected.txt b/LayoutTests/fast/css-grid-layout/grid-item-should-not-be-stretched-when-height-or-margin-change-expected.txt >new file mode 100644 >index 0000000000000000000000000000000000000000..d950e0e0fb3cd0af743f357e7bca3c780b7fbb7a >--- /dev/null >+++ b/LayoutTests/fast/css-grid-layout/grid-item-should-not-be-stretched-when-height-or-margin-change-expected.txt >@@ -0,0 +1,4 @@ >+The grids below had initially 'stretched' items, but we have changed 'height' and 'margin' to values which don't allow stretch. This test verifies that the layout algorithm properly detects such changes and clear the override height accordingly. >+ >+PASS >+PASS >diff --git a/LayoutTests/fast/css-grid-layout/grid-item-should-not-be-stretched-when-height-or-margin-change.html b/LayoutTests/fast/css-grid-layout/grid-item-should-not-be-stretched-when-height-or-margin-change.html >new file mode 100644 >index 0000000000000000000000000000000000000000..7b4f30f8acd418b76c07f3a77787394dffa7a882 >--- /dev/null >+++ b/LayoutTests/fast/css-grid-layout/grid-item-should-not-be-stretched-when-height-or-margin-change.html >@@ -0,0 +1,37 @@ >+<!DOCTYPE HTML> >+<link href="resources/grid.css" rel="stylesheet"> >+<script src="../../resources/check-layout.js"></script> >+<style> >+.grid { >+ -webkit-grid-template: 200px 200px / 200px 200px; >+ width: -webkit-fit-content; >+ position: relative; >+} >+#fromFixedHeight { height: 100px; } >+#fromMarginAuto { margin: auto; } >+</style> >+<p>The grids below had initially 'stretched' items, but we have changed 'height' and 'margin' to values which don't allow stretch. This test verifies that the layout algorithm properly detects such changes and clear the override height accordingly.</p> >+<div class="grid"> >+ <div id="toFixedHeight" class="firstRowFirstColumn" data-expected-height="100"></div> >+ <div class="firstRowSecondColumn" data-expected-height="200"></div> >+ <div class="secondRowFirstColumn" data-expected-height="200"></div> >+ <div id="fromFixedHeight" class="secondRowSecondColumn" data-expected-height="200"></div> >+</div> >+<div class="grid"> >+ <div id="toMarginAuto" class="firstRowFirstColumn" data-expected-height="100"> >+ <div style="height: 100px"></div> >+ </div> >+ <div class="firstRowSecondColumn" data-expected-height="200"></div> >+ <div class="secondRowFirstColumn" data-expected-height="200"></div> >+ <div id="fromMarginAuto" class="secondRowSecondColumn" data-expected-height="200"> >+ <div style="height: 100px"></div> >+ </div> >+</div> >+<script> >+document.body.offsetLeft; >+document.getElementById("fromFixedHeight").style.height = "auto"; >+document.getElementById("toFixedHeight").style.height = "100px"; >+document.getElementById("fromMarginAuto").style.margin = "0"; >+document.getElementById("toMarginAuto").style.margin = "auto"; >+checkLayout(".grid"); >+</script>
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 145604
:
254173
|
254181
| 254483