Merge branch 't/13334' into major

CHANGES.md

@@ -12,6 +12,7 @@ New Features:
 
 Fixed Issues:
 
+* [#13334](http://dev.ckeditor.com/ticket/13334): Fixed: Error after nesting widgets and playing with undo/redo.
 * [#13118](http://dev.ckeditor.com/ticket/13118): Fixed: The [`editor.getSelectedHtml()`](http://docs.ckeditor.com/#!/api/CKEDITOR.editor-method-getSelectedHtml) method throws error when called in the source mode.
 * [#13158](http://dev.ckeditor.com/ticket/13158): Fixed: Error after canceling dialog when creating a widget.
 * [#13197](http://dev.ckeditor.com/ticket/13197): Fixed: Linked inline image2's alignment class is not transferred to widget wrapper.

core/dom/node.js

@@ -515,16 +515,19 @@ CKEDITOR.tools.extend( CKEDITOR.dom.node.prototype, {
 	 *		alert( parents[ 0 ].getName() + ',' + parents[ 2 ].getName() ); // 'html,p'
 	 *
 	 * @param {Boolean} [closerFirst=false] Determines the order of returned nodes.
+	 * @param {CKEDITOR.dom.node} [lastParent=null] Guard node. Nodes that are parents of `lastParent` will be ommited.
+	 * If `parent` is `null` or `parent` is not an ancestor of this node all parents will be returned.
 	 * @returns {Array} Returns an array of {@link CKEDITOR.dom.node}.
 	 */
-	getParents: function( closerFirst ) {
+
+	getParents: function( closerFirst, lastParent ) {
 		var node = this;
 		var parents = [];
 
 		do {
 			parents[ closerFirst ? 'push' : 'unshift' ]( node );
 		}
-		while ( ( node = node.getParent() ) );
+		while ( !node.equals( lastParent ) && ( node = node.getParent() ) );
 
 		return parents;
 	},

plugins/widget/plugin.js

@@ -1204,7 +1204,8 @@
 		 * @returns {Boolean} Whether an editable was successfully initialized.
 		 */
 		initEditable: function( editableName, definition ) {
-			var editable = this.wrapper.findOne( definition.selector );
+			// Don't fetch just first element which matched selector but look for a correct one. (#13334)
+			var editable = this._findOneNotNested( definition.selector );
 
 			if ( editable && editable.is( CKEDITOR.dtd.$editable ) ) {
 				editable = new NestedEditable( this.editor, editable, {
@@ -1245,6 +1246,45 @@
 			return false;
 		},
 
+		/**
+		 * Looks inside wrapper element to find a node that
+		 * matches given selector and is not nested in other widget. (#13334)
+		 *
+		 * @since 4.5
+		 * @private
+		 * @param {String} selector Selector to match.
+		 * @returns {CKEDITOR.dom.node} Matched node or null if a node has not been found.
+		 */
+		_findOneNotNested: function( selector ) {
+			var match = null,
+				parents;
+
+			var matchedElements = this.wrapper.find( selector );
+
+			for ( var i = 0; i < matchedElements.count(); i++ ) {
+				match = matchedElements.getItem( i );
+
+				parents = match.getParents( true, this.wrapper );
+				// Don't include wrapper element.
+				parents.pop();
+
+				for ( var j = 0; j < parents.length; j++ ) {
+					// One of parents is a widget wrapper, so this match is already a part of other widget.
+					if ( Widget.isDomWidgetWrapper( parents[ j ] ) ) {
+						match = null;
+						break;
+					}
+				}
+
+				// The first match is a good match.
+				// Other matches are probably parts of other widgets instances.
+				if ( match != null )
+					break;
+			}
+
+			return match;
+		},
+
 		/**
 		 * Checks if a widget has already been initialized and has not been destroyed yet.
 		 *
@@ -3757,6 +3797,8 @@
 
 /**
  * An object containing definitions of nested editables (editable name => {@link CKEDITOR.plugins.widget.nestedEditable.definition}).
+ * Note that editables *have to* be defined in the same order as they are in DOM / {@link CKEDITOR.plugins.widget.definition#template template}.
+ * Otherwise errors will occur when nesting widgets inside each other.
  *
  *		editables: {
  *			header: 'h1',

tests/core/dom/node.html

@@ -31,4 +31,9 @@ <h1>Title</h1>
 
 	text
 
-</p>
+</p>
+<div id="guardParent">
+	<div>
+		<span id="getParentsWithGuard"></span>
+	</div>
+</div>

tests/core/dom/node.js

@@ -673,6 +673,21 @@
 
 			assert.areSame( 3, node.getParents().length );
 			assert.areSame( node.getParents()[ 0 ], node.getParents( true )[ 2 ] );
+
+		},
+
+		test_getParents_guard: function() {
+			var guard = newNode( document.getElementById( 'guardParent' ) ),
+				node = newNode( document.getElementById( 'getParentsWithGuard' ) ),
+				wrongParent = newNode( document.getElementById( 'remove' ) );
+
+			assert.areSame( 3, node.getParents( false, guard ).length );
+			// guard element is not a parent of node - all nodes up to root are returned (+ body, html)
+			assert.areSame( 5, node.getParents( false, wrongParent ).length );
+			assert.areSame( node.getParents( false, guard )[ 0 ], guard );
+			assert.areSame( node.getParents( false, guard )[ 0 ], node.getParents( true, guard )[ 2 ] );
+			// guard and base node are the same elements - we should get only that node
+			assert.isTrue( node.getParents( false, node )[ 0 ].equals( node ) );
 		},
 
 		test_getCommonAncestor: function() {
@@ -779,4 +794,4 @@
 
 	} );
 
-}() );
+}() );

tests/plugins/widget/manual/block.html

@@ -99,9 +99,33 @@ <h2>Broadcasting and <em>quotes</em> <a id="quotes" name="quotes"></a></h2>
 		CKEDITOR.tools.enableHtml5Elements( document );
 	}
 
+	CKEDITOR.addCss( '.testwidget {	border: 1px dashed #AAAAFF;	} .col1, .col2 { border: 1px dashed #AA9090; margin: 5px; }' );
+	CKEDITOR.plugins.add( 'doublecolumn', {
+		requires: 'widget',
+		button: 'doublecolumn',
+		init: function( editor ) {
+			editor.widgets.add( 'doublecolumn', {
+				button: 'doublecolumn',
+				template: '<div class="testwidget"><div class="col1"></div><div class="col2"></div></div>',
+				editables: {
+					col1: { selector: '.col1' },
+					col2: { selector: '.col2' }
+				},
+				upcast: function( element ) {
+					return element.hasClass( 'testwidget' );
+				}
+			} );
+		}
+	} );
+
 	CKEDITOR.replace( 'editor1', {
-		height: 400
+		height: 400,
+		extraPlugins: 'doublecolumn',
+		allowedContent: true
 	} );
 
-	CKEDITOR.inline( 'editor2' );
-</script>
+	CKEDITOR.inline( 'editor2', {
+		extraPlugins: 'doublecolumn',
+		allowedContent: true
+	} );
+</script>

tests/plugins/widget/manual/block.md

@@ -2,6 +2,7 @@
 @bender-ui: collapsed
 @bender-ckeditor-plugins: wysiwygarea, toolbar, sourcearea, table, undo, indent, justify, clipboard, floatingspace, basicstyles, image2, codesnippet, link, elementspath, blockquote, format, htmlwriter, list, maximize
 
+Add some widgets and nested widgets (use 'empty' icon for that).
 Test block widgets features:
  - create,
  - edit,
@@ -10,4 +11,5 @@ Test block widgets features:
  - cut/copy and paste,
  - editing in nested editable,
  - remove,
- - undo/redo.
+ - undo/redo,
+ - switch multiple times between source and wysiwyg mode.

tests/plugins/widget/manual/inline.html

@@ -73,13 +73,36 @@ <h2>Broadcasting and <em>quotes</em> <a id="quotes" name="quotes"></a></h2>
 	if ( CKEDITOR.env.ie && CKEDITOR.env.version < 9 ) {
 		CKEDITOR.tools.enableHtml5Elements( document );
 	}
+	CKEDITOR.addCss( '.testwidget {	border: 1px dashed #AAAAFF;	} .col1, .col2 { border: 1px dashed #AA9090; margin: 5px; }' );
+	CKEDITOR.plugins.add( 'doublecolumn', {
+		requires: 'widget',
+		button: 'doublecolumn',
+		init: function( editor ) {
+			editor.widgets.add( 'doublecolumn', {
+				button: 'doublecolumn',
+				template: '<div class="testwidget"><div class="col1"></div><div class="col2"></div></div>',
+				editables: {
+					col1: { selector: '.col1' },
+					col2: { selector: '.col2' }
+				},
+				upcast: function( element ) {
+					return element.hasClass( 'testwidget' );
+				}
+			} );
+		}
+	} );
+
 
 	CKEDITOR.replace( 'editor1', {
 		height: 400,
-		mathJaxLib: '../../mathjax/' + bender.config.mathJaxLibPath
+		mathJaxLib: '../../mathjax/' + bender.config.mathJaxLibPath,
+		extraPlugins: 'doublecolumn',
+		allowedContent: true
 	} );
 
 	CKEDITOR.inline( 'editor2', {
-		mathJaxLib: '../../mathjax/' + bender.config.mathJaxLibPath
+		mathJaxLib: '../../mathjax/' + bender.config.mathJaxLibPath,
+		extraPlugins: 'doublecolumn',
+		allowedContent: true
 	} );
-</script>
+</script>

tests/plugins/widget/manual/inline.md

@@ -7,6 +7,8 @@ Test inline widgets features:
  - edit,
  - select,
  - drag and drop,
- - copy and paste,
+ - cut/copy and paste,
+ - editing in nested editable,
  - remove,
- - undo/redo.
+ - undo/redo,
+ - switch multiple times between source and wysiwyg mode.

tests/plugins/widget/nestedwidgets.js

@@ -344,6 +344,71 @@
 					editor.execCommand( 'paste', html );
 				} );
 			} );
+		},
+
+		'test findOneNotNested': function() {
+			var editor = this.editors.editor;
+
+			var editorHtml =
+				'<div data-widget="test_findCorrectEditable" id="test_findCorrectEditable">' +
+					'<div>' +
+						'<div data-cke-widget-wrapper="1">' + // Added wrapper so we simulate that nested widget is already initialized.
+							'<div data-widget="test_findCorrectEditable"><div class="col1"></div><div class="col2" id="nestedcol2"></div></div>' +
+						'</div>' +
+					'</div>' +
+					'<div class="col2" id="uppercol2"></div>' +
+				'</div>';
+
+			editor.widgets.add( 'test_findCorrectEditable', {} );
+
+			this.editorBots.editor.setData( editorHtml, function() {
+				var widget = getWidgetById( editor, 'test_findCorrectEditable' );
+				var col1 = widget._findOneNotNested( '.col1' );
+				var col2 = widget._findOneNotNested( '.col2' );
+
+				// .col1 is only in another widget so it should not be found.
+				assert.areSame( null, col1, 'findOneNotNested for selector .col1 returns' );
+
+				// findOneNotNested should find .col2 which is not in another widget.
+				assert.areSame( 'uppercol2', col2.getId(), 'findOneNotNested returned .col2 with id' );
+			} );
+		},
+
+		// #13334
+		'test editables are not matched from among nested widgets': function() {
+			var editor = this.editors.editor;
+
+			editor.widgets.add( 'testwidget', {
+				template: '<div class="testwidget"><div class="col1"></div><div class="col2"></div></div>',
+				editables: {
+					col1: { selector: '.col1' },
+					col2: { selector: '.col2' }
+				},
+				upcast: function( element ) {
+					return element.hasClass( 'testwidget' );
+				}
+			} );
+
+			// Test widget nested inside test widget.
+			var editorHtml =
+			'<div class="testwidget" id="upperwidget">' +
+				'<div class="col1" id="uppercol1">' +
+					'<div class="testwidget" id="nestedwidget"><div class="col1" id="nestedcol1"></div><div class="col2" id="nestedcol2"></div></div>' +
+				'</div>' +
+				'<div class="col2" id="uppercol2"></div>' +
+			'</div>';
+
+			this.editorBots.editor.setData( editorHtml, function() {
+				var widgetUpper = getWidgetById( editor, 'upperwidget' );
+				var widgetNested = getWidgetById( editor, 'nestedwidget' );
+
+				// Check if nested editables belong only to nested widget
+				// and upper edtiables belong only to upper widget.
+				assert.areSame( 'uppercol1', widgetUpper.editables.col1.getId(), 'upper widget has editable .col1 with id' );
+				assert.areSame( 'uppercol2', widgetUpper.editables.col2.getId(), 'upper widget has editable .col2 with id' );
+				assert.areSame( 'nestedcol1', widgetNested.editables.col1.getId(), 'nested widget has editable .col1 with id' );
+				assert.areSame( 'nestedcol2', widgetNested.editables.col2.getId(), 'nested widget has editable .col2 with id' );
+			} );
 		}
 	} );
 } )();