diff --git a/CHANGES.md b/CHANGES.md index 8f2bdc0880d..cd4bb4aec85 100644 --- a/CHANGES.md +++ b/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. diff --git a/core/dom/node.js b/core/dom/node.js index 6900d27e40a..a620c48a2c1 100644 --- a/core/dom/node.js +++ b/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; }, diff --git a/plugins/widget/plugin.js b/plugins/widget/plugin.js index 008c168bb69..4887bd91f04 100644 --- a/plugins/widget/plugin.js +++ b/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', diff --git a/tests/core/dom/node.html b/tests/core/dom/node.html index 248d5d31aa3..f97ff3f7353 100644 --- a/tests/core/dom/node.html +++ b/tests/core/dom/node.html @@ -31,4 +31,9 @@

Title

text -

\ No newline at end of file +

+
+
+ +
+
diff --git a/tests/core/dom/node.js b/tests/core/dom/node.js index 7ce06e52d6b..95518c4676e 100644 --- a/tests/core/dom/node.js +++ b/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 @@ } ); -}() ); \ No newline at end of file +}() ); diff --git a/tests/plugins/widget/manual/block.html b/tests/plugins/widget/manual/block.html index 351d86abb46..792ba2c829d 100644 --- a/tests/plugins/widget/manual/block.html +++ b/tests/plugins/widget/manual/block.html @@ -99,9 +99,33 @@

Broadcasting and quotes

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: '
', + 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' ); - \ No newline at end of file + CKEDITOR.inline( 'editor2', { + extraPlugins: 'doublecolumn', + allowedContent: true + } ); + diff --git a/tests/plugins/widget/manual/block.md b/tests/plugins/widget/manual/block.md index 9634cd5bd96..d9ce81cf1ea 100644 --- a/tests/plugins/widget/manual/block.md +++ b/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. \ No newline at end of file + - undo/redo, + - switch multiple times between source and wysiwyg mode. diff --git a/tests/plugins/widget/manual/inline.html b/tests/plugins/widget/manual/inline.html index 29897f062d1..8f7fa37d8b4 100644 --- a/tests/plugins/widget/manual/inline.html +++ b/tests/plugins/widget/manual/inline.html @@ -73,13 +73,36 @@

Broadcasting and quotes

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: '
', + 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 } ); - \ No newline at end of file + diff --git a/tests/plugins/widget/manual/inline.md b/tests/plugins/widget/manual/inline.md index 687e8eb5b0f..e290e4a8b6c 100644 --- a/tests/plugins/widget/manual/inline.md +++ b/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. \ No newline at end of file + - undo/redo, + - switch multiple times between source and wysiwyg mode. diff --git a/tests/plugins/widget/nestedwidgets.js b/tests/plugins/widget/nestedwidgets.js index cf061fe438a..698a72c99db 100644 --- a/tests/plugins/widget/nestedwidgets.js +++ b/tests/plugins/widget/nestedwidgets.js @@ -344,6 +344,71 @@ editor.execCommand( 'paste', html ); } ); } ); + }, + + 'test findOneNotNested': function() { + var editor = this.editors.editor; + + var editorHtml = + '
' + + '
' + + '
' + // Added wrapper so we simulate that nested widget is already initialized. + '
' + + '
' + + '
' + + '
' + + '
'; + + 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: '
', + editables: { + col1: { selector: '.col1' }, + col2: { selector: '.col2' } + }, + upcast: function( element ) { + return element.hasClass( 'testwidget' ); + } + } ); + + // Test widget nested inside test widget. + var editorHtml = + '
' + + '
' + + '
' + + '
' + + '
' + + '
'; + + 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' ); + } ); } } ); } )();