Reviewed by Adele.
<rdar://problem/
5156801> REGRESSION: Crash at DeleteSelectionCommand::doApply() when deleting table content
* editing/DeleteSelectionCommand.cpp:
(WebCore::DeleteSelectionCommand::handleGeneralDelete): Use a RefPtr
for node. If the node to be removed contains the selection, and if
the next node to be removed (nextNode) is inside the deletion UI,
removing node will remove nextNode from the document. nextNode is
a RefPtr, but node isn't and when nextNode falls out of scope the node
that node points to will be destroyed and we'll end up using a stale pointer.
Long term we should probably just disable the deletion UI before editing
operations because the undo of the removal of node in the situation
described above relies on the presence of the deletion UI, but it isn't
present because its added and removed in a non-undoable way.
LayoutTests:
Reviewed by Adele.
<rdar://problem/
5156801> REGRESSION: Crash at DeleteSelectionCommand::doApply() when deleting table content
* editing/deleting/
5156801-2.html: Added.
* platform/mac/editing/deleting: Added.
* platform/mac/editing/deleting/
5156801-2-expected.checksum: Added.
* platform/mac/editing/deleting/
5156801-2-expected.png: Added.
* platform/mac/editing/deleting/
5156801-2-expected.txt: Added.
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@25203
268f45cc-cd09-0410-ab3c-
d52691b4dbfc
+2007-08-23 Justin Garcia <justin.garcia@apple.com>
+
+ Reviewed by Adele.
+
+ <rdar://problem/5156801> REGRESSION: Crash at DeleteSelectionCommand::doApply() when deleting table content
+
+ * editing/deleting/5156801-2.html: Added.
+ * platform/mac/editing/deleting: Added.
+ * platform/mac/editing/deleting/5156801-2-expected.checksum: Added.
+ * platform/mac/editing/deleting/5156801-2-expected.png: Added.
+ * platform/mac/editing/deleting/5156801-2-expected.txt: Added.
+
2007-08-22 Justin Garcia <justin.garcia@apple.com>
Reviewed by Adam.
--- /dev/null
+<body>
+<p>This tests for a crash when deleting the contents of a table cell. You should just see 'Cached' below.</p>
+<div contenteditable="true"><table id="table" border=0 cellpadding=0 cellspacing=0><tr><td><br><a href="fakelink.html">Cached</a><a href=""fakelink.html">Similar</a></td></tr></table>
+</div>
+<script>
+s = window.getSelection();
+table = document.getElementById("table");
+s.setPosition(table, table.childNodes.length);
+s.modify("move", "backward", "character");
+document.execCommand("Delete");
+document.execCommand("Delete");
+document.execCommand("Delete");
+document.execCommand("Delete");
+document.execCommand("Delete");
+document.execCommand("Delete");
+document.execCommand("Delete");
+</script>
+</body>
--- /dev/null
+e334e5a84a78ed37798ad6b85aac99e2
\ No newline at end of file
--- /dev/null
+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
+ RenderBlock {P} at (0,0) size 784x18
+ RenderText {#text} at (0,0) size 626x18
+ text run at (0,0) width 626: "This tests for a crash when deleting the contents of a table cell. You should just see 'Cached' below."
+ RenderBlock {DIV} at (0,34) size 784x36
+ RenderTable {TABLE} at (0,0) size 48x36
+ RenderTableSection {TBODY} at (0,0) size 48x36
+ RenderTableRow {TR} at (0,0) size 48x36
+ RenderTableCell {TD} at (0,0) size 48x36 [r=0 c=0 rs=1 cs=1]
+ RenderBR {BR} at (0,0) size 0x18
+ RenderInline {A} at (0,0) size 48x18 [color=#0000EE]
+ RenderText {#text} at (0,18) size 48x18
+ text run at (0,18) width 48: "Cached"
+caret: position 6 of child 0 {#text} of child 1 {A} of child 0 {TD} of child 0 {TR} of child 0 {TBODY} of child 0 {TABLE} of child 3 {DIV} of child 0 {BODY} of child 0 {HTML} of document
+2007-08-23 Justin Garcia <justin.garcia@apple.com>
+
+ Reviewed by Adele.
+
+ <rdar://problem/5156801> REGRESSION: Crash at DeleteSelectionCommand::doApply() when deleting table content
+
+ * editing/DeleteSelectionCommand.cpp:
+ (WebCore::DeleteSelectionCommand::handleGeneralDelete): Use a RefPtr
+ for node. If the node to be removed contains the selection, and if
+ the next node to be removed (nextNode) is inside the deletion UI,
+ removing node will remove nextNode from the document. nextNode is
+ a RefPtr, but node isn't and when nextNode falls out of scope the node
+ that node points to will be destroyed and we'll end up using a stale pointer.
+ Long term we should probably just disable the deletion UI before editing
+ operations because the undo of the removal of node in the situation
+ described above relies on the presence of the deletion UI, but it isn't
+ present because its added and removed in a non-undoable way.
+
2007-08-23 Mitz Pettel <mitz@webkit.org>
Reviewed by Darin.
}
else {
// The selection to delete spans more than one node.
- Node *node = startNode;
+ RefPtr<Node> node = startNode;
if (startOffset > 0) {
if (startNode->isTextNode()) {
// in a text node that needs to be trimmed
- Text *text = static_cast<Text *>(node);
+ Text *text = static_cast<Text *>(node.get());
deleteTextFromNode(text, startOffset, text->length() - startOffset);
node = node->traverseNextNode();
} else {
// handle deleting all nodes that are completely selected
while (node && node != m_downstreamEnd.node()) {
- if (Range::compareBoundaryPoints(Position(node, 0), m_downstreamEnd) >= 0) {
+ if (Range::compareBoundaryPoints(Position(node.get(), 0), m_downstreamEnd) >= 0) {
// traverseNextSibling just blew past the end position, so stop deleting
node = 0;
- } else if (!m_downstreamEnd.node()->isDescendantOf(node)) {
+ } else if (!m_downstreamEnd.node()->isDescendantOf(node.get())) {
RefPtr<Node> nextNode = node->traverseNextSibling();
// if we just removed a node from the end container, update end position so the
// check above will work
ASSERT(node->nodeIndex() < (unsigned)m_downstreamEnd.offset());
m_downstreamEnd = Position(m_downstreamEnd.node(), m_downstreamEnd.offset() - 1);
}
- removeNode(node);
+ removeNode(node.get());
node = nextNode.get();
} else {
Node* n = node->lastDescendant();
if (m_downstreamEnd.node() == n && m_downstreamEnd.offset() >= n->caretMaxOffset()) {
- removeNode(node);
+ removeNode(node.get());
node = 0;
} else
node = node->traverseNextNode();