Changeset 41645 in webkit


Ignore:
Timestamp:
Mar 12, 2009 1:29:30 PM (15 years ago)
Author:
eric@webkit.org
Message:

Reviewed by Darin Fisher and Justin Garcia.

Safari crashes during drag and drop in Google presentations
due to mutation event handlers removing DOM content during insertNode
https://bugs.webkit.org/show_bug.cgi?id=22634

Added a bunch of "null" checks to make sure nodes are still
in the document before we operate on them. This is an
inelegant solution, but it's the best we have for now.

Test: editing/selection/crash-on-drag-with-mutation-events.html

  • editing/CompositeEditCommand.cpp: (WebCore::CompositeEditCommand::insertNodeAt):
  • editing/ReplaceSelectionCommand.cpp: (WebCore::ReplaceSelectionCommand::doApply):
Location:
trunk
Files:
2 added
4 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r41644 r41645  
     12009-03-02  Eric Seidel  <eric@webkit.org>
     2
     3        Reviewed by Darin Fisher and Justin Garcia.
     4
     5        Safari crashes during drag and drop in Google presentations
     6        due to mutation event handlers removing DOM content during insertNode
     7        https://bugs.webkit.org/show_bug.cgi?id=22634
     8
     9        * editing/selection/crash-on-drag-with-mutation-events-expected.txt: Added.
     10        * editing/selection/crash-on-drag-with-mutation-events.html: Added.
     11
    1122009-03-12  Simon Fraser  <simon.fraser@apple.com>
    213
  • trunk/WebCore/ChangeLog

    r41644 r41645  
     12009-03-02  Eric Seidel  <eric@webkit.org>
     2
     3        Reviewed by Darin Fisher and Justin Garcia.
     4
     5        Safari crashes during drag and drop in Google presentations
     6        due to mutation event handlers removing DOM content during insertNode
     7        https://bugs.webkit.org/show_bug.cgi?id=22634
     8
     9        Added a bunch of "null" checks to make sure nodes are still
     10        in the document before we operate on them.  This is an
     11        inelegant solution, but it's the best we have for now.
     12       
     13        Test: editing/selection/crash-on-drag-with-mutation-events.html
     14
     15        * editing/CompositeEditCommand.cpp:
     16        (WebCore::CompositeEditCommand::insertNodeAt):
     17        * editing/ReplaceSelectionCommand.cpp:
     18        (WebCore::ReplaceSelectionCommand::doApply):
     19
    1202009-03-12  Dimitri Glazkov  <dglazkov@chromium.org>
    221
  • trunk/WebCore/editing/CompositeEditCommand.cpp

    r41553 r41645  
    172172    else if (refChild->isTextNode() && caretMaxOffset(refChild) > offset) {
    173173        splitTextNode(static_cast<Text *>(refChild), offset);
     174
     175        // Mutation events (bug 22634) from the text node insertion may have removed the refChild
     176        if (!refChild->inDocument())
     177            return;
    174178        insertNodeBefore(insertChild, refChild);
    175179    } else
  • trunk/WebCore/editing/ReplaceSelectionCommand.cpp

    r41553 r41645  
    840840    fragment.removeNode(refNode);
    841841    insertNodeAtAndUpdateNodesInserted(refNode, insertionPos);
    842    
     842
     843    // Mutation events (bug 22634) may have already removed the inserted content
     844    if (!refNode->inDocument())
     845        return;
     846
    843847    while (node) {
    844848        Node* next = node->nextSibling();
    845849        fragment.removeNode(node);
    846850        insertNodeAfterAndUpdateNodesInserted(node, refNode.get());
     851
     852        // Mutation events (bug 22634) may have already removed the inserted content
     853        if (!node->inDocument())
     854            return;
     855
    847856        refNode = node;
    848857        node = next;
     
    897906        // comes after and prevent that from happening.
    898907        VisiblePosition endOfInsertedContent = positionAtEndOfInsertedContent();
    899         if (startOfParagraph(endOfInsertedContent) == startOfParagraphToMove)
     908        if (startOfParagraph(endOfInsertedContent) == startOfParagraphToMove) {
    900909            insertNodeAt(createBreakElement(document()).get(), endOfInsertedContent.deepEquivalent());
    901        
     910            // Mutation events (bug 22634) triggered by inserting the <br> might have removed the content we're about to move
     911            if (!startOfParagraphToMove.deepEquivalent().node()->inDocument())
     912                return;
     913        }
     914
    902915        // FIXME: Maintain positions for the start and end of inserted content instead of keeping nodes.  The nodes are
    903916        // only ever used to create positions where inserted content starts/ends.
Note: See TracChangeset for help on using the changeset viewer.