removeRedundantBlocks() may run into a null pointer while iterating over the tree, if the tree is unexpectedly modified during the execution of a command. Adding null check to quit this step if a parent node cannot be resolved.
<rdar://problem/76264118>
Created attachment 425933 [details] Patch
Comment on attachment 425933 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=425933&action=review > Source/WebCore/editing/DeleteSelectionCommand.cpp:869 > + RefPtr<Node> node = makeRefPtr(m_endingPosition.containerNode()); > + RefPtr<Node> rootNode = makeRefPtr(node->rootEditableElement()); Use auto? > Source/WebCore/editing/DeleteSelectionCommand.cpp:871 > + while (node && (node != rootNode)) { We don't usually nest parentheses like this. > Source/WebCore/editing/DeleteSelectionCommand.cpp:877 > - node = m_endingPosition.anchorNode(); > + node = makeRefPtr(m_endingPosition.anchorNode()); No need to call makeRefPtr since node is already of type RefPtr<Node>. > Source/WebCore/editing/DeleteSelectionCommand.cpp:879 > - node = node->parentNode(); > + node = makeRefPtr(node->parentNode()); Ditto. > LayoutTests/editing/execCommand/remove-node-during-command-crash.html:24 > + document.write('PASS') Missing semicolon at the end. Also, can we say that this test passes if WebKit doesn't crash?
Created attachment 426154 [details] Patch
Tried to add the text to the test body saying the test passes if it doesn't crash but it either would stop the test from reproducing, or would get eaten by the command. Hopefully that is appropriate.
Comment on attachment 426154 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=426154&action=review > Source/WebCore/ChangeLog:3 > + Editing null pointer dereference while resolving command I've just renamed the bug. Please update the title here. It's overly generic. Sorry I didn't notice earlier. > LayoutTests/ChangeLog:3 > + Editing null pointer dereference while resolving command Ditto. > LayoutTests/ChangeLog:8 > + Add regression test. Nit: Add *a* regression test.
Created attachment 426267 [details] Patch
Let's at least wait for the stress EWS to make sure the new test isn't flaky, etc...
Committed r276186 (236668@main): <https://commits.webkit.org/236668@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 426267 [details].