LayoutTests:
Reviewed by Hyatt.
- test for <rdar://problem/4928671> Crash due to runaway recursion when fieldset has display: table-row (12066)
* fast/css/fieldset-display-row-expected.checksum: Added.
* fast/css/fieldset-display-row-expected.png: Added.
* fast/css/fieldset-display-row-expected.txt: Added.
* fast/css/fieldset-display-row.html: Added.
WebCore:
Reviewed by Hyatt.
- fix <rdar://problem/4928671> Crash due to runaway recursion when fieldset has display: table-row (12066)
Test: fast/css/fieldset-display-row.html
* rendering/RenderContainer.cpp: (WebCore::RenderContainer::addChild):
Change all decision making about special handling for new children to use
the class of the render object instead of the style, except for the cases
where the style is the only way to tell (TABLE_COLUMN_GROUP and TABLE_CAPTION).
Thus a RenderFieldset with table-row style won't get handled as if it was
a table row.
* rendering/RenderTable.cpp: (WebCore::RenderTable::addChild): Same thing.
Except that here the styles that we ahave to look at are TABLE_CAPTION,
TABLE_HEADER_GROUP, TABLE_FOOTER_GROUP, and TABLE_ROW_GROUP.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@21520 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 8497a4b..b350c7e 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,14 @@
+2007-05-16 Darin Adler <darin@apple.com>
+
+ Reviewed by Hyatt.
+
+ - test for <rdar://problem/4928671> Crash due to runaway recursion when fieldset has display: table-row (12066)
+
+ * fast/css/fieldset-display-row-expected.checksum: Added.
+ * fast/css/fieldset-display-row-expected.png: Added.
+ * fast/css/fieldset-display-row-expected.txt: Added.
+ * fast/css/fieldset-display-row.html: Added.
+
2007-05-15 Sam Weinig <sam@webkit.org>
Reviewed by Oliver.
diff --git a/LayoutTests/fast/css/fieldset-display-row-expected.checksum b/LayoutTests/fast/css/fieldset-display-row-expected.checksum
new file mode 100644
index 0000000..596adbf
--- /dev/null
+++ b/LayoutTests/fast/css/fieldset-display-row-expected.checksum
@@ -0,0 +1 @@
+2afe0d17088030287d65727c1e80fc95
\ No newline at end of file
diff --git a/LayoutTests/fast/css/fieldset-display-row-expected.png b/LayoutTests/fast/css/fieldset-display-row-expected.png
new file mode 100644
index 0000000..62635f8
--- /dev/null
+++ b/LayoutTests/fast/css/fieldset-display-row-expected.png
Binary files differ
diff --git a/LayoutTests/fast/css/fieldset-display-row-expected.txt b/LayoutTests/fast/css/fieldset-display-row-expected.txt
new file mode 100644
index 0000000..d4437cf
--- /dev/null
+++ b/LayoutTests/fast/css/fieldset-display-row-expected.txt
@@ -0,0 +1,8 @@
+layer at (0,0) size 800x600
+ RenderView at (0,0) size 800x600
+layer at (0,0) size 800x600
+ RenderBlock {HTML} at (0,0) size 800x600
+ RenderBody {BODY} at (8,8) size 784x584
+ RenderFieldSet {FIELDSET} at (2,0) size 780x37 [border: (2px groove #C0C0C0)]
+ RenderText {#text} at (14,7) size 375x18
+ text run at (14,7) width 375: "If you can see this fieldset without crashing, then all is well."
diff --git a/LayoutTests/fast/css/fieldset-display-row.html b/LayoutTests/fast/css/fieldset-display-row.html
new file mode 100644
index 0000000..7ef5cff
--- /dev/null
+++ b/LayoutTests/fast/css/fieldset-display-row.html
@@ -0,0 +1,11 @@
+<html>
+<head>
+ <title>Test HTML Page</title>
+ <style type="text/css">
+ fieldset { display: table-row; }
+ </style>
+</head>
+<body>
+ <fieldset>If you can see this fieldset without crashing, then all is well.</fieldset>
+</body>
+</html>
diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index 9cc0772..9d9aa47 100644
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,22 @@
+2007-05-16 Darin Adler <darin@apple.com>
+
+ Reviewed by Hyatt.
+
+ - fix <rdar://problem/4928671> Crash due to runaway recursion when fieldset has display: table-row (12066)
+
+ Test: fast/css/fieldset-display-row.html
+
+ * rendering/RenderContainer.cpp: (WebCore::RenderContainer::addChild):
+ Change all decision making about special handling for new children to use
+ the class of the render object instead of the style, except for the cases
+ where the style is the only way to tell (TABLE_COLUMN_GROUP and TABLE_CAPTION).
+ Thus a RenderFieldset with table-row style won't get handled as if it was
+ a table row.
+
+ * rendering/RenderTable.cpp: (WebCore::RenderTable::addChild): Same thing.
+ Except that here the styles that we ahave to look at are TABLE_CAPTION,
+ TABLE_HEADER_GROUP, TABLE_FOOTER_GROUP, and TABLE_ROW_GROUP.
+
2007-05-16 Zack Rusin <zrusin@trolltech.com>
Reviewed by Lars.
diff --git a/WebCore/rendering/RenderContainer.cpp b/WebCore/rendering/RenderContainer.cpp
index ad24b10..5042681 100644
--- a/WebCore/rendering/RenderContainer.cpp
+++ b/WebCore/rendering/RenderContainer.cpp
@@ -86,47 +86,24 @@
{
bool needsTable = false;
- if(!newChild->isText() && !newChild->isReplaced()) {
- switch(newChild->style()->display()) {
- case LIST_ITEM:
- updateListMarkerNumbers(beforeChild ? beforeChild : m_lastChild);
- break;
- case INLINE:
- case BLOCK:
- case INLINE_BLOCK:
- case RUN_IN:
- case COMPACT:
- case BOX:
- case INLINE_BOX:
- case TABLE:
- case INLINE_TABLE:
- case TABLE_COLUMN:
- break;
- case TABLE_COLUMN_GROUP:
- case TABLE_CAPTION:
- case TABLE_ROW_GROUP:
- case TABLE_HEADER_GROUP:
- case TABLE_FOOTER_GROUP:
- if (!isTable())
- needsTable = true;
- break;
- case TABLE_ROW:
- if (!isTableSection())
- needsTable = true;
- break;
- case TABLE_CELL:
- if (!isTableRow())
- needsTable = true;
- // I'm not 100% sure this is the best way to fix this, but without this
- // change we recurse infinitely when trying to render the CSS2 test page:
- // http://www.bath.ac.uk/%7Epy8ieh/internet/eviltests/htmlbodyheadrendering2.html.
- // See Radar 2925291.
- if (isTableCell() && !m_firstChild && !newChild->isTableCell())
- needsTable = false;
- break;
- case NONE:
- break;
- }
+ if (newChild->isListItem())
+ updateListMarkerNumbers(beforeChild ? beforeChild : m_lastChild);
+ else if (newChild->isTableCol() && newChild->style()->display() == TABLE_COLUMN_GROUP)
+ needsTable = !isTable();
+ else if (newChild->isRenderBlock() && newChild->style()->display() == TABLE_CAPTION)
+ needsTable = !isTable();
+ else if (newChild->isTableSection())
+ needsTable = !isTable();
+ else if (newChild->isTableRow())
+ needsTable = !isTableSection();
+ else if (newChild->isTableCell()) {
+ needsTable = !isTableRow();
+ // I'm not 100% sure this is the best way to fix this, but without this
+ // change we recurse infinitely when trying to render the CSS2 test page:
+ // http://www.bath.ac.uk/%7Epy8ieh/internet/eviltests/htmlbodyheadrendering2.html.
+ // See Radar 2925291.
+ if (needsTable && isTableCell() && !m_firstChild && !newChild->isTableCell())
+ needsTable = false;
}
if (needsTable) {
diff --git a/WebCore/rendering/RenderTable.cpp b/WebCore/rendering/RenderTable.cpp
index ee942c7..09a02d2 100644
--- a/WebCore/rendering/RenderTable.cpp
+++ b/WebCore/rendering/RenderTable.cpp
@@ -114,81 +114,67 @@
bool wrapInAnonymousSection = true;
bool isTableElement = element() && element()->hasTagName(tableTag);
- switch (child->style()->display()) {
- case TABLE_CAPTION:
- if (child->isRenderBlock()) {
- // First caption wins.
- if (beforeChild && m_caption) {
- RenderObject* o = beforeChild->previousSibling();
- while (o && o != m_caption)
- o = o->previousSibling();
- if (!o)
- m_caption = 0;
+ if (child->isRenderBlock() && child->style()->display() == TABLE_CAPTION) {
+ // First caption wins.
+ if (beforeChild && m_caption) {
+ RenderObject* o = beforeChild->previousSibling();
+ while (o && o != m_caption)
+ o = o->previousSibling();
+ if (!o)
+ m_caption = 0;
+ }
+ if (!m_caption)
+ m_caption = static_cast<RenderBlock*>(child);
+ wrapInAnonymousSection = false;
+ } else if (child->isTableCol()) {
+ m_hasColElements = true;
+ wrapInAnonymousSection = false;
+ } else if (child->isTableSection()) {
+ switch (child->style()->display()) {
+ case TABLE_HEADER_GROUP:
+ if (child->isTableSection()) {
+ resetSectionPointerIfNotBefore(m_head, beforeChild);
+ if (!m_head) {
+ m_head = static_cast<RenderTableSection*>(child);
+ } else {
+ resetSectionPointerIfNotBefore(m_firstBody, beforeChild);
+ if (!m_firstBody)
+ m_firstBody = static_cast<RenderTableSection*>(child);
+ }
}
- if (!m_caption)
- m_caption = static_cast<RenderBlock*>(child);
- }
- wrapInAnonymousSection = false;
- break;
- case TABLE_COLUMN:
- case TABLE_COLUMN_GROUP:
- m_hasColElements = true;
- wrapInAnonymousSection = false;
- break;
- case TABLE_HEADER_GROUP:
- if (child->isTableSection()) {
- resetSectionPointerIfNotBefore(m_head, beforeChild);
- if (!m_head) {
- m_head = static_cast<RenderTableSection*>(child);
- } else {
+ wrapInAnonymousSection = false;
+ break;
+ case TABLE_FOOTER_GROUP:
+ if (child->isTableSection()) {
+ resetSectionPointerIfNotBefore(m_foot, beforeChild);
+ if (!m_foot) {
+ m_foot = static_cast<RenderTableSection*>(child);
+ wrapInAnonymousSection = false;
+ break;
+ }
+ }
+ // Fall through.
+ case TABLE_ROW_GROUP:
+ if (child->isTableSection()) {
resetSectionPointerIfNotBefore(m_firstBody, beforeChild);
- if (!m_firstBody)
+ if (!m_firstBody)
m_firstBody = static_cast<RenderTableSection*>(child);
}
- }
- wrapInAnonymousSection = false;
- break;
- case TABLE_FOOTER_GROUP:
- if (child->isTableSection()) {
- resetSectionPointerIfNotBefore(m_foot, beforeChild);
- if (!m_foot) {
- m_foot = static_cast<RenderTableSection*>(child);
- wrapInAnonymousSection = false;
- break;
- }
- }
- // Fall through.
- case TABLE_ROW_GROUP:
- if (child->isTableSection()) {
- resetSectionPointerIfNotBefore(m_firstBody, beforeChild);
- if (!m_firstBody)
- m_firstBody = static_cast<RenderTableSection*>(child);
- }
- wrapInAnonymousSection = false;
- break;
- case TABLE_CELL:
- case TABLE_ROW:
- wrapInAnonymousSection = true;
- break;
- case BLOCK:
- case BOX:
- case COMPACT:
- case INLINE:
- case INLINE_BLOCK:
- case INLINE_BOX:
- case INLINE_TABLE:
- case LIST_ITEM:
- case NONE:
- case RUN_IN:
- case TABLE:
- // Allow a form to just sit at the top level.
- wrapInAnonymousSection = !isTableElement || !child->element() || !(child->element()->hasTagName(formTag) && document()->isHTMLDocument());
-
- // FIXME: Allow the delete button container element to sit at the top level. This is needed until http://bugs.webkit.org/show_bug.cgi?id=11363 is fixed.
- if (wrapInAnonymousSection && child->element() && child->element()->isHTMLElement() && static_cast<HTMLElement*>(child->element())->id() == DeleteButtonController::containerElementIdentifier)
wrapInAnonymousSection = false;
- break;
+ break;
+ default:
+ ASSERT_NOT_REACHED();
}
+ } else if (child->isTableCell() || child->isTableRow()) {
+ wrapInAnonymousSection = true;
+ } else {
+ // Allow a form to just sit at the top level.
+ wrapInAnonymousSection = !isTableElement || !child->element() || !(child->element()->hasTagName(formTag) && document()->isHTMLDocument());
+
+ // FIXME: Allow the delete button container element to sit at the top level. This is needed until http://bugs.webkit.org/show_bug.cgi?id=11363 is fixed.
+ if (wrapInAnonymousSection && child->element() && child->element()->isHTMLElement() && static_cast<HTMLElement*>(child->element())->id() == DeleteButtonController::containerElementIdentifier)
+ wrapInAnonymousSection = false;
+ }
if (!wrapInAnonymousSection) {
// If the next renderer is actually wrapped in an anonymous table section, we need to go up and find that.
