Title: [138770] trunk
- Revision
- 138770
- Author
- t...@chromium.org
- Date
- 2013-01-03 17:21:07 -0800 (Thu, 03 Jan 2013)
Log Message
incorrect flexbox relayout with overflow, padding and absolute positioning
https://bugs.webkit.org/show_bug.cgi?id=106022
Reviewed by Ojan Vafai.
Source/WebCore:
The problem was we were entering simplified layout, which doesn't apply the stretch
behavior for flex children. That should be fine if we had kept the override size from
the previous layout. So clear the override size during layout rather than after layout.
Not clearing the override size re-triggers bug 98611. Fix this by always forcing a
layout of an image if it has an override size set. The presence of an override size means
we did some special layout that may not be covered by re-computing the logical width and
height.
Test: css3/flexbox/stretch-simplified-layout.html
* rendering/RenderFlexibleBox.cpp:
(WebCore::RenderFlexibleBox::layoutBlock): Don't clear the override size after layout.
(WebCore::RenderFlexibleBox::computeMainAxisPreferredSizes): Clear the override size right before layout.
* rendering/RenderFlexibleBox.h:
(RenderFlexibleBox): Remove clearChildOverrideSizes() since it's not called anymore.
* rendering/RenderImage.cpp:
(WebCore::RenderImage::imageDimensionsChanged): If an override size is present, force a relayout.
Also cleaned up the code a bit to avoid computing the width and height unless we need to.
LayoutTests:
* css3/flexbox/stretch-simplified-layout-expected.txt: Added.
* css3/flexbox/stretch-simplified-layout.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (138769 => 138770)
--- trunk/LayoutTests/ChangeLog 2013-01-04 01:18:03 UTC (rev 138769)
+++ trunk/LayoutTests/ChangeLog 2013-01-04 01:21:07 UTC (rev 138770)
@@ -1,3 +1,13 @@
+2013-01-03 Tony Chang <t...@chromium.org>
+
+ incorrect flexbox relayout with overflow, padding and absolute positioning
+ https://bugs.webkit.org/show_bug.cgi?id=106022
+
+ Reviewed by Ojan Vafai.
+
+ * css3/flexbox/stretch-simplified-layout-expected.txt: Added.
+ * css3/flexbox/stretch-simplified-layout.html: Added.
+
2013-01-03 Terry Anderson <tdander...@chromium.org>
Unreviewed gardening. Marking fast/events/5056619.html as flaky
Added: trunk/LayoutTests/css3/flexbox/stretch-simplified-layout-expected.txt (0 => 138770)
--- trunk/LayoutTests/css3/flexbox/stretch-simplified-layout-expected.txt (rev 0)
+++ trunk/LayoutTests/css3/flexbox/stretch-simplified-layout-expected.txt 2013-01-04 01:21:07 UTC (rev 138770)
@@ -0,0 +1 @@
+PASS
Added: trunk/LayoutTests/css3/flexbox/stretch-simplified-layout.html (0 => 138770)
--- trunk/LayoutTests/css3/flexbox/stretch-simplified-layout.html (rev 0)
+++ trunk/LayoutTests/css3/flexbox/stretch-simplified-layout.html 2013-01-04 01:21:07 UTC (rev 138770)
@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+<html>
+<head>
+<link href="" rel="stylesheet">
+<script src=""
+<script>
+window._onload_ = function() {
+ document.body.offsetHeight;
+
+ document.getElementById('to-hide').style.display = "none";
+ checkLayout(".flexbox");
+};
+</script>
+</head>
+<body>
+<div class="flexbox" style="height: 100px;" data-expected-height="100">
+ <div style="width: 100%; overflow: auto; padding-bottom: 100px; background-color: red;" data-expected-height="100">
+ <div style="position: relative; height: 100px; background-color: green;" data-expected-height="100">
+ <div id="to-hide" style="position: absolute;" data-expected-height="0"></div>
+ </div>
+ </div>
+</div>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (138769 => 138770)
--- trunk/Source/WebCore/ChangeLog 2013-01-04 01:18:03 UTC (rev 138769)
+++ trunk/Source/WebCore/ChangeLog 2013-01-04 01:21:07 UTC (rev 138770)
@@ -1,3 +1,30 @@
+2013-01-03 Tony Chang <t...@chromium.org>
+
+ incorrect flexbox relayout with overflow, padding and absolute positioning
+ https://bugs.webkit.org/show_bug.cgi?id=106022
+
+ Reviewed by Ojan Vafai.
+
+ The problem was we were entering simplified layout, which doesn't apply the stretch
+ behavior for flex children. That should be fine if we had kept the override size from
+ the previous layout. So clear the override size during layout rather than after layout.
+
+ Not clearing the override size re-triggers bug 98611. Fix this by always forcing a
+ layout of an image if it has an override size set. The presence of an override size means
+ we did some special layout that may not be covered by re-computing the logical width and
+ height.
+
+ Test: css3/flexbox/stretch-simplified-layout.html
+
+ * rendering/RenderFlexibleBox.cpp:
+ (WebCore::RenderFlexibleBox::layoutBlock): Don't clear the override size after layout.
+ (WebCore::RenderFlexibleBox::computeMainAxisPreferredSizes): Clear the override size right before layout.
+ * rendering/RenderFlexibleBox.h:
+ (RenderFlexibleBox): Remove clearChildOverrideSizes() since it's not called anymore.
+ * rendering/RenderImage.cpp:
+ (WebCore::RenderImage::imageDimensionsChanged): If an override size is present, force a relayout.
+ Also cleaned up the code a bit to avoid computing the width and height unless we need to.
+
2013-01-03 Adam Klein <ad...@chromium.org>
Unreviewed rebaseline of binding tests after r138754
Modified: trunk/Source/WebCore/rendering/RenderFlexibleBox.cpp (138769 => 138770)
--- trunk/Source/WebCore/rendering/RenderFlexibleBox.cpp 2013-01-04 01:18:03 UTC (rev 138769)
+++ trunk/Source/WebCore/rendering/RenderFlexibleBox.cpp 2013-01-04 01:21:07 UTC (rev 138770)
@@ -342,7 +342,6 @@
layoutPositionedObjects(relayoutChildren || isRoot());
computeRegionRangeForBlock();
- clearChildOverrideSizes();
repaintChildrenDuringLayoutIfMoved(oldChildRects);
// FIXME: css3/flexbox/repaint-rtl-column.html seems to repaint more overflow than it needs to.
@@ -414,12 +413,6 @@
flipForRightToLeftColumn();
}
-void RenderFlexibleBox::clearChildOverrideSizes()
-{
- for (RenderBox* child = firstChildBox(); child; child = child->nextSiblingBox())
- child->clearOverrideSize();
-}
-
bool RenderFlexibleBox::hasOrthogonalFlow(RenderBox* child) const
{
// FIXME: If the child is a flexbox, then we need to check isHorizontalFlow.
@@ -858,6 +851,8 @@
if (child->isOutOfFlowPositioned())
continue;
+ child->clearOverrideSize();
+
// Only need to layout here if we will need to get the logicalHeight of the child in computeNextFlexLine.
Length childMainAxisMin = isHorizontalFlow() ? child->style()->minWidth() : child->style()->minHeight();
if (hasOrthogonalFlow(child) && (flexBasisForChild(child).isAuto() || childMainAxisMin.isAuto())) {
Modified: trunk/Source/WebCore/rendering/RenderFlexibleBox.h (138769 => 138770)
--- trunk/Source/WebCore/rendering/RenderFlexibleBox.h 2013-01-04 01:18:03 UTC (rev 138769)
+++ trunk/Source/WebCore/rendering/RenderFlexibleBox.h 2013-01-04 01:21:07 UTC (rev 138770)
@@ -139,7 +139,6 @@
bool hasAutoMarginsInCrossAxis(RenderBox* child) const;
bool updateAutoMarginsInCrossAxis(RenderBox* child, LayoutUnit availableAlignmentSpace);
void repositionLogicalHeightDependentFlexItems(Vector<LineContext>&, LayoutUnit& oldClientAfterEdge);
- void clearChildOverrideSizes();
void appendChildFrameRects(ChildFrameRects&);
void repaintChildrenDuringLayoutIfMoved(const ChildFrameRects&);
Modified: trunk/Source/WebCore/rendering/RenderImage.cpp (138769 => 138770)
--- trunk/Source/WebCore/rendering/RenderImage.cpp 2013-01-04 01:18:03 UTC (rev 138769)
+++ trunk/Source/WebCore/rendering/RenderImage.cpp 2013-01-04 01:21:07 UTC (rev 138770)
@@ -222,21 +222,27 @@
if (intrinsicSizeChanged) {
if (!preferredLogicalWidthsDirty())
setPreferredLogicalWidthsDirty(true);
- LogicalExtentComputedValues computedValues;
- computeLogicalWidthInRegion(computedValues);
- LayoutUnit newWidth = computedValues.m_extent;
- computeLogicalHeight(height(), 0, computedValues);
- LayoutUnit newHeight = computedValues.m_extent;
+ bool hasOverrideSize = hasOverrideHeight() || hasOverrideWidth();
+ if (!hasOverrideSize && !imageSizeChanged) {
+ LogicalExtentComputedValues computedValues;
+ computeLogicalWidthInRegion(computedValues);
+ LayoutUnit newWidth = computedValues.m_extent;
+ computeLogicalHeight(height(), 0, computedValues);
+ LayoutUnit newHeight = computedValues.m_extent;
+
+ imageSizeChanged = width() != newWidth || height() != newHeight;
+ }
+
// FIXME: We only need to recompute the containing block's preferred size
// if the containing block's size depends on the image's size (i.e., the container uses shrink-to-fit sizing).
// There's no easy way to detect that shrink-to-fit is needed, always force a layout.
bool containingBlockNeedsToRecomputePreferredSize =
- style()->logicalWidth().type() == Percent
- || style()->logicalMaxWidth().type() == Percent
- || style()->logicalMinWidth().type() == Percent;
+ style()->logicalWidth().isPercent()
+ || style()->logicalMaxWidth().isPercent()
+ || style()->logicalMinWidth().isPercent();
- if (imageSizeChanged || width() != newWidth || height() != newHeight || containingBlockNeedsToRecomputePreferredSize) {
+ if (imageSizeChanged || hasOverrideSize || containingBlockNeedsToRecomputePreferredSize) {
shouldRepaint = false;
if (!selfNeedsLayout())
setNeedsLayout(true);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-changes
