Title: [133717] trunk
- Revision
- 133717
- Author
- [email protected]
- Date
- 2012-11-06 20:55:23 -0800 (Tue, 06 Nov 2012)
Log Message
Crash due to column span under button element
https://bugs.webkit.org/show_bug.cgi?id=101402
Reviewed by Abhishek Arya.
Source/WebCore:
When there is a column-spanning child of a RenderButton
splitBlocks() must split the RenderButton as well as its
only permitted direct child, the anonymous block referenced
by m_inner. A crash was occurring because splitBlocks()
calls addChildIgnoringAnonymousColumnBlocks() to add the
cloned m_inner to the cloned RenderButton, which meant the
m_inner for the cloned RenderButton was not being set even
though a child was being added. This violates state
assumptions in the RenderButton code.
This patch prevents any descendants of RenderButton from
spanning columns. Also, it adds a precautionary check in
RenderButton::removeChild() to mitigate problems if similar
state problems are found in future.
* rendering/RenderBlock.cpp:
(WebCore::RenderBlock::containingColumnsBlock):
* rendering/RenderButton.cpp:
(WebCore::RenderButton::removeChild):
LayoutTests:
Test creates crashing condition for bug 101402.
* fast/block/colspan-under-button-crash.html: Added.
* fast/block/colspan-under-button-crash-expected.txt: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (133716 => 133717)
--- trunk/LayoutTests/ChangeLog 2012-11-07 04:50:50 UTC (rev 133716)
+++ trunk/LayoutTests/ChangeLog 2012-11-07 04:55:23 UTC (rev 133717)
@@ -1,3 +1,15 @@
+2012-11-06 Ken Buchanan <[email protected]>
+
+ Crash due to column span under button element
+ https://bugs.webkit.org/show_bug.cgi?id=101402
+
+ Reviewed by Abhishek Arya.
+
+ Test creates crashing condition for bug 101402.
+
+ * fast/block/colspan-under-button-crash.html: Added.
+ * fast/block/colspan-under-button-crash-expected.txt: Added.
+
2012-11-06 Jaehun Lim <[email protected]>
[EFL][WK2] fast/dom/shadow/shadowdom-for-object-only-shadow.html fails
Added: trunk/LayoutTests/fast/block/colspan-under-button-crash-expected.txt (0 => 133717)
--- trunk/LayoutTests/fast/block/colspan-under-button-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/block/colspan-under-button-crash-expected.txt 2012-11-07 04:55:23 UTC (rev 133717)
@@ -0,0 +1 @@
+PASS if no crash or assert in debug
Added: trunk/LayoutTests/fast/block/colspan-under-button-crash.html (0 => 133717)
--- trunk/LayoutTests/fast/block/colspan-under-button-crash.html (rev 0)
+++ trunk/LayoutTests/fast/block/colspan-under-button-crash.html 2012-11-07 04:55:23 UTC (rev 133717)
@@ -0,0 +1,34 @@
+<!DOCTYPE html>
+<html>
+<style>
+.c0 { display: inherit; }
+.c10 { display: table-column-group; -webkit-column-span: all; }
+.c11[class$="c11"] { vertical-align: -10; -webkit-column-width: 1px; }
+.c17 { overflow: hidden; position: fixed; }
+.c17::after { position: inherit; content: no-close-quote;</style>
+<script>
+window._onload_ = function() {
+ pElem = document.createElement('p');
+ pElem.setAttribute('class', 'c11');
+
+ citeElem = document.createElement('cite');
+ citeElem.setAttribute('class', 'c10');
+
+ buttonElem = document.createElement('button');
+ buttonElem.setAttribute('class', 'c0');
+
+ document.documentElement.appendChild(pElem);
+ pElem.appendChild(buttonElem);
+ buttonElem.appendChild(citeElem);
+
+ document.documentElement.offsetHeight;
+ buttonElem.setAttribute('class', 'c17');
+ document.documentElement.offsetHeight;
+ document.documentElement.removeChild(pElem);
+
+ document.body.innerHTML = "PASS if no crash or assert in debug";
+ if (window.testRunner)
+ testRunner.dumpAsText();
+}
+</script>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (133716 => 133717)
--- trunk/Source/WebCore/ChangeLog 2012-11-07 04:50:50 UTC (rev 133716)
+++ trunk/Source/WebCore/ChangeLog 2012-11-07 04:55:23 UTC (rev 133717)
@@ -1,3 +1,30 @@
+2012-11-06 Ken Buchanan <[email protected]>
+
+ Crash due to column span under button element
+ https://bugs.webkit.org/show_bug.cgi?id=101402
+
+ Reviewed by Abhishek Arya.
+
+ When there is a column-spanning child of a RenderButton
+ splitBlocks() must split the RenderButton as well as its
+ only permitted direct child, the anonymous block referenced
+ by m_inner. A crash was occurring because splitBlocks()
+ calls addChildIgnoringAnonymousColumnBlocks() to add the
+ cloned m_inner to the cloned RenderButton, which meant the
+ m_inner for the cloned RenderButton was not being set even
+ though a child was being added. This violates state
+ assumptions in the RenderButton code.
+
+ This patch prevents any descendants of RenderButton from
+ spanning columns. Also, it adds a precautionary check in
+ RenderButton::removeChild() to mitigate problems if similar
+ state problems are found in future.
+
+ * rendering/RenderBlock.cpp:
+ (WebCore::RenderBlock::containingColumnsBlock):
+ * rendering/RenderButton.cpp:
+ (WebCore::RenderButton::removeChild):
+
2012-11-06 Shinya Kawanaka <[email protected]>
[Shadow] Pseudo custom-elements should start with 'x-'.
Modified: trunk/Source/WebCore/rendering/RenderBlock.cpp (133716 => 133717)
--- trunk/Source/WebCore/rendering/RenderBlock.cpp 2012-11-07 04:50:50 UTC (rev 133716)
+++ trunk/Source/WebCore/rendering/RenderBlock.cpp 2012-11-07 04:55:23 UTC (rev 133717)
@@ -533,7 +533,10 @@
// FIXME: Table manages its own table parts, most of which are RenderBoxes.
// Multi-column code cannot handle splitting the flow in table. Disabling it
// to prevent crashes.
- if (curr->isTable())
+ // Similarly, RenderButton maintains an anonymous block child and overrides
+ // addChild() to prevent itself from having additional direct children. This
+ // causes problems for split flows.
+ if (curr->isTable() || curr->isRenderButton())
return 0;
RenderBlock* currBlock = toRenderBlock(curr);
Modified: trunk/Source/WebCore/rendering/RenderButton.cpp (133716 => 133717)
--- trunk/Source/WebCore/rendering/RenderButton.cpp 2012-11-07 04:50:50 UTC (rev 133716)
+++ trunk/Source/WebCore/rendering/RenderButton.cpp 2012-11-07 04:55:23 UTC (rev 133717)
@@ -60,7 +60,11 @@
void RenderButton::removeChild(RenderObject* oldChild)
{
- if (oldChild == m_inner || !m_inner) {
+ // m_inner should be the only child, but checking for direct children who
+ // are not m_inner prevents security problems when that assumption is
+ // violated.
+ if (oldChild == m_inner || !m_inner || oldChild->parent() == this) {
+ ASSERT(oldChild == m_inner || !m_inner);
RenderDeprecatedFlexibleBox::removeChild(oldChild);
m_inner = 0;
} else
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo/webkit-changes
