Merge branch 't/5217' into major

Reinmar · Reinmar · commit ef06fd53d60e · 2014-03-13T13:32:26.000+01:00
diff --git a/CHANGES.md b/CHANGES.md
@@ -9,6 +9,10 @@ CKEditor 4 Changelog
 
 New Features:
 
+* [#5217](http://dev.ckeditor.com/ticket/5217): Setting data (including switching between modes) creates new undo snapshot. Besides that:
+  * Introduced the [`editable.status`](http://docs.ckeditor.com/#!/api/CKEDITOR.editable-property-status) property.
+  * Introduced new option `forceUpdate` for the [`editor.lockSnapshot`](http://docs.ckeditor.com/#!/api/CKEDITOR.editor-event-lockSnapshot) event.
+  * Fixed: selection not being unlocked in inline editor after setting data ([#15000](http://dev.ckeditor.com/ticket/11500)).
 * [#10202](http://dev.ckeditor.com/ticket/10202): Introduced wildcards support in the [Allowed Content Rules](http://docs.ckeditor.com/#!/guide/dev_allowed_content_rules) format.
 * [#11532](http://dev.ckeditor.com/ticket/11532): Introduced the [`editor.addContentsCss()`](http://docs.ckeditor.com/#!/api/CKEDITOR.editor-method-addContentsCss) method that can be used for adding custom CSS files.
 * [#11536](http://dev.ckeditor.com/ticket/11536): Added the [`CKEDITOR.tools.htmlDecode`](http://docs.ckeditor.com/#!/api/CKEDITOR.tools-method-htmlDecode) method for decoding HTML entities.
diff --git a/core/creators/themedui.js b/core/creators/themedui.js
@@ -151,20 +151,43 @@ CKEDITOR.replaceClass = 'ckeditor';
 		editor.fire( 'beforeSetMode', newMode );
 
 		if ( editor.mode ) {
-			var isDirty = editor.checkDirty();
-
-			editor._.previousMode = editor.mode;
+			var isDirty = editor.checkDirty(),
+				previousModeData = editor._.previousModeData,
+				currentData,
+				unlockSnapshot = 0;
 
 			editor.fire( 'beforeModeUnload' );
 
-			// Detach the current editable.
+			// Detach the current editable. While detaching editable will set
+			// cached editor's data (with internal setData call). We use this
+			// data below to avoid two getData() calls in a row.
 			editor.editable( 0 );
 
+			editor._.previousMode = editor.mode;
+			// Get cached data, which was set while detaching editable.
+			editor._.previousModeData = currentData = editor.getData( 1 );
+
+			// If data has not been modified in the mode which we are currently leaving,
+			// avoid making snapshot right after initializing new mode.
+			// http://dev.ckeditor.com/ticket/5217#comment:20
+			// Tested by:
+			// 'test switch mode with unrecoreded, inner HTML specific content (boguses)'
+			// 'test switch mode with unrecoreded, inner HTML specific content (boguses) plus changes in source mode'
+			if ( editor.mode == 'source' && previousModeData == currentData ) {
+				// We need to make sure that unlockSnapshot will update the last snapshot
+				// (will not create new one) if lockSnapshot is not called on outdated snapshots stack.
+				// Additionally, forceUpdate prevents from making content image now, which is useless
+				// (because it equals editor data not inner HTML).
+				editor.fire( 'lockSnapshot', { forceUpdate: true } );
+				unlockSnapshot = 1;
+			}
+
 			// Clear up the mode space.
 			editor.ui.space( 'contents' ).setHtml( '' );
 
 			editor.mode = '';
-		}
+		} else
+			editor._.previousModeData = editor.getData( 1 );
 
 		// Fire the mode handler.
 		this._.modes[ newMode ]( function() {
@@ -174,6 +197,14 @@ CKEDITOR.replaceClass = 'ckeditor';
 			if ( isDirty !== undefined )
 				!isDirty && editor.resetDirty();
 
+			if ( unlockSnapshot )
+				editor.fire( 'unlockSnapshot' );
+			// Since snapshot made on dataReady (which normally catches changes done by setData)
+			// won't work because editor.mode was not set yet (it's set in this function), we need
+			// to make special snapshot for changes done in source mode here.
+			else if ( newMode == 'wysiwyg' )
+				editor.fire( 'saveSnapshot' );
+
 			// Delay to avoid race conditions (setMode inside setMode).
 			setTimeout( function() {
 				editor.fire( 'mode' );
diff --git a/core/editable.js b/core/editable.js
@@ -28,6 +28,21 @@
 
 			this.editor = editor;
 
+			/**
+			 * Indicates editable initialization status. The following statuses are available:
+			 *
+			 *	* **unloaded**: the initial state; editable's instance has been created but
+			 *	is not fully loaded (in particular has no data),
+			 *	* **ready**: editable is fully initialized; `ready` status is set after
+			 *	first {@link CKEDITOR.editor#method-setData} has been called.
+			 *	* **detached**: the editable has been detached.
+			 *
+			 * @since 4.3.3
+			 * @readonly
+			 * @property {String}
+			 */
+			this.status = 'unloaded';
+
 			/**
 			 * Indicate whether the editable element has gained focus.
 			 *
@@ -398,6 +413,11 @@
 					data = this.editor.dataProcessor.toHtml( data );
 
 				this.setHtml( data );
+
+				// Editable is ready after first setData.
+				if ( this.status == 'unloaded' )
+					this.status = 'ready';
+
 				this.editor.fire( 'dataReady' );
 			},
 
@@ -429,6 +449,8 @@
 				// Cleanup the element.
 				this.removeClass( 'cke_editable' );
 
+				this.status = 'detached';
+
 				// Save the editor reference which will be lost after
 				// calling detach from super class.
 				var editor = this.editor;
diff --git a/core/editor.js b/core/editor.js
@@ -935,10 +935,15 @@
 		 * @param {Boolean} internal Whether to suppress any event firing when copying data internally inside the editor.
 		 */
 		setData: function( data, callback, internal ) {
-			if ( callback ) {
-				this.on( 'dataReady', function( evt ) {
-					evt.removeListener();
-					callback.call( evt.editor );
+			!internal && this.fire( 'saveSnapshot' );
+
+			if ( callback || !internal ) {
+				this.once( 'dataReady', function( evt ) {
+					if ( !internal )
+						this.fire( 'saveSnapshot' );
+
+					if ( callback )
+						callback.call( evt.editor );
 				} );
 			}
 
diff --git a/core/selection.js b/core/selection.js
@@ -773,8 +773,26 @@
 			editable.attachListener( editable, 'keydown', getOnKeyDownListener( editor ), null, null, -1 );
 		} );
 
-		// Clear the cached range path before unload. (#7174)
-		editor.on( 'contentDomUnload', editor.forceNextSelectionCheck, editor );
+		editor.on( 'setData', function() {
+			// Invalidate locked selection when unloading DOM.
+			// (#9521, #5217#comment:32 and #11500#comment:11)
+			editor.unlockSelection();
+
+			// Webkit's selection will mess up after the data loading.
+			if ( CKEDITOR.env.webkit )
+				clearSelection();
+		} );
+
+		// Catch all the cases which above setData listener couldn't catch.
+		// For example: switching to source mode and destroying editor.
+		editor.on( 'contentDomUnload', function() {
+			editor.unlockSelection();
+		} );
+
+		// IE9 might cease to work if there's an object selection inside the iframe (#7639).
+		if ( CKEDITOR.env.ie9Compat )
+			editor.on( 'beforeDestroy', clearSelection, null, null, 9 );
+
 		// Check selection change on data reload.
 		editor.on( 'dataReady', function() {
 			// Clean up fake selection after setting data.
@@ -783,6 +801,7 @@
 
 			editor.selectionChange( 1 );
 		} );
+
 		// When loaded data are ready check whether hidden selection container was not loaded.
 		editor.on( 'loadSnapshot', function() {
 			// TODO replace with el.find() which will be introduced in #9764,
@@ -795,24 +814,6 @@
 				el.remove();
 		}, null, null, 100 );
 
-		function clearSelection() {
-			var sel = editor.getSelection();
-			sel && sel.removeAllRanges();
-		}
-
-		// Clear dom selection before editable destroying to fix some browser
-		// craziness.
-
-		// IE9 might cease to work if there's an object selection inside the iframe (#7639).
-		CKEDITOR.env.ie9Compat && editor.on( 'beforeDestroy', clearSelection, null, null, 9 );
-		// Webkit's selection will mess up after the data loading.
-		CKEDITOR.env.webkit && editor.on( 'setData', clearSelection );
-
-		// Invalidate locked selection when unloading DOM (e.g. after setData). (#9521)
-		editor.on( 'contentDomUnload', function() {
-			editor.unlockSelection();
-		} );
-
 		editor.on( 'key', function( evt ) {
 			if ( editor.mode != 'wysiwyg' )
 				return;
@@ -825,6 +826,11 @@
 			if ( handler )
 				return handler( { editor: editor, selected: sel.getSelectedElement(), selection: sel, keyEvent: evt } );
 		} );
+
+		function clearSelection() {
+			var sel = editor.getSelection();
+			sel && sel.removeAllRanges();
+		}
 	} );
 
 	CKEDITOR.on( 'instanceReady', function( evt ) {
diff --git a/plugins/sourcearea/plugin.js b/plugins/sourcearea/plugin.js
@@ -94,6 +94,7 @@
 		proto: {
 			setData: function( data ) {
 				this.setValue( data );
+				this.status = 'ready';
 				this.editor.fire( 'dataReady' );
 			},
 
diff --git a/plugins/undo/plugin.js b/plugins/undo/plugin.js
@@ -160,9 +160,12 @@
 			 * @param data
 			 * @param {Boolean} [data.dontUpdate] When set to `true` the last snapshot will not be updated
 			 * with the current contents and selection. Read more in the {@link CKEDITOR.plugins.undo.UndoManager#lock} method.
+			 * @param {Boolean} [data.forceUpdate] When set to `true` the last snapshot will always be updated
+			 * with the current contents and selection. Read more in the {@link CKEDITOR.plugins.undo.UndoManager#lock} method.
 			 */
 			editor.on( 'lockSnapshot', function( evt ) {
-				undoManager.lock( evt.data && evt.data.dontUpdate );
+				var data = evt.data;
+				undoManager.lock( data && data.dontUpdate, data && data.forceUpdate );
 			} );
 
 			/**
@@ -414,15 +417,21 @@
 		 * Saves a snapshot of the document image for later retrieval.
 		 */
 		save: function( onContentOnly, image, autoFireChange ) {
-			// Do not change snapshots stack when locked.
-			if ( this.locked )
+			var editor = this.editor;
+			// Do not change snapshots stack when locked, editor is not ready,
+			// editable is not ready or when editor is in mode difference than 'wysiwyg'.
+			if ( this.locked || editor.status != 'ready' || editor.mode != 'wysiwyg' )
+				return false;
+
+			var editable = editor.editable();
+			if ( !editable || editable.status != 'ready' )
 				return false;
 
 			var snapshots = this.snapshots;
 
 			// Get a content image.
 			if ( !image )
-				image = new Image( this.editor );
+				image = new Image( editor );
 
 			// Do nothing if it was not possible to retrieve an image.
 			if ( image.contents === false )
@@ -437,8 +446,7 @@
 					if ( image.equalsSelection( this.currentImage ) )
 						return false;
 				} else
-					this.editor.fire( 'change' );
-
+					editor.fire( 'change' );
 			}
 
 			// Drop future snapshots.
@@ -627,25 +635,37 @@
 		 * done during the lock will be merged into the previous snapshot or the next one. Use this option to gain
 		 * more control over this behavior. For example, it is possible to group changes done during the lock into
 		 * a separate snapshot.
+		 * @param {Boolean} [forceUpdate] When set to `true` the last snapshot will always be updated with the
+		 * current contents and selection regardless of the current state of undo manager.
+		 * When not set the last snapshot will be updated only if undo manager was up to date when locking.
+		 * Additionally, this option makes it possible to lock snapshot when editor is not in `wysiwyg` mode,
+		 * because when it's passed snapshots will not need to be compared.
 		 */
-		lock: function( dontUpdate ) {
+		lock: function( dontUpdate, forceUpdate ) {
 			if ( !this.locked ) {
 				if ( dontUpdate )
 					this.locked = { level: 1 };
 				else {
-					// Make a contents image. Don't include bookmarks, because:
-					// * we don't compare them,
-					// * there's a chance that DOM has been changed since
-					// locked (e.g. fake) selection was made, so createBookmark2 could fail.
-					// http://dev.ckeditor.com/ticket/11027#comment:3
-					var imageBefore = new Image( this.editor, true );
-
-					// If current editor content matches the tip of snapshot stack,
-					// the stack tip must be updated by unlock, to include any changes made
-					// during this period.
-					var matchedTip = this.currentImage && this.currentImage.equalsContent( imageBefore );
-
-					this.locked = { update: matchedTip ? imageBefore : null, level: 1 };
+					var update = null;
+
+					if ( forceUpdate )
+						update = true;
+					else {
+						// Make a contents image. Don't include bookmarks, because:
+						// * we don't compare them,
+						// * there's a chance that DOM has been changed since
+						// locked (e.g. fake) selection was made, so createBookmark2 could fail.
+						// http://dev.ckeditor.com/ticket/11027#comment:3
+						var imageBefore = new Image( this.editor, true );
+
+						// If current editor content matches the tip of snapshot stack,
+						// the stack tip must be updated by unlock, to include any changes made
+						// during this period.
+						if ( this.currentImage && this.currentImage.equalsContent( imageBefore ) )
+							update = imageBefore;
+					}
+
+					this.locked = { update: update, level: 1 };
 				}
 			}
 			// Increase the level of lock.
@@ -664,13 +684,20 @@
 			if ( this.locked ) {
 				// Decrease level of lock and check if equals 0, what means that undoM is completely unlocked.
 				if ( !--this.locked.level ) {
-					var updateImage = this.locked.update,
-						newImage = updateImage && new Image( this.editor, true );
+					var update = this.locked.update;
 
 					this.locked = null;
 
-					if ( updateImage && !updateImage.equalsContent( newImage ) )
+					// forceUpdate was passed to lock().
+					if ( update === true )
 						this.update();
+					// update is instance of Image.
+					else if ( update ) {
+						var newImage = new Image( this.editor, true );
+
+						if ( !update.equalsContent( newImage ) )
+							this.update();
+					}
 				}
 			}
 		}
diff --git a/plugins/wysiwygarea/plugin.js b/plugins/wysiwygarea/plugin.js
@@ -299,6 +299,10 @@
 			editor.document.$.title = this._.docTitle;
 
 		CKEDITOR.tools.setTimeout( function() {
+			// Editable is ready after first setData.
+			if ( this.status == 'unloaded' )
+				this.status = 'ready';
+
 			editor.fire( 'contentDom' );
 
 			if ( this._.isPendingFocus ) {
