Changeset 275652 in webkit


Ignore:
Timestamp:
Apr 7, 2021 10:21:10 PM (3 years ago)
Author:
commit-queue@webkit.org
Message:

Nullptr dereference in ReplaceSelectionCommand::removeRedundantStylesAndKeepStyleSpanInline
https://bugs.webkit.org/show_bug.cgi?id=224259

Patch by Julian Gonzalez <julian_a_gonzalez@apple.com> on 2021-04-07
Reviewed by Ryosuke Niwa.

Source/WebCore:

When pruning after removing the end <br> in ReplaceSelectionCommand::doApply(), make sure
that insertedNodes is updated properly (given that we may be removing an ancestor
of the start or end of insertedNodes).

Test: editing/inserting/insert-display-contents-crash.html

  • editing/ReplaceSelectionCommand.cpp:

(WebCore::ReplaceSelectionCommand::InsertedNodes::willRemovePossibleAncestorNode):
(WebCore::ReplaceSelectionCommand::InsertedNodes::willRemoveNode):
(WebCore::ReplaceSelectionCommand::doApply):

  • editing/ReplaceSelectionCommand.h:

LayoutTests:

Add a test to catch the editing crash fixed here; thanks to Tuomas Karkkainen
for its basic structure.

  • editing/inserting/insert-display-contents-crash-expected.txt: Added.
  • editing/inserting/insert-display-contents-crash.html: Added.
Location:
trunk
Files:
2 added
4 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r275621 r275652  
     12021-04-07  Julian Gonzalez  <julian_a_gonzalez@apple.com>
     2
     3        Nullptr dereference in ReplaceSelectionCommand::removeRedundantStylesAndKeepStyleSpanInline
     4        https://bugs.webkit.org/show_bug.cgi?id=224259
     5
     6        Reviewed by Ryosuke Niwa.
     7
     8        Add a test to catch the editing crash fixed here; thanks to Tuomas Karkkainen
     9        for its basic structure.
     10
     11        * editing/inserting/insert-display-contents-crash-expected.txt: Added.
     12        * editing/inserting/insert-display-contents-crash.html: Added.
     13
    1142021-04-07  Robert Jenner  <jenner@apple.com>
    215
  • trunk/Source/WebCore/ChangeLog

    r275651 r275652  
     12021-04-07  Julian Gonzalez  <julian_a_gonzalez@apple.com>
     2
     3        Nullptr dereference in ReplaceSelectionCommand::removeRedundantStylesAndKeepStyleSpanInline
     4        https://bugs.webkit.org/show_bug.cgi?id=224259
     5
     6        Reviewed by Ryosuke Niwa.
     7
     8        When pruning after removing the end <br> in ReplaceSelectionCommand::doApply(), make sure
     9        that insertedNodes is updated properly (given that we may be removing an ancestor
     10        of the start or end of insertedNodes).
     11
     12        Test: editing/inserting/insert-display-contents-crash.html
     13
     14        * editing/ReplaceSelectionCommand.cpp:
     15        (WebCore::ReplaceSelectionCommand::InsertedNodes::willRemovePossibleAncestorNode):
     16        (WebCore::ReplaceSelectionCommand::InsertedNodes::willRemoveNode):
     17        (WebCore::ReplaceSelectionCommand::doApply):
     18        * editing/ReplaceSelectionCommand.h:
     19
    1202021-04-07  Jean-Yves Avenard  <jya@apple.com>
    221
  • trunk/Source/WebCore/editing/ReplaceSelectionCommand.cpp

    r275410 r275652  
    401401}
    402402
     403inline void ReplaceSelectionCommand::InsertedNodes::willRemovePossibleAncestorNode(Node* node)
     404{
     405    bool containsFirstNode = node->contains(m_firstNodeInserted.get());
     406    bool containsLastNode = node->contains(m_lastNodeInserted.get());
     407    if (containsFirstNode && containsLastNode) {
     408        m_firstNodeInserted = nullptr;
     409        m_lastNodeInserted = nullptr;
     410        return;
     411    }
     412
     413    if (containsLastNode)
     414        m_lastNodeInserted = NodeTraversal::previousSkippingChildren(*node);
     415    else if (containsFirstNode)
     416        m_firstNodeInserted = NodeTraversal::nextSkippingChildren(*node);
     417
     418    if (!m_lastNodeInserted)
     419        m_lastNodeInserted = m_firstNodeInserted;
     420    else if (!m_firstNodeInserted)
     421        m_firstNodeInserted = m_lastNodeInserted;
     422    else if (m_firstNodeInserted->isDescendantOf(m_lastNodeInserted.get()))
     423        std::swap(m_firstNodeInserted, m_lastNodeInserted);
     424}
     425
    403426inline void ReplaceSelectionCommand::InsertedNodes::willRemoveNode(Node* node)
    404427{
     428    ASSERT(!m_firstNodeInserted || !m_firstNodeInserted->isDescendantOf(node));
     429    ASSERT(!m_lastNodeInserted || !m_lastNodeInserted->isDescendantOf(node));
     430
    405431    if (m_firstNodeInserted == node && m_lastNodeInserted == node) {
    406432        m_firstNodeInserted = nullptr;
     
    13041330        document().updateLayoutIgnorePendingStylesheets();
    13051331        if (auto nodeToRemove = makeRefPtr(highestNodeToRemoveInPruning(parent.get()))) {
    1306             insertedNodes.willRemoveNode(nodeToRemove.get());
     1332            insertedNodes.willRemovePossibleAncestorNode(nodeToRemove.get());
    13071333            removeNode(*nodeToRemove);
    13081334        }
    13091335    }
     1336
     1337    if (insertedNodes.isEmpty())
     1338        return;
    13101339
    13111340    makeInsertedContentRoundTrippableWithHTMLTreeBuilder(insertedNodes);
  • trunk/Source/WebCore/editing/ReplaceSelectionCommand.h

    r265176 r275652  
    6969        void respondToNodeInsertion(Node*);
    7070        void willRemoveNodePreservingChildren(Node*);
     71        void willRemovePossibleAncestorNode(Node*);
    7172        void willRemoveNode(Node*);
    7273        void didReplaceNode(Node*, Node* newNode);
Note: See TracChangeset for help on using the changeset viewer.