Commit 8b2cd1fe authored by hyatt@apple.com's avatar hyatt@apple.com

WebCore:

2009-01-25  David Hyatt  <hyatt@apple.com>

        Fix for https://bugs.webkit.org/show_bug.cgi?id=23524, lots of missing content in table sections.

        The new table code created a bug involving  markAllDescendantsWithFloatsForLayout, namely that it could
        end up marking ancestors of a block as needing layout when that block was still in the process of
        doing a layout.

        The fix is to add a parameter to markAllDescendantsWithFloatsForLayout that says whether or not
        we are "mid-layout."  If this flag is set, then the method will make sure to do only local dirtying
        of objects to avoid accidentally marking a clean ancestor as needing layout again.

        Ultimately the second parameter to setNeedsLayout and setChildNeedsLayout should just be removed,
        with a check of whether or not we are mid-layout being done by those methods instead.

        Reviewed by Oliver Hunt

        Added fast/repaint/dynamic-table-vertical-alignment-change.html

        * rendering/RenderBlock.cpp:
        (WebCore::RenderBlock::collapseMargins):
        (WebCore::RenderBlock::clearFloatsIfNeeded):
        (WebCore::RenderBlock::layoutBlockChildren):
        (WebCore::RenderBlock::markAllDescendantsWithFloatsForLayout):
        * rendering/RenderBlock.h:
        * rendering/RenderObject.cpp:
        (WebCore::RenderObject::removeFromObjectLists):
        * rendering/RenderObject.h:
        * rendering/RenderTableSection.cpp:
        (WebCore::RenderTableSection::layoutRows):

LayoutTests:

2009-01-25  David Hyatt  <hyatt@apple.com>

        Add layout test for https://bugs.webkit.org/show_bug.cgi?id=23524.

        Reviewed by Oliver Hunt

        * fast/repaint/dynamic-table-vertical-alignment-change.html: Added.
        * platform/mac/fast/repaint/dynamic-table-vertical-alignment-change-expected.checksum: Added.
        * platform/mac/fast/repaint/dynamic-table-vertical-alignment-change-expected.png: Added.
        * platform/mac/fast/repaint/dynamic-table-vertical-alignment-change-expected.txt: Added.



git-svn-id: http://svn.webkit.org/repository/webkit/trunk@40238 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent 9f6b66c7
2009-01-25 David Hyatt <hyatt@apple.com>
Add layout test for https://bugs.webkit.org/show_bug.cgi?id=23524.
Reviewed by Oliver Hunt
* fast/repaint/dynamic-table-vertical-alignment-change.html: Added.
* platform/mac/fast/repaint/dynamic-table-vertical-alignment-change-expected.checksum: Added.
* platform/mac/fast/repaint/dynamic-table-vertical-alignment-change-expected.png: Added.
* platform/mac/fast/repaint/dynamic-table-vertical-alignment-change-expected.txt: Added.
2009-01-25 Dan Bernstein <mitz@apple.com>
Reviewed by Oliver Hunt.
......
<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01//EN"
"http://www.w3.org/TR/html4/strict.dtd">
<head>
<script src="repaint.js" type="text/javascript"></script>
<script type="text/javascript">
function repaintTest()
{
document.getElementById("target").style.cssFloat = 'left';
document.getElementById("target").style.backgroundColor = 'green';
document.getElementById("target").style.width = '100px';
document.getElementById("target").style.height='100px';
}
</script>
</head>
<body onload="runRepaintTest();">
<p>
Repaint test for <i><a href="https://bugs.webkit.org/show_bug.cgi?id=23524">Bugzilla bug 23524</a></i>
Make sure that a table section doesn't stop painting when vertical alignment of cells dynamically changes.
</p>
<div style="width: 800px;">
<table>
<tr valign="top">
<td>
<div id="target"></div>
</td>
<td>
<div style="margin: 0 0 1px 0;"></div>
<div></div>
<div></div>
</td>
</tr>
</table>
</div>
\ No newline at end of file
layer at (0,0) size 808x585
RenderView at (0,0) size 800x585
layer at (0,0) size 808x182
RenderBlock {HTML} at (0,0) size 800x182
RenderBody {BODY} at (8,16) size 784x158
RenderBlock {P} at (0,0) size 784x36
RenderText {#text} at (0,0) size 100x18
text run at (0,0) width 100: "Repaint test for "
RenderInline {I} at (0,0) size 124x18
RenderInline {A} at (0,0) size 124x18 [color=#0000EE]
RenderText {#text} at (100,0) size 124x18
text run at (100,0) width 124: "Bugzilla bug 23524"
RenderText {#text} at (224,0) size 751x36
text run at (224,0) width 4: " "
text run at (228,0) width 523: "Make sure that a table section doesn't stop painting when vertical alignment of cells"
text run at (0,18) width 136: "dynamically changes."
RenderBlock {DIV} at (0,52) size 800x106
RenderTable {TABLE} at (0,0) size 110x106
RenderTableSection {TBODY} at (0,0) size 110x106
RenderTableRow {TR} at (0,2) size 110x102
RenderTableCell {TD} at (2,2) size 102x102 [r=0 c=0 rs=1 cs=1]
RenderBlock (floating) {DIV} at (1,1) size 100x100 [bgcolor=#008000]
RenderTableCell {TD} at (106,2) size 2x3 [r=0 c=1 rs=1 cs=1]
RenderBlock {DIV} at (1,1) size 0x0
RenderBlock {DIV} at (1,2) size 0x0
RenderBlock {DIV} at (1,2) size 0x0
2009-01-25 David Hyatt <hyatt@apple.com>
Fix for https://bugs.webkit.org/show_bug.cgi?id=23524, lots of missing content in table sections.
The new table code created a bug involving markAllDescendantsWithFloatsForLayout, namely that it could
end up marking ancestors of a block as needing layout when that block was still in the process of
doing a layout.
The fix is to add a parameter to markAllDescendantsWithFloatsForLayout that says whether or not
we are "mid-layout." If this flag is set, then the method will make sure to do only local dirtying
of objects to avoid accidentally marking a clean ancestor as needing layout again.
Ultimately the second parameter to setNeedsLayout and setChildNeedsLayout should just be removed,
with a check of whether or not we are mid-layout being done by those methods instead.
Reviewed by Oliver Hunt
Added fast/repaint/dynamic-table-vertical-alignment-change.html
* rendering/RenderBlock.cpp:
(WebCore::RenderBlock::collapseMargins):
(WebCore::RenderBlock::clearFloatsIfNeeded):
(WebCore::RenderBlock::layoutBlockChildren):
(WebCore::RenderBlock::markAllDescendantsWithFloatsForLayout):
* rendering/RenderBlock.h:
* rendering/RenderObject.cpp:
(WebCore::RenderObject::removeFromObjectLists):
* rendering/RenderObject.h:
* rendering/RenderTableSection.cpp:
(WebCore::RenderTableSection::layoutRows):
2009-01-25 Darin Adler <darin@apple.com>
Reviewed by Mark Rowe.
......@@ -1066,7 +1066,7 @@ void RenderBlock::collapseMargins(RenderBox* child, MarginInfo& marginInfo, int
child->setChildNeedsLayout(true, false);
if (!child->avoidsFloats() && child->containsFloats())
child->markAllDescendantsWithFloatsForLayout();
static_cast<RenderBlock*>(child)->markAllDescendantsWithFloatsForLayout();
// Our guess was wrong. Make the child lay itself out again.
child->layoutIfNeeded();
......@@ -1121,7 +1121,7 @@ void RenderBlock::clearFloatsIfNeeded(RenderBox* child, MarginInfo& marginInfo,
// So go ahead and mark the item as dirty.
child->setChildNeedsLayout(true, false);
if (!child->avoidsFloats() && child->containsFloats())
child->markAllDescendantsWithFloatsForLayout();
static_cast<RenderBlock*>(child)->markAllDescendantsWithFloatsForLayout();
child->layoutIfNeeded();
}
......@@ -1342,7 +1342,7 @@ void RenderBlock::layoutBlockChildren(bool relayoutChildren, int& maxFloatBottom
}
if (markDescendantsWithFloats)
child->markAllDescendantsWithFloatsForLayout();
static_cast<RenderBlock*>(child)->markAllDescendantsWithFloatsForLayout();
if (child->isRenderBlock())
previousFloatBottom = max(previousFloatBottom, oldRect.y() + static_cast<RenderBlock*>(child)->floatBottom());
......@@ -3052,9 +3052,9 @@ bool RenderBlock::containsFloat(RenderObject* o)
return false;
}
void RenderBlock::markAllDescendantsWithFloatsForLayout(RenderBox* floatToRemove)
void RenderBlock::markAllDescendantsWithFloatsForLayout(RenderBox* floatToRemove, bool inLayout)
{
setChildNeedsLayout(true);
setChildNeedsLayout(true, !inLayout);
if (floatToRemove)
removeFloatingObject(floatToRemove);
......@@ -3064,7 +3064,7 @@ void RenderBlock::markAllDescendantsWithFloatsForLayout(RenderBox* floatToRemove
for (RenderObject* child = firstChild(); child; child = child->nextSibling()) {
if (isBlockFlow() && !child->isFloatingOrPositioned() &&
((floatToRemove ? child->containsFloat(floatToRemove) : child->containsFloats()) || child->shrinkToAvoidFloats()))
child->markAllDescendantsWithFloatsForLayout(floatToRemove);
static_cast<RenderBlock*>(child)->markAllDescendantsWithFloatsForLayout(floatToRemove, inLayout);
}
}
}
......
......@@ -177,7 +177,7 @@ public:
bool positionNewFloats();
void clearFloats();
int getClearDelta(RenderBox* child);
virtual void markAllDescendantsWithFloatsForLayout(RenderBox* floatToRemove = 0);
void markAllDescendantsWithFloatsForLayout(RenderBox* floatToRemove = 0, bool inLayout = true);
void markPositionedObjectsForLayout();
virtual bool containsFloats() { return m_floatingObjects && !m_floatingObjects->isEmpty(); }
......
......@@ -508,10 +508,6 @@ bool RenderObject::hasStaticY() const
return (style()->top().isAuto() && style()->bottom().isAuto()) || style()->top().isStatic();
}
void RenderObject::markAllDescendantsWithFloatsForLayout(RenderBox*)
{
}
void RenderObject::setPrefWidthsDirty(bool b, bool markParents)
{
bool alreadyDirty = m_prefWidthsDirty;
......@@ -2271,7 +2267,7 @@ void RenderObject::removeFromObjectLists()
}
if (outermostBlock)
outermostBlock->markAllDescendantsWithFloatsForLayout(toRenderBox(this));
outermostBlock->markAllDescendantsWithFloatsForLayout(toRenderBox(this), false);
}
if (isPositioned()) {
......
......@@ -362,7 +362,6 @@ public:
RenderObject* container() const;
RenderObject* hoverAncestor() const;
virtual void markAllDescendantsWithFloatsForLayout(RenderBox* floatToRemove = 0);
void markContainingBlocksForLayout(bool scheduleRelayout = true, RenderObject* newRoot = 0);
void setNeedsLayout(bool b, bool markParents = true);
void setChildNeedsLayout(bool b, bool markParents = true);
......
......@@ -107,7 +107,7 @@ void RenderSVGRoot::layout()
for (RenderObject* child = firstChild(); child; child = child->nextSibling()) {
if (selfNeedsLayout()) // either bounds or transform changed, force kids to relayout
child->setNeedsLayout(true);
child->setNeedsLayout(true, false);
child->layoutIfNeeded();
ASSERT(!child->needsLayout());
......
......@@ -295,7 +295,7 @@ void RenderTable::layout()
for (RenderObject* child = firstChild(); child; child = child->nextSibling()) {
// FIXME: What about a form that has a display value that makes it a table section?
if (child->needsLayout() && !(child->element() && child->element()->hasTagName(formTag)))
if (child->needsLayout() && !(child->element() && child->element()->hasTagName(formTag) && !child->isTableSection()))
child->layout();
if (child->isTableSection()) {
RenderTableSection* section = static_cast<RenderTableSection*>(child);
......
......@@ -587,6 +587,8 @@ int RenderTableSection::layoutRows(int toAdd)
}
}
ASSERT(!needsLayout());
statePusher.pop();
setHeight(m_rowPos[totalRows]);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment