diff --git a/CHANGES.md b/CHANGES.md index 5492e170ec1..55d863f8793 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -19,6 +19,7 @@ Fixed Issues: * [#13453](http://dev.ckeditor.com/ticket/13453): Fixed: Drag&drop of a whole content of the editor throws an error. * [#13465](http://dev.ckeditor.com/ticket/13465): Fixed: Error is thrown and widget is lost when drag&drop if it is the only content of the editor. * [#13414](http://dev.ckeditor.com/ticket/13414): Fixed: Content auto paragraphing in a nested editable despite editor configuration. +* [#13429](http://dev.ckeditor.com/ticket/13429): Fixed: Wrong selection after content insertion by [Auto Embed](http://ckeditor.com/addon/autoembed) plugin. Other Changes: diff --git a/plugins/autoembed/plugin.js b/plugins/autoembed/plugin.js index 08fee443ab4..b28e137d394 100644 --- a/plugins/autoembed/plugin.js +++ b/plugins/autoembed/plugin.js @@ -84,15 +84,53 @@ noNotifications: true, callback: function() { // DOM might be invalidated in the meantime, so find the anchor again. - var anchor = editor.editable().findOne( 'a[data-cke-autoembed="' + id + '"]' ), - range = editor.createRange(); + var anchor = editor.editable().findOne( 'a[data-cke-autoembed="' + id + '"]' ); // Anchor might be removed in the meantime. if ( anchor ) { - range.setStartAt( anchor, CKEDITOR.POSITION_BEFORE_START ); - range.setEndAt( anchor, CKEDITOR.POSITION_AFTER_END ); + var selection = editor.getSelection(), + insertRange = editor.createRange(), + editable = editor.editable(); + + // Save the changes in editor contents that happened *after* the link was pasted + // but before it gets embedded (i.e. user pasted and typed). + editor.fire( 'saveSnapshot' ); + + // Lock snapshot so we don't make unnecessary undo steps in + // editable.insertElement() below, which would include bookmarks. (#13429) + editor.fire( 'lockSnapshot', { dontUpdate: true } ); + + // Bookmark current selection. (#13429) + var bookmark = selection.createBookmarks( false )[ 0 ], + startNode = bookmark.startNode, + endNode = bookmark.endNode || startNode; + + // When url is pasted, IE8 sets the caret after element instead of inside it. + // So, if user hasn't changed selection, bookmark is inserted right after . + // Then, after pasting embedded content, bookmark is still in DOM but it is + // inside the original element. After selection recreation it would end up before widget: + //

A

B

-->

A

B

-->

A ^

B

+ // We have to fix this IE8 behavior so it is the same as on other browsers. + if ( CKEDITOR.env.ie && CKEDITOR.env.version < 9 && !bookmark.endNode && startNode.equals( anchor.getNext() ) ) { + anchor.append( startNode ); + } + + insertRange.setStartBefore( anchor ); + insertRange.setEndAfter( anchor ); + + editable.insertElement( wrapper, insertRange ); + + // If both bookmarks are still in DOM, it means that selection was not inside + // an anchor that got substituted. We can safely recreate that selection. (#13429) + if ( editable.contains( startNode ) && editable.contains( endNode ) ) { + selection.selectBookmarks( [ bookmark ] ); + } else { + // If one of bookmarks is not in DOM, clean up leftovers. + startNode.remove(); + endNode.remove(); + } - editor.editable().insertElementIntoRange( wrapper, range ); + editor.fire( 'unlockSnapshot' ); } notification.hide(); diff --git a/tests/plugins/autoembed/autoembed.js b/tests/plugins/autoembed/autoembed.js index 9653730f3a4..7ce2d06062c 100644 --- a/tests/plugins/autoembed/autoembed.js +++ b/tests/plugins/autoembed/autoembed.js @@ -375,5 +375,95 @@ bender.test( { editor.execCommand( 'paste', 'https://foo.bar/g/notification/test/1' ); wait(); } ); + }, + + // #13429. + 'test selection after auto embedding - empty editor': function() { + var bot = this.editorBot, + editor = bot.editor, + pastedText = 'https://foo.bar/g/200/382'; + + bot.setData( '', function() { + editor.focus(); + editor.execCommand( 'paste', pastedText ); + + wait( function() { + // Check if there is exactly one additional

created after the widget. + assert.areSame( '

\u00A0

', editor.getData(), 'right editor data after paste' ); + var p = editor.editable().findOne( 'p' ); + + // Check if caret is inside newly created

. + var range = editor.getSelection().getRanges()[ 0 ]; + assert.isTrue( range.collapsed, 'selection after paste is collapsed' ); + assert.areSame( 0, range.startOffset, 'selection at the beginning of the paragraph' ); + assert.isTrue( range.startContainer.equals( p ), 'selection inside correct p element' ); + }, 200 ); + } ); + }, + + // #13429. + 'test selection after auto embedding - inside content': function() { + var bot = this.editorBot, + editor = bot.editor, + pastedText = 'https://foo.bar/g/200/382'; + + bot.setData( '

foo

bar

', function() { + editor.focus(); + + // Set caret at the end of the first

. + var range = editor.createRange(); + range.setStart( this.editor.editable().find( 'p' ).getItem( 0 ).getFirst(), 3 ); + range.collapse(); + // '

foo^

bar

' + editor.getSelection().selectRanges( [ range ] ); + + editor.execCommand( 'paste', pastedText ); + + wait( function() { + // Get the second

. + var p = editor.editable().find( 'p' ).getItem( 1 ); + var range = editor.getSelection().getRanges()[ 0 ]; + var container = range.startContainer; + + // Check if caret is inside second

. + // Different browsers set startContainer differently, + // so we check if it is in

or in a text node inside that

. + assert.isTrue( range.collapsed, 'selection after paste is collapsed' ); + assert.areSame( 0, range.startOffset, 'selection anchored at the beginning of the paragraph' ); + assert.isTrue( !!new CKEDITOR.dom.elementPath( container ).contains( p ), 'selection inside correct p element' ); + assert.isInnerHtmlMatching( 'bar@', ( container.type == CKEDITOR.NODE_TEXT ? container.getParent() : container ).getHtml(), 'selection inside correct p element' ); + }, 200 ); + } ); + }, + + // #13429. + 'test selection after auto embedding - content and selection change before insert': function() { + var bot = this.editorBot, + editor = bot.editor, + editable = editor.editable(), + pastedText = 'https://foo.bar/g/200/382'; + + bot.setData( '', function() { + editor.focus(); + editor.execCommand( 'paste', pastedText ); + + var pastedAnchor = editable.findOne( 'a' ); + + // After link has been pasted, "type" some text... + var text = new CKEDITOR.dom.text( 'foo' ); + text.insertAfter( pastedAnchor ); + + // ..and make a selection on that text. + // "[foo]" + var range = editor.createRange(); + range.setStartBefore( text ); + range.setEndAfter( text ); + range.select(); + + wait( function() { + assert.areSame( '

foo

', editor.getData(), 'right editor data after paste' ); + assert.isMatching( /[{\[]\u200b?foo[}\]]/, bender.tools.selection.getWithHtml( editor ), 'selection anchored at the right position' ); + }, 200 ); + } ); } } ); diff --git a/tests/plugins/autoembed/manual/autoembed.md b/tests/plugins/autoembed/manual/autoembed.md index 248dd52e338..c3440262779 100644 --- a/tests/plugins/autoembed/manual/autoembed.md +++ b/tests/plugins/autoembed/manual/autoembed.md @@ -11,4 +11,6 @@ Things to check: * Other content changes. * Pasting more complex content (only single links should be embedded). * Undo/redo. Note: There should be two steps – one reverting autoembed and one reverting link paste. -* That in the 1st editor `embed` is used and in the 2nd editor `embedsemantic` (check the data). \ No newline at end of file +* After pasting a link write some text **before** the content is embedded. Alternatively, simply change the selection. Check if the selection is **exactly the same** as before the insertion. +* Check if all operations are saved in undo steps (paste link, write text, insert content, write text.) +* That in the 1st editor `embed` is used and in the 2nd editor `embedsemantic` (check the data).