Created attachment 54477 [details] test case Encountered in Google Docs. See http://crbug.com/42633 for more details. Test case attached. Regression range: 53845:53990 Likely a result of http://trac.webkit.org/changeset/53927. Possibly related to bug 38231 even though that's not a recent regression.
Created attachment 54522 [details] Patch
(In reply to comment #0) > Likely a result of http://trac.webkit.org/changeset/53927. Yup, that change was completely wrong. The guy who wrote it didn't know what he was doing. Hopefully that guy understands this better now. The new patch reverts r53927 and adds a comment explaining what the code is supposed to do. It includes a new fix to the problem. The problem was that some nodes are block elements even though they can't contain text or other elements (e.g., <hr>). In this case, we would try to split the <hr>, but that doesn't make sense. Rather than try to split, we just treat the individual node as a block.
Comment on attachment 54522 [details] Patch The code changes look correct to me. > +++ b/LayoutTests/editing/execCommand/crash-indenting-list-item.html > @@ -0,0 +1,9 @@ > +<div contentEditable><ul><li id='foo'></li></ul></div> > +<script> > +if (window.layoutTestController) > + layoutTestController.dumpAsText(); > +window.getSelection().setBaseAndExtent(foo, 0, foo, 0); > +// This test passes if it does not crash. > +document.execCommand('indent', false, null); > +document.getElementById("foo").innerText = "PASSED"; > +</script> We now have dump-as-markup that's perfect for something like this. Ideally this test would also show that the li got indented (which dump-as-markup would show). All you need to do to make this work with dump-as-markup is add the following to the top of the file: <script src="../../resources/dump-as-markup.js"></script>
Created attachment 55199 [details] Patch
(In reply to comment #4) > Created an attachment (id=55199) [details] > Patch Switched to a dump-as-markup test as Ojan suggested.
Comment on attachment 55199 [details] Patch This looks good. The old code was clearly violating an assert in splitTreeToNode. I would bet that an audit of call calls to splitTreeToNode would find other cases like this. I almost wonder if splitTreeToNode should just deal with this case (i.e. not split and just return the passed in Node), but I'm on the fence. This is certainly fine as a fix to the crash.
Committed r59739: <http://trac.webkit.org/changeset/59739>