ckeditor
TypeError when dragging nested widget out when parent is selected
Type of report
Bug
Provide detailed reproduction steps (if any)
- Create a widget that contains an editable area, and place another widget inside as the last child
- Select the parent widget by clicking on a non-editable region of it (or the drag handle)
- Drag the child widget out of the parent using its drag handle. The parent widget will remain selected until the child is dropped.
Expected result
The child is moved out of the parent successfully.
Actual result
The child widget disappears, and an error is logged to the console
Uncaught TypeError: Cannot read property 'type' of null (range.js:803)
As far as I can tell, the error is occurring because the selection end is considered to be the last element within the widget, and the selection data is cached. When the child is that last element, and it is dragged out, it triggers a paste event that looks at the current selection elements. Because the widget has already been removed, but the selection is cached, this results in it trying to get properties of the removed element, producing an error.
Other details
- Browser: Chrome 76
- OS: Linux
- CKEditor version: 4.5.0+
It turns out that the issue only occurs when you also call editor.getSelection().createBookmarks2( true );, because it tries to normalise the text nodes and discovers that one is missing.
An MCVE plugin is:
CKEDITOR.plugins.add( 'widgetdemo', {
requires: 'widget',
icons: 'blockquote',
allowedContent: 'blockquote',
init: function( editor ) {
editor.widgets.add( 'widgetdemo', {
button: 'insert quote',
template: '<blockquote style="background-color: rgba(0, 0, 0, 0.3)"><div class="quote-contents"><p>Hello</p></div></blockquote>',
editables: {
content: {
selector: '.quote-contents'
}
},
});
editor.on('paste', () => {
editor.getSelection().createBookmarks2();
});
}
} );
With nested widgets created using this plugin, if the inner one is at the very bottom of the outer, it can be reproduced by clicking the move widget of the outer widget, then using the move widget of the inner widget to drag the inner one somewhere else.
I created fiddle with your plugin, but I still can't reproduce the issue: https://jsfiddle.net/Comandeer/jmrnus4y/
Is something missing there?
I'm able to reproduce it in that fiddle in both Firefox and Chrome (on Fedora 30). I've created a video here to demonstrate it: https://www.jackw.net/ckeditor-widget-bug.webm
The video doesn't include my cursor, but the sequence of clicks is: add the widgets, move the inner widget to the bottom of the outer one, then click the drag tool on the outer one to select it, then move the inner one out.
That's strange, as moving the inner widget to the very end of the containing widget shouldn't be possible at all. It seems that somehow bookmarks allows such behaviour and only after text normalization. The issue is located in betweenTextNodes private function in CKEDITOR.dom.range, which assumes that there are always some preceding and following nodes:
https://github.com/ckeditor/ckeditor-dev/blob/433b751b6971fc4e4889679301274f34f162f171/core/dom/range.js#L817-L818
The fix would be easy enough, add additional checks if such nodes really exist.
OTOH, our documentation warns that this kind of bookmarks shouldn't be used when the DOM is expected to be changed – and dropping widget in a new place is mutating DOM. It seems that CKEDITOR.dom.selection#createBookmarks works fine (demo) and it's the correct way to add bookmarks in such situation.
FWIW we're seeing this bug in the wild and are not calling createBookmarks ourselves... We have a custom widget and if the user deletes it and then clicks out of the editor, the UndoManager.save triggers this same call to createBookmarks2 and causes the error. So pushing the fix for this would be appropriate.
