Changeset 66032 in webkit


Ignore:
Timestamp:
Aug 25, 2010 12:19:26 PM (14 years ago)
Author:
rniwa@webkit.org
Message:

2010-08-25 Ryosuke Niwa <​rniwa@webkit.org>

Reviewed by Darin Adler.

Various designmode=&quot;on&quot;/&quot;off&quot; &amp; execCommand(&quot;Undo&quot;) NULL pointer crashes
​https://bugs.webkit.org/show_bug.cgi?id=32823

The bug was caused by changeSelectionAfterCommand which updates the selection
without checking the whether new selection is valid or not.

Fixed changeSelectionAfterCommand so that it won't update the selection
when either end of the new selection is orphaned. Also fixed various editing commands
to exit early if either end of the selection is orphaned.

Tests: editing/undo/orphaned-selection-crash-bug32823-1.html

editing/undo/orphaned-selection-crash-bug32823-2.html
editing/undo/orphaned-selection-crash-bug32823-3.html
editing/undo/orphaned-selection-crash-bug32823-4.html

  • editing/Editor.cpp: (WebCore::Editor::changeSelectionAfterCommand): No longer sets orphaned selection.
  • editing/VisibleSelection.h: (WebCore::VisibleSelection::isNonOrphanedRange): Added. (WebCore::VisibleSelection::isNonOrphanedCaretOrRange): Added.
  • editing/DeleteSelectionCommand.cpp: (WebCore::DeleteSelectionCommand::doApply): Added an early exist. See above.
  • editing/FormatBlockCommand.cpp: (WebCore::FormatBlockCommand::doApply): Ditto.
  • editing/IndentOutdentCommand.cpp: (WebCore::IndentOutdentCommand::doApply): Ditto.
  • editing/InsertLineBreakCommand.cpp: (WebCore::InsertLineBreakCommand::doApply): Ditto.
  • editing/InsertListCommand.cpp: (WebCore::InsertListCommand::doApply): Ditto.
  • editing/InsertParagraphSeparatorCommand.cpp: (WebCore::InsertParagraphSeparatorCommand::doApply): Ditto.
  • editing/InsertTextCommand.cpp: (WebCore::InsertTextCommand::input): Ditto.
  • editing/MoveSelectionCommand.cpp: (WebCore::MoveSelectionCommand::doApply): Ditto.
  • editing/RemoveFormatCommand.cpp: (WebCore::RemoveFormatCommand::doApply): Ditto.
  • editing/ReplaceSelectionCommand.cpp: (WebCore::ReplaceSelectionCommand::doApply): Ditto.
  • editing/TypingCommand.cpp: (WebCore::TypingCommand::doApply): Ditto.
  • editing/UnlinkCommand.cpp: (WebCore::UnlinkCommand::doApply): Ditto.

2010-08-25 Ryosuke Niwa <​rniwa@webkit.org>

Reviewed by Darin Adler.

Various designmode="on"/"off" & execCommand("Undo") NULL pointer crashes
​https://bugs.webkit.org/show_bug.cgi?id=32823

These tests ensure WebKit doesn't crash when undoing some editing commands failed
and either end of endingSelection() became orphaned.
All tests are copied from the bug to prevent regression.

  • editing/undo/orphaned-selection-crash-bug32823-1-expected.txt: Added.
  • editing/undo/orphaned-selection-crash-bug32823-1.html: Added.
  • editing/undo/orphaned-selection-crash-bug32823-2-expected.txt: Added.
  • editing/undo/orphaned-selection-crash-bug32823-2.html: Added.
  • editing/undo/orphaned-selection-crash-bug32823-3-expected.txt: Added.
  • editing/undo/orphaned-selection-crash-bug32823-3.html: Added.
  • editing/undo/orphaned-selection-crash-bug32823-4-expected.txt: Added.
  • editing/undo/orphaned-selection-crash-bug32823-4.html: Added.
  • editing/undo/redo-split-text-with-removal-expected.txt: Caret is restored.
Location:
trunk
Files:
8 added
18 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r66029 r66032  
     12010-08-25  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        Reviewed by Darin Adler.
     4
     5        Various designmode="on"/"off" & execCommand("Undo") NULL pointer crashes
     6        https://bugs.webkit.org/show_bug.cgi?id=32823
     7
     8        These tests ensure WebKit doesn't crash when undoing some editing commands failed
     9        and either end of endingSelection() became orphaned.
     10        All tests are copied from the bug to prevent regression.
     11
     12        * editing/undo/orphaned-selection-crash-bug32823-1-expected.txt: Added.
     13        * editing/undo/orphaned-selection-crash-bug32823-1.html: Added.
     14        * editing/undo/orphaned-selection-crash-bug32823-2-expected.txt: Added.
     15        * editing/undo/orphaned-selection-crash-bug32823-2.html: Added.
     16        * editing/undo/orphaned-selection-crash-bug32823-3-expected.txt: Added.
     17        * editing/undo/orphaned-selection-crash-bug32823-3.html: Added.
     18        * editing/undo/orphaned-selection-crash-bug32823-4-expected.txt: Added.
     19        * editing/undo/orphaned-selection-crash-bug32823-4.html: Added.
     20        * editing/undo/redo-split-text-with-removal-expected.txt: Caret is restored.
     21
    1222010-08-25  Ojan Vafai  <ojan@chromium.org>
    223

  • trunk/LayoutTests/editing/undo/redo-split-text-with-removal-expected.txt

    r64303 r66032  
    1616after redo:
    1717| <div>
    18 |   "hello"
     18|   "<#selection-caret>hello"

  • trunk/WebCore/ChangeLog

    r66028 r66032  
     12010-08-25  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        Reviewed by Darin Adler.
     4
     5        Various designmode=&quot;on&quot;/&quot;off&quot; &amp; execCommand(&quot;Undo&quot;) NULL pointer crashes
     6        https://bugs.webkit.org/show_bug.cgi?id=32823
     7
     8        The bug was caused by changeSelectionAfterCommand which updates the selection
     9        without checking the whether new selection is valid or not.
     10
     11        Fixed changeSelectionAfterCommand so that it won't update the selection
     12        when either end of the new selection is orphaned. Also fixed various editing commands
     13        to exit early if either end of the selection is orphaned.
     14
     15        Tests: editing/undo/orphaned-selection-crash-bug32823-1.html
     16               editing/undo/orphaned-selection-crash-bug32823-2.html
     17               editing/undo/orphaned-selection-crash-bug32823-3.html
     18               editing/undo/orphaned-selection-crash-bug32823-4.html
     19
     20        * editing/Editor.cpp:
     21        (WebCore::Editor::changeSelectionAfterCommand): No longer sets orphaned selection.
     22        * editing/VisibleSelection.h:
     23        (WebCore::VisibleSelection::isNonOrphanedRange): Added.
     24        (WebCore::VisibleSelection::isNonOrphanedCaretOrRange): Added.
     25        * editing/DeleteSelectionCommand.cpp:
     26        (WebCore::DeleteSelectionCommand::doApply): Added an early exist. See above.
     27        * editing/FormatBlockCommand.cpp:
     28        (WebCore::FormatBlockCommand::doApply): Ditto.
     29        * editing/IndentOutdentCommand.cpp:
     30        (WebCore::IndentOutdentCommand::doApply): Ditto.
     31        * editing/InsertLineBreakCommand.cpp:
     32        (WebCore::InsertLineBreakCommand::doApply): Ditto.
     33        * editing/InsertListCommand.cpp:
     34        (WebCore::InsertListCommand::doApply): Ditto.
     35        * editing/InsertParagraphSeparatorCommand.cpp:
     36        (WebCore::InsertParagraphSeparatorCommand::doApply): Ditto.
     37        * editing/InsertTextCommand.cpp:
     38        (WebCore::InsertTextCommand::input): Ditto.
     39        * editing/MoveSelectionCommand.cpp:
     40        (WebCore::MoveSelectionCommand::doApply): Ditto.
     41        * editing/RemoveFormatCommand.cpp:
     42        (WebCore::RemoveFormatCommand::doApply): Ditto.
     43        * editing/ReplaceSelectionCommand.cpp:
     44        (WebCore::ReplaceSelectionCommand::doApply): Ditto.
     45        * editing/TypingCommand.cpp:
     46        (WebCore::TypingCommand::doApply): Ditto.
     47        * editing/UnlinkCommand.cpp:
     48        (WebCore::UnlinkCommand::doApply): Ditto.
     49
    1502010-08-25  Simon Fraser  <simon.fraser@apple.com>
    251

  • trunk/WebCore/editing/DeleteSelectionCommand.cpp

    r65208 r66032  
    736736    if (!m_hasSelectionToDelete)
    737737        m_selectionToDelete = endingSelection();
    738    
    739     if (!m_selectionToDelete.isRange())
     738
     739    if (!m_selectionToDelete.isNonOrphanedRange())
    740740        return;
    741741

  • trunk/WebCore/editing/Editor.cpp

    r65919 r66032  
    29432943void Editor::changeSelectionAfterCommand(const VisibleSelection& newSelection, bool closeTyping, bool clearTypingStyle)
    29442944{
     2945    // If the new selection is orphaned, then don't update the selection.
     2946    if (newSelection.start().isOrphan() || newSelection.end().isOrphan())
     2947        return;
     2948
    29452949    // If there is no selection change, don't bother sending shouldChangeSelection, but still call setSelection,
    29462950    // because there is work that it must do in this situation.

  • trunk/WebCore/editing/FormatBlockCommand.cpp

    r42507 r66032  
    7171void FormatBlockCommand::doApply()
    7272{
    73     if (endingSelection().isNone())
     73    if (!endingSelection().isNonOrphanedCaretOrRange())
    7474        return;
    7575   

  • trunk/WebCore/editing/IndentOutdentCommand.cpp

    r63039 r66032  
    330330void IndentOutdentCommand::doApply()
    331331{
    332     if (endingSelection().isNone())
     332    if (!endingSelection().isNonOrphanedCaretOrRange())
    333333        return;
    334334

  • trunk/WebCore/editing/InsertLineBreakCommand.cpp

    r63773 r66032  
    9191    deleteSelection();
    9292    VisibleSelection selection = endingSelection();
    93     if (selection.isNone())
     93    if (!selection.isNonOrphanedCaretOrRange())
    9494        return;
    9595   

  • trunk/WebCore/editing/InsertListCommand.cpp

    r64337 r66032  
    9898void InsertListCommand::doApply()
    9999{
    100     if (endingSelection().isNone())
     100    if (!endingSelection().isNonOrphanedCaretOrRange())
    101101        return;
    102    
     102
    103103    if (!endingSelection().rootEditableElement())
    104104        return;

  • trunk/WebCore/editing/InsertParagraphSeparatorCommand.cpp

    r63773 r66032  
    149149{
    150150    bool splitText = false;
    151     if (endingSelection().isNone())
     151    if (!endingSelection().isNonOrphanedCaretOrRange())
    152152        return;
    153153   

  • trunk/WebCore/editing/InsertTextCommand.cpp

    r65468 r66032  
    112112    ASSERT(text.find('\n') == notFound);
    113113
    114     if (endingSelection().isNone())
     114    if (!endingSelection().isNonOrphanedCaretOrRange())
    115115        return;
    116116

  • trunk/WebCore/editing/MoveSelectionCommand.cpp

    r56175 r66032  
    4141{
    4242    VisibleSelection selection = endingSelection();
    43     ASSERT(selection.isRange());
     43    ASSERT(selection.isNonOrphanedRange());
    4444
    4545    Position pos = m_position;

  • trunk/WebCore/editing/RemoveFormatCommand.cpp

    r59956 r66032  
    5050{
    5151    Frame* frame = document()->frame();
    52    
     52
     53    if (!frame->selection()->selection().isNonOrphanedCaretOrRange())
     54        return;
     55
    5356    // Make a plain text string from the selection to remove formatting like tables and lists.
    5457    String string = plainText(frame->selection()->selection().toNormalizedRange().get());

  • trunk/WebCore/editing/ReplaceSelectionCommand.cpp

    r59956 r66032  
    783783    ASSERT(selection.isCaretOrRange());
    784784    ASSERT(selection.start().node());
    785     if (selection.isNone() || !selection.start().node())
     785    if (!selection.isNonOrphanedCaretOrRange() || !selection.start().node())
    786786        return;
    787787   

  • trunk/WebCore/editing/TypingCommand.cpp

    r65468 r66032  
    245245void TypingCommand::doApply()
    246246{
    247     if (endingSelection().isNone())
     247    if (!endingSelection().isNonOrphanedCaretOrRange())
    248248        return;
    249249       

  • trunk/WebCore/editing/UnlinkCommand.cpp

    r47688 r66032  
    3939{
    4040    // FIXME: If a caret is inside a link, we should remove it, but currently we don't.
    41     if (!endingSelection().isRange())
     41    if (!endingSelection().isNonOrphanedRange())
    4242        return;
    43        
     43
    4444    pushPartiallySelectedAnchorElementsDown();
    4545

  • trunk/WebCore/editing/VisibleSelection.h

    r56175 r66032  
    7474    bool isRange() const { return selectionType() == RangeSelection; }
    7575    bool isCaretOrRange() const { return selectionType() != NoSelection; }
     76    bool isNonOrphanedRange() const { return isRange() && !start().isOrphan() && !end().isOrphan(); }
     77    bool isNonOrphanedCaretOrRange() const { return isCaretOrRange() && !start().isOrphan() && !end().isOrphan(); }
    7678
    7779    bool isBaseFirst() const { return m_baseIsFirst; }

  • trunk/WebCore/editing/htmlediting.h

    r65021 r66032  
    219219// VisibleSelection
    220220// -------------------------------------------------------------------------
    221    
     221
    222222// Functions returning VisibleSelection
    223 
    224223VisibleSelection avoidIntersectionWithNode(const VisibleSelection&, Node*);
    225224VisibleSelection selectionForParagraphIteration(const VisibleSelection&);
Note: See TracChangeset for help on using the changeset viewer.