Similar to 218132: #0 0x4ce0df2ce in WTF::OptionSet<WebCore::Node::NodeFlag>::containsAny(WTF::OptionSet<WebCore::Node::NodeFlag>) const+0xbe #1 0x4ce0df1a9 in WTF::OptionSet<WebCore::Node::NodeFlag>::contains(WebCore::Node::NodeFlag) const+0xd9 #2 0x4ce0df0cc in WebCore::Node::hasNodeFlag(WebCore::Node::NodeFlag) const+0xc #3 0x4ce0df7dd in WebCore::Node::isElementNode() const+0xd #4 0x4ce0df7c8 in WTF::TypeCastTraits<WebCore::Element const, WebCore::ContainerNode const, false>::isType(WebCore::Node const&)+0x8 #5 0x4ce0df7b8 in WTF::TypeCastTraits<WebCore::Element const, WebCore::ContainerNode const, false>::isOfType(WebCore::ContainerNode const&)+0x8 #6 0x4ce0df7e8 in bool WTF::is<WebCore::Element, WebCore::ContainerNode>(WebCore::ContainerNode&)+0x8 #7 0x4d227cce7 in WebCore::CompositeEditCommand::splitTreeToNode(WebCore::Node&, WebCore::Node&, bool)+0x227 #8 0x4d2329f03 in WebCore::InsertListCommand::unlistifyParagraph(WebCore::VisiblePosition const&, WebCore::HTMLElement*, WebCore::Node*)+0x723 #9 0x4d23293a4 in WebCore::InsertListCommand::doApplyForSingleParagraph(bool, WebCore::HTMLQualifiedName const&, WebCore::SimpleRange&)+0xbd4 #10 0x4d23286f6 in WebCore::InsertListCommand::doApply()+0xce6 #11 0x4d22545c6 in WebCore::CompositeEditCommand::apply()+0x216 #12 0x4d23143e8 in WebCore::executeInsertOrderedList(WebCore::Frame&, WebCore::Event*, WebCore::EditorCommandSource, WTF::String const&)+0xc8 #13 0x4d22d8d9b in WebCore::Editor::Command::execute(WTF::String const&, WebCore::Event*) const+0xdb #14 0x4d1f61aa3 in WebCore::Document::execCommand(WTF::String const&, bool, WTF::String const&)+0xf3
<rdar://problem/70702563>
<rdar://problem/66864853> Null Ptr Deref @ WebCore::CompositeEditCommand::splitTreeToNode -- DOS WebKitTestRunner WK2
Created attachment 412366 [details] Patch
Comment on attachment 412366 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=412366&action=review > Source/WebCore/editing/CompositeEditCommand.cpp:1716 > + for (node = &start; node && node->parentNode() && node->parentNode() != adjustedEnd; node = node->parentNode()) { > if (!is<Element>(*node->parentNode())) Can we avoid calling parentNode() four times here?
Fixed that in the new patch, which also addresses the failure of the new test on the debug build.
Created attachment 412688 [details] Patch
Comment on attachment 412688 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=412688&action=review > Source/WebCore/editing/InsertListCommand.cpp:324 > + Node* lcnParentNode = listChildNode->parentNode(); Please don't use raw pointer. Use makeRefPtr to store it in RefPtr. Also please don't abbreviations like lcn.
Comment on attachment 412688 [details] Patch I think these test failures are legitimate.
(In reply to Ryosuke Niwa from comment #7) > Comment on attachment 412688 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=412688&action=review > > > Source/WebCore/editing/InsertListCommand.cpp:324 > > + Node* lcnParentNode = listChildNode->parentNode(); > > Please don't use raw pointer. Use makeRefPtr to store it in RefPtr. > Also please don't abbreviations like lcn. Fixed in new patch.
(In reply to Ryosuke Niwa from comment #8) > Comment on attachment 412688 [details] > Patch > > I think these test failures are legitimate. Agreed, I can now reproduce them locally. I have a new patch that I'm preparing that should address them.
Created attachment 413379 [details] Patch
Comment on attachment 413379 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=413379&action=review > LayoutTests/editing/inserting/insert-list-in-iframe-in-list-expected.txt:3 > +This tests that we do not crah while inserting either list. crash
Comment on attachment 413379 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=413379&action=review > Source/WebCore/editing/InsertListCommand.cpp:324 > + RefPtr<Node> listChildNodeParentNode = makeRefPtr(listChildNode->parentNode()); Use auto. > Source/WebCore/editing/InsertListCommand.cpp:325 > + if (listChildNodeParentNode && listChildNodeParentNode != listNode) Declare the variable in if as in: if (auto listChildNodeParentNode = ~; listChildNodeParentNode != listNode)
Comment on attachment 413379 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=413379&action=review >> Source/WebCore/editing/InsertListCommand.cpp:325 >> + if (listChildNodeParentNode && listChildNodeParentNode != listNode) > > Declare the variable in if as in: > if (auto listChildNodeParentNode = ~; listChildNodeParentNode != listNode) if (auto listChildNodeParentNode = makeRefPtr(listChildNode->parentNode()); listChildNodeParentNode && listChildNodeParentNode != listNode) Hooray C++17!
There is no security implication here.
(In reply to Alex Christensen from comment #12) > Comment on attachment 413379 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=413379&action=review > > > LayoutTests/editing/inserting/insert-list-in-iframe-in-list-expected.txt:3 > > +This tests that we do not crah while inserting either list. > > crash Thanks :P
(In reply to Alex Christensen from comment #14) > Comment on attachment 413379 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=413379&action=review > > >> Source/WebCore/editing/InsertListCommand.cpp:325 > >> + if (listChildNodeParentNode && listChildNodeParentNode != listNode) > > > > Declare the variable in if as in: > > if (auto listChildNodeParentNode = ~; listChildNodeParentNode != listNode) > > if (auto listChildNodeParentNode = makeRefPtr(listChildNode->parentNode()); > listChildNodeParentNode && listChildNodeParentNode != listNode) > > Hooray C++17! Upgrading my mental C++ model is a slow process :) Thank you both.
Created attachment 413617 [details] Patch
Committed r269609: <https://trac.webkit.org/changeset/269609> All reviewed patches have been landed. Closing bug and clearing flags on attachment 413617 [details].