Commit 4855c4f9 authored by justing's avatar justing

WebCore:

        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: http://svn.webkit.org/repository/webkit/trunk@25203 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent da7897e2
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.
<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>
e334e5a84a78ed37798ad6b85aac99e2
\ No newline at end of file
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.
......@@ -405,12 +405,12 @@ void DeleteSelectionCommand::handleGeneralDelete()
}
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 {
......@@ -420,10 +420,10 @@ void DeleteSelectionCommand::handleGeneralDelete()
// 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
......@@ -431,12 +431,12 @@ void DeleteSelectionCommand::handleGeneralDelete()
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();
......
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