Executing a formatBlock command on a selection containing an element with contenteditable="false" causes a crash due to a null pointer. Downstream bug: http://code.google.com/p/chromium/issues/detail?id=70160
Created attachment 80371 [details] Patch
Comment on attachment 80371 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=80371&action=review > Source/WebCore/editing/FormatBlockCommand.cpp:72 > + // Root is null for elements with contenteditable=false. > + if (!root) > + return; Why don’t other commands need this kind of check? Is this really specific to FormatBlock?
editableRootForPosition is only used by a handfull of commands and FormatBlock appears to be the only one where it's assumed to always return a node. I tried quite a few commands and FormatBlock was the only one I could get to trigger a crash. I could include more commands in the test if you'd like (and make it a generic contenteditable=false/execCommand test).
(In reply to comment #3) > I could include more commands in the test if you'd like (and make it a generic contenteditable=false/execCommand test). Sure, that’d be nice long term, but not needed now.
Comment on attachment 80371 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=80371&action=review >> Source/WebCore/editing/FormatBlockCommand.cpp:72 >> + return; > > Why don’t other commands need this kind of check? Is this really specific to FormatBlock? I figured out the answer to my own question. The simple editing commands all have code in them that checks and does nothing if the nodes involved are not editable. And the complex commands are built out of the simple commands. So in most cases, no checking is needed. There may be some advantage to doing some higher level checking so we don’t end up with undoable commands on the undo chain that work hard to do nothing.
Makes sense. Thanks for the review!
The commit-queue encountered the following flaky tests while processing attachment 80371 [details]: http/tests/xmlhttprequest/basic-auth.html bug 51613 (author: ap@webkit.org) The commit-queue is continuing to process your patch.
Comment on attachment 80371 [details] Patch Clearing flags on attachment: 80371 Committed r76903: <http://trac.webkit.org/changeset/76903>
All reviewed patches have been landed. Closing bug.