Title: [122489] trunk/Source/WebCore
- Revision
- 122489
- Author
- [email protected]
- Date
- 2012-07-12 11:46:53 -0700 (Thu, 12 Jul 2012)
Log Message
https://bugs.webkit.org/show_bug.cgi?id=91000
REGRESSION (r122244): Overflow elements don't shrink as much as they should.
Reviewed by Simon Fraser.
This is a fix for a a regression from https://bugs.webkit.org/show_bug.cgi?id=90646.
I incorrectly analyzed the issue with Robert Hogan's negative margin patch and fooled myself into putting back
in an incorrect minimum width check from long ago.
What should have happened in the test case I patched is that the overflow element should shrink to 0. The issue
with improving the logical top estimate in the previous patch is it made the clear delta become 0. This in turn
exposed a bug in our clearing algorithm with Robert's changes where you could need a relayout even if you didn't
actually move. This issue only occurs because the floats list is getting changed mid-layout because of negative margins.
The patch changes getClearDelta to call setChildNeedsLayout(true) on children whose widths change even when their
positions do not. In effect this dynamic addition of new floats after you have done a layout on the child already means
that you can need to lay out again despite not actually having to move.
To handle this, the code that does the relayout is now called if the child needs a relayout. This is done even if
the logical top estimate matches the final position.
No new tests required, since the test in fast/block/float is now correctly covering the issue.
* rendering/RenderBlock.cpp:
(WebCore::RenderBlock::layoutBlockChild):
(WebCore::RenderBlock::getClearDelta):
* rendering/RenderBox.cpp:
(WebCore::RenderBox::shrinkLogicalWidthToAvoidFloats):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (122488 => 122489)
--- trunk/Source/WebCore/ChangeLog 2012-07-12 18:31:09 UTC (rev 122488)
+++ trunk/Source/WebCore/ChangeLog 2012-07-12 18:46:53 UTC (rev 122489)
@@ -1,3 +1,35 @@
+2012-07-11 David Hyatt <[email protected]>
+
+ https://bugs.webkit.org/show_bug.cgi?id=91000
+ REGRESSION (r122244): Overflow elements don't shrink as much as they should.
+
+ Reviewed by Simon Fraser.
+
+ This is a fix for a a regression from https://bugs.webkit.org/show_bug.cgi?id=90646.
+
+ I incorrectly analyzed the issue with Robert Hogan's negative margin patch and fooled myself into putting back
+ in an incorrect minimum width check from long ago.
+
+ What should have happened in the test case I patched is that the overflow element should shrink to 0. The issue
+ with improving the logical top estimate in the previous patch is it made the clear delta become 0. This in turn
+ exposed a bug in our clearing algorithm with Robert's changes where you could need a relayout even if you didn't
+ actually move. This issue only occurs because the floats list is getting changed mid-layout because of negative margins.
+
+ The patch changes getClearDelta to call setChildNeedsLayout(true) on children whose widths change even when their
+ positions do not. In effect this dynamic addition of new floats after you have done a layout on the child already means
+ that you can need to lay out again despite not actually having to move.
+
+ To handle this, the code that does the relayout is now called if the child needs a relayout. This is done even if
+ the logical top estimate matches the final position.
+
+ No new tests required, since the test in fast/block/float is now correctly covering the issue.
+
+ * rendering/RenderBlock.cpp:
+ (WebCore::RenderBlock::layoutBlockChild):
+ (WebCore::RenderBlock::getClearDelta):
+ * rendering/RenderBox.cpp:
+ (WebCore::RenderBox::shrinkLogicalWidthToAvoidFloats):
+
2012-07-12 James Weatherall <[email protected]>
storage tests are flaky (crashing) on windows
Modified: trunk/Source/WebCore/rendering/RenderBlock.cpp (122488 => 122489)
--- trunk/Source/WebCore/rendering/RenderBlock.cpp 2012-07-12 18:31:09 UTC (rev 122488)
+++ trunk/Source/WebCore/rendering/RenderBlock.cpp 2012-07-12 18:46:53 UTC (rev 122489)
@@ -2427,7 +2427,9 @@
setLogicalTopForChild(child, logicalTopAfterClear, ApplyLayoutDelta);
// Now we have a final top position. See if it really does end up being different from our estimate.
- if (logicalTopAfterClear != logicalTopEstimate) {
+ // clearFloatsIfNeeded can also mark the child as needing a layout even though we didn't move. This happens
+ // when collapseMargins dynamically adds overhanging floats because of a child with negative margins.
+ if (logicalTopAfterClear != logicalTopEstimate || child->needsLayout()) {
if (child->shrinkToAvoidFloats()) {
// The child's width depends on the line width.
// When the child shifts to clear an item, its width can
@@ -4583,6 +4585,10 @@
if (availableLogicalWidthAtNewLogicalTopOffset == availableLogicalWidthForContent(newLogicalTop))
return newLogicalTop - logicalTop;
+ RenderRegion* region = regionAtBlockOffset(logicalTopForChild(child));
+ LayoutRect borderBox = child->borderBoxRectInRegion(region, offsetFromLogicalTopOfFirstPage() + logicalTopForChild(child), DoNotCacheRenderBoxRegionInfo);
+ LayoutUnit childLogicalWidthAtOldLogicalTopOffset = isHorizontalWritingMode() ? borderBox.width() : borderBox.height();
+
// FIXME: None of this is right for perpendicular writing-mode children.
LayoutUnit childOldLogicalWidth = child->logicalWidth();
LayoutUnit childOldMarginLeft = child->marginLeft();
@@ -4591,8 +4597,8 @@
child->setLogicalTop(newLogicalTop);
child->computeLogicalWidth();
- RenderRegion* region = regionAtBlockOffset(logicalTopForChild(child));
- LayoutRect borderBox = child->borderBoxRectInRegion(region, offsetFromLogicalTopOfFirstPage() + logicalTopForChild(child), DoNotCacheRenderBoxRegionInfo);
+ region = regionAtBlockOffset(logicalTopForChild(child));
+ borderBox = child->borderBoxRectInRegion(region, offsetFromLogicalTopOfFirstPage() + logicalTopForChild(child), DoNotCacheRenderBoxRegionInfo);
LayoutUnit childLogicalWidthAtNewLogicalTopOffset = isHorizontalWritingMode() ? borderBox.width() : borderBox.height();
child->setLogicalTop(childOldLogicalTop);
@@ -4600,8 +4606,14 @@
child->setMarginLeft(childOldMarginLeft);
child->setMarginRight(childOldMarginRight);
- if (childLogicalWidthAtNewLogicalTopOffset <= availableLogicalWidthAtNewLogicalTopOffset)
+ if (childLogicalWidthAtNewLogicalTopOffset <= availableLogicalWidthAtNewLogicalTopOffset) {
+ // Even though we may not be moving, if the logical width did shrink because of the presence of new floats, then
+ // we need to force a relayout as though we shifted. This happens because of the dynamic addition of overhanging floats
+ // from previous siblings when negative margins exist on a child (see the addOverhangingFloats call at the end of collapseMargins).
+ if (childLogicalWidthAtOldLogicalTopOffset != childLogicalWidthAtNewLogicalTopOffset)
+ child->setChildNeedsLayout(true, MarkOnlyThis);
return newLogicalTop - logicalTop;
+ }
newLogicalTop = nextFloatLogicalBottomBelow(newLogicalTop);
ASSERT(newLogicalTop >= logicalTop);
Modified: trunk/Source/WebCore/rendering/RenderBox.cpp (122488 => 122489)
--- trunk/Source/WebCore/rendering/RenderBox.cpp 2012-07-12 18:31:09 UTC (rev 122488)
+++ trunk/Source/WebCore/rendering/RenderBox.cpp 2012-07-12 18:46:53 UTC (rev 122489)
@@ -1178,7 +1178,6 @@
}
LayoutUnit result = cb->availableLogicalWidthForLine(logicalTopPosition, false, containingBlockRegion, adjustedPageOffsetForContainingBlock) - childMarginStart - childMarginEnd;
- result = max(result, minPreferredLogicalWidth()); // Don't shrink below our minimum preferred logical width.
// We need to see if margins on either the start side or the end side can contain the floats in question. If they can,
// then just using the line width is inaccurate. In the case where a float completely fits, we don't need to use the line
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo/webkit-changes
