Merge branch 't/13429d'

Author: oleq

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:
 

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 <a> element instead of inside it.
+					// So, if user hasn't changed selection, bookmark is inserted right after <a>.
+					// 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:
+					// <p>A <a /><bm /></p><p>B</p>  -->  <p>A <bm /></p><widget /><p>B</p>  -->  <p>A ^</p><widget /><p>B</p>
+					// 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();

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 <p> created after the widget.
+				assert.areSame( '<div data-oembed-url="' + pastedText + '"><img src="' + pastedText + '" /></div><p>\u00A0</p>', editor.getData(), 'right editor data after paste' );
+				var p = editor.editable().findOne( 'p' );
+
+				// Check if caret is inside newly created <p>.
+				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( '<p>foo</p><p>bar</p>', function() {
+			editor.focus();
+
+			// Set caret at the end of the first <p>.
+			var range = editor.createRange();
+			range.setStart( this.editor.editable().find( 'p' ).getItem( 0 ).getFirst(), 3 );
+			range.collapse();
+			// '<p>foo^</p><p>bar</p>'
+			editor.getSelection().selectRanges( [ range ] );
+
+			editor.execCommand( 'paste', pastedText );
+
+			wait( function() {
+				// Get the second <p>.
+				var p = editor.editable().find( 'p' ).getItem( 1 );
+				var range = editor.getSelection().getRanges()[ 0 ];
+				var container = range.startContainer;
+
+				// Check if caret is inside second <p>.
+				// Different browsers set startContainer differently,
+				// so we check if it is in <p> or in a text node inside that <p>.
+				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( '<div data-oembed-url="' + pastedText + '"><img src="' + pastedText + '" /></div><p>foo</p>', editor.getData(), 'right editor data after paste' );
+				assert.isMatching( /[{\[]\u200b?foo[}\]]/, bender.tools.selection.getWithHtml( editor ), 'selection anchored at the right position' );
+			}, 200 );
+		} );
 	}
 } );

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).
+* 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).