[Webkit-unassigned] [Bug 224259] Nullptr dereference in ReplaceSelectionCommand::removeRedundantStylesAndKeepStyleSpanInline

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=224259

Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |darin at apple.com
 Attachment #425464|review?                     |review+
              Flags|                            |

--- Comment #8 from Darin Adler <darin at apple.com> ---
Comment on attachment 425464
  --> https://bugs.webkit.org/attachment.cgi?id=425464
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=425464&action=review

Our new code has many branches. Our new test does not cover them. I think we need more tests to cover all the cases that require different logic.

> Source/WebCore/editing/ReplaceSelectionCommand.cpp:428
> +    ASSERT(!m_firstNodeInserted || m_firstNodeInserted == node || !node->contains(m_firstNodeInserted.get()));
> +    ASSERT(!m_lastNodeInserted || m_lastNodeInserted == node || !node->contains(m_lastNodeInserted.get()));

Seems like we should consider "!isDescendantOf" here to shorten these assertions:

    ASSERT(!m_firstNodeInserted || !m_firstNodeInserted->isDescendantOf(node));
    ASSERT(!m_lastNodeInserted || !m_lastNodeInserted->isDescendantOf(node));

Would be even better if we had a non-member version of isDescendantOf that understands that null isn’t a descendant of anything.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20210408/7767d01f/attachment.htm>