Merge branch 't/13410'

CHANGES.md

@@ -18,6 +18,7 @@ Fixed Issues:
 * [#13397](http://dev.ckeditor.com/ticket/13397): Fixed: Drag&drop a widget inside its nested widget crashes the editor.
 * [#13420](http://dev.ckeditor.com/ticket/13420): Fixed: The [Auto Embed](http://ckeditor.com/addon/autoembed) plugin ignores encoded characters in URL parameters.
 * [#13184](http://dev.ckeditor.com/ticket/13184): Fixed: Web page reloaded after drop on editor UI.
+* [#13410](http://dev.ckeditor.com/ticket/13410): Fixed: Error thrown in the [Auto Embed](http://ckeditor.com/addon/autoembed) plugin when undoing right after pasting a link.
 
 ## CKEditor 4.5.1
 

plugins/autoembed/plugin.js

@@ -94,7 +94,9 @@
 				finalizeCreation();
 			},
 
-			error: finalizeCreation
+			errorCallback: function() {
+				editor.widgets.destroy( instance, true );
+			}
 		} );
 
 		function finalizeCreation() {

plugins/embedbase/plugin.js

@@ -215,6 +215,21 @@
 				function finishLoading( response ) {
 					request.response = response;
 
+					// Check if widget is still valid.
+					if ( !that.editor.widgets.instances[ that.id ] ) {
+						// %REMOVE_START%
+						window.console && console.log && console.log( // jshint ignore:line
+							'[CKEDITOR.plugins.embedBase.baseDefinition.loadContent] Widget no longer belongs to current editor\'s widgets list.'
+						);
+						// %REMOVE_END%
+
+						if ( request.task ) {
+							request.task.done();
+						}
+
+						return;
+					}
+
 					if ( that._handleResponse( request ) ) {
 						that._cacheResponse( url, response );
 						if ( opts.callback ) {

plugins/widget/plugin.js

@@ -1989,7 +1989,8 @@
 
 		// Remove widgets which have no corresponding elements in DOM.
 		for ( i in instances ) {
-			if ( !editable.contains( instances[ i ].wrapper ) )
+			// #13410 Remove widgets that are ready. This prevents from destroying widgets that are during loading process.
+			if ( instances[ i ].isReady() && !editable.contains( instances[ i ].wrapper ) )
 				this.destroy( instances[ i ], true );
 		}
 

tests/plugins/autoembed/autoembed.js

@@ -59,29 +59,29 @@ bender.test( {
 	},
 
 	'test embedding when request failed': function() {
-		var bot = this.editorBot;
+		var bot = this.editorBot,
+			instanceDestroyedSpy = sinon.spy();
+
 		jsonpCallback = function( urlTemplate, urlParams, callback, errorCallback ) {
-			errorCallback();
+			resume( function() {
+				errorCallback();
+
+				assert.areSame( '<p><a href="https://foo.bar/g/200/302">https://foo.bar/g/200/302</a></p>', bot.getData( 1 ), 'link was not auto embedded' );
+				assert.isTrue( instanceDestroyedSpy.called, 'Widget instance destroyed.' );
+			} );
 		};
 
 		bot.setData( '', function() {
 			bot.editor.focus();
 			this.editor.execCommand( 'paste', 'https://foo.bar/g/200/302' );
 
+			// Check if errorCallback was called - it should destroy widget instance.
+			this.editor.widgets.once( 'instanceDestroyed', instanceDestroyedSpy );
+
 			// Note: afterPaste is fired asynchronously, but we can test editor data immediately.
-			assert.areSame(
-				'<p><a href="https://foo.bar/g/200/302">https://foo.bar/g/200/302</a></p>',
-				bot.getData( 1 ),
-				'link was pasted correctly'
-			);
+			assert.areSame( '<p><a href="https://foo.bar/g/200/302">https://foo.bar/g/200/302</a></p>', bot.getData( 1 ), 'link was pasted correctly' );
 
-			wait( function() {
-				assert.areSame(
-					'<p><a href="https://foo.bar/g/200/302">https://foo.bar/g/200/302</a></p>',
-					bot.getData( 1 ),
-					'link was not auto embedded'
-				);
-			}, 200 );
+			wait();
 		} );
 	},
 
@@ -311,5 +311,28 @@ bender.test( {
 			this.editor.execCommand( 'paste', '<a href="x">x</a>' );
 			wait();
 		} );
+	},
+
+	'test when user press undo before embedding process finishes': function() {
+		var bot = this.editorBot,
+			editor = bot.editor,
+			pastedText = 'https://foo.bar/g/200/382',
+			finalizeCreationSpy = sinon.spy( CKEDITOR.plugins.widget.repository.prototype, 'finalizeCreation' );
+
+		editor.once( 'afterPaste', function() {
+			editor.execCommand( 'undo' );
+
+		}, null, null, 900 );
+
+		bot.setData( '', function() {
+			editor.focus();
+			editor.resetUndo();
+			editor.execCommand( 'paste', pastedText );
+
+			wait( function() {
+				CKEDITOR.plugins.widget.repository.prototype.finalizeCreation.restore();
+				assert.areSame( finalizeCreationSpy.called, false, 'finalize creation was not called' );
+			}, 200 );
+		} );
 	}
 } );

tests/plugins/autoembed/manual/undoautoembed.html

@@ -0,0 +1,17 @@
+<p>Test URLs</p>
+
+<ul>
+	<li><input type="text" value="https://placekitten.com/g/200/300"></li>
+	<li><input type="text" value="https://twitter.com/reinmarpl/status/573118615274315776"></li>
+</ul>
+
+<textarea id="editor1" cols="10" rows="10"></textarea>
+
+<script>
+	embedTools.delayJsonp();
+	CKEDITOR.replace( 'editor1', {
+		extraPlugins: 'embed',
+		height: 500,
+		width: 600
+	} );
+</script>

tests/plugins/autoembed/manual/undoautoembed.md

@@ -0,0 +1,9 @@
+@bender-tags: tc, 4.5.2, 13410
+@bender-ui: collapsed
+@bender-include: ../../embedbase/_helpers/tools.js
+@bender-ckeditor-plugins: wysiwygarea,sourcearea,htmlwriter,entities,toolbar,elementspath,undo,clipboard,format,basicstyles,autolink,autoembed,link
+
+1. Copy one of the test URLs.
+1. Press CTRL+V to paste it into editor.
+1. Press CTRL+Z to undo **before** embedded content is loaded.
+1. Expected: embedding process should not be performed, pasted link should be removed.

tests/plugins/embedbase/definition.js

@@ -473,5 +473,44 @@ bender.test( {
 
 			wait();
 		} );
+	},
+
+	'test if embedding is canceled when widget is no longer valid': function() {
+		var bot = this.editorBots.inline,
+			editor = bot.editor,
+			successCallbackSpy = sinon.spy(),
+			errorCallbackSpy = sinon.spy();
+
+		bot.setData( dataWithWidget, function() {
+			var widget = getWidgetById( editor, 'w1' ),
+				task;
+
+			jsonpCallback = function( urlTemplate, urlParams, callback ) {
+				resume( function() {
+					callback( {
+						type: 'rich',
+						html: '<p>url:' + urlParams.url + '</p>'
+					} );
+
+					assert.isTrue( task.isDone(), 'The task is done.' );
+					assert.isFalse( successCallbackSpy.called, 'Success callback was not called.' );
+					assert.isFalse( errorCallbackSpy.called, 'Error callback was not called.' );
+				} );
+			};
+
+			widget.on( 'sendRequest', function( evt ) {
+				task = evt.data.task;
+			} );
+
+			widget.loadContent( '//canceling/handleresponse', {
+				callback: successCallbackSpy,
+				errorCallback: errorCallbackSpy
+			} );
+
+			editor.widgets.destroy( widget );
+
+			wait();
+		} );
+
 	}
-} );
+} );

tests/plugins/widget/widgetsrepoapi.js

@@ -1109,6 +1109,41 @@
 			} );
 		},
 
+		'test widgets.checkWidgets - do not remove widgets before finalize creation': function() {
+			var editor = this.editor,
+				editorBot = this.editorBot;
+
+			editor.widgets.add( 'testCheckWidgets1', {
+				template: '<b>X</b>',
+				init: function() {
+					this.on( 'edit', function( evt ) {
+						var id = this.id;
+
+						// Prevent automatic insertion, so code won't explode later.
+						evt.cancel();
+						assert.areSame( '<p>foo</p>', editor.getData() );
+
+						editor.widgets.checkWidgets();
+
+						// Widget should not be removed before finalizeCreation call.
+						assert.areSame( this, editor.widgets.instances[ id ], 'widget was not removed from repository' );
+
+						// Call finalizeCreation and check if widget will be removed from repository when
+						// no representation in DOM will be found.
+						editor.widgets.finalizeCreation( this.wrapper.getParent( true ) );
+						this.wrapper.remove();
+						editor.widgets.checkWidgets();
+						objectAssert.ownsNoKeys( editor.widgets.instances, 'widget was removed from repository' );
+					} );
+				}
+			} );
+
+			editorBot.setData( '', function() {
+				editorBot.setHtmlWithSelection( '<p>foo^</p>' );
+				editor.execCommand( 'testCheckWidgets1' );
+			} );
+		},
+
 		'test widgets.checkWidgets does not initialize widgets in temporary elements': function() {
 			var editor = this.editor,
 				editorBot = this.editorBot;
@@ -1656,4 +1691,4 @@
 			assert.isTrue( Widget.isDomDragHandlerContainer( domEmWidgetDragHandlerContainer ) );
 		}
 	} );
-} )();
+} )();