Skip to content

Commit

Permalink
Merge branch 't/12018' into major
Browse files Browse the repository at this point in the history
  • Loading branch information
Reinmar committed Jun 3, 2014
2 parents 965630a + 89b013b commit 511070d
Show file tree
Hide file tree
Showing 5 changed files with 460 additions and 41 deletions.
61 changes: 52 additions & 9 deletions plugins/widget/plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@
* Checks if all widget instances are still present in the DOM.
* Destroys those instances that are not present.
* Reinitializes widgets on widget wrappers for which widget instances
* cannot be found.
* cannot be found. Takes nested widgets into account too.
*
* This method triggers the {@link #event-checkWidgets} event whose listeners
* can cancel the method's execution or modify its options.
Expand Down Expand Up @@ -321,7 +321,7 @@
},

/**
* Destroys the widget instance.
* Destroys the widget instance and all its nested widgets (widgets inside its nested editables).
*
* @param {CKEDITOR.plugins.widget} widget The widget instance to be destroyed.
* @param {Boolean} [offline] Whether the widget is offline (detached from the DOM tree) —
Expand All @@ -341,8 +341,32 @@
*
* @param {Boolean} [offline] Whether the widgets are offline (detached from the DOM tree) —
* in this case the DOM (attributes, classes, etc.) will not be cleaned up.
* @param {CKEDITOR.dom.element} [container] Container within widgets will be destroyed.
* This option will be ignored if `offline` flag was set to `true`, because in such case
* it is not possible to find widgets within passed block.
*/
destroyAll: function( offline ) {
destroyAll: function( offline, container ) {
if ( container && !offline ) {
var wrappers = container.find( '.cke_widget_wrapper' ),
l = wrappers.count(),
i = 0,
widget;

// Length is constant, because this is not a live node list.
// Note: since querySelectorAll returns nodes in document order,
// outer widgets are always placed before their nested widgets and therefore
// are destroyed before them.
for ( ; i < l; ++i ) {
widget = this.getByElement( wrappers.getItem( i ), true );
// Widget might not be found, because it could be a nested widget,
// which would be destroyed when destroying its parent.
if ( widget )
this.destroy( widget );
}

return;
}

var instances = this.instances,
widget;

Expand Down Expand Up @@ -1005,7 +1029,7 @@
},

/**
* Destroys a nested editable.
* Destroys a nested editable and all nested widgets.
*
* @param {String} editableName Nested editable name.
* @param {Boolean} [offline] See {@link #method-destroy} method.
Expand All @@ -1018,6 +1042,7 @@
this.editor.focusManager.remove( editable );

if ( !offline ) {
this.repository.destroyAll( false, editable );
editable.removeClass( 'cke_widget_editable' );
editable.removeClass( 'cke_widget_editable_focused' );
editable.removeAttributes( [ 'contenteditable', 'data-cke-widget-editable', 'data-cke-enter-mode' ] );
Expand Down Expand Up @@ -1158,6 +1183,7 @@

// Finally, process editable's data. This data wasn't processed when loading
// editor's data, becuase they need to be processed separately, with its own filters and settings.
editable._.initialSetData = true;
editable.setData( editable.getHtml() );

return true;
Expand Down Expand Up @@ -1486,6 +1512,7 @@
// Call the base constructor.
CKEDITOR.dom.element.call( this, element.$ );
this.editor = editor;
this._ = {};
var filter = this.filter = config.filter;

// If blockless editable - always use BR mode.
Expand All @@ -1503,9 +1530,20 @@
* and the {@link CKEDITOR.editor#filter}. This ensures that the data was filtered and prepared to be
* edited like the {@link CKEDITOR.editor#method-setData editor data}.
*
* Before content is changed all nested widgets are destroyed. Afterwards, after new content is loaded
* all nested widgets are initialized.
*
* @param {String} data
*/
setData: function( data ) {
// For performance reasons don't call destroyAll when initializing nested editable,
// because there are not widgets inside.
if ( !this._.initialSetData ) {
// Destroy all nested widgets before setting data.
this.editor.widgets.destroyAll( false, this );
}
this._.initialSetData = false;

data = this.editor.dataProcessor.toHtml( data, {
context: this.getName(),
filter: this.filter,
Expand Down Expand Up @@ -1715,7 +1753,7 @@

var editable = this.editor.editable(),
instances = this.instances,
newInstances, i, count, wrapper;
newInstances, i, count, wrapper, notYetInitialized;

if ( !editable )
return;
Expand All @@ -1736,10 +1774,15 @@
// Create widgets on existing wrappers if they do not exists.
for ( i = 0, count = wrappers.count(); i < count; i++ ) {
wrapper = wrappers.getItem( i );

// Check if there's no instance for this widget and that
// wrapper is not inside some temporary element like copybin (#11088).
if ( !this.getByElement( wrapper, true ) && !findParent( wrapper, isDomTemp ) ) {
notYetInitialized = !this.getByElement( wrapper, true );

// Check if:
// * there's no instance for this widget
// * wrapper is not inside some temporary element like copybin (#11088)
// * it was a nested widget's wrapper which has been detached from DOM,
// when nested editable has been initialized (it overwrites its innerHTML
// and initializes nested widgets).
if ( notYetInitialized && !findParent( wrapper, isDomTemp ) && editable.contains( wrapper ) ) {
// Add cke_widget_new class because otherwise
// widget will not be created on such wrapper.
wrapper.addClass( 'cke_widget_new' );
Expand Down
11 changes: 9 additions & 2 deletions tests/plugins/widget/_helpers/tools.js
Original file line number Diff line number Diff line change
Expand Up @@ -175,8 +175,15 @@ var widgetTestsTools = ( function() {
return JSON.parse( decodeURIComponent( widget.element.data( 'cke-widget-data' ) ) );
}

function getWidgetById( editor, id ) {
return editor.widgets.getByElement( editor.document.getById( id ) );
// @param {Boolean} [byElement] If true, the passed id has to be widget element's id.
// Important for nested widgets, so parent widget is not mistakenly found.
function getWidgetById( editor, id, byElement ) {
var widget = editor.widgets.getByElement( editor.document.getById( id ) );

if ( widget && byElement )
return widget.element.$.id == id ? widget : null;

return widget;
}

// Retrives widget by its offset among parsed widgets.
Expand Down
88 changes: 88 additions & 0 deletions tests/plugins/widget/nestededitables.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@
data2Attr = widgetTestsTools.data2Attribute,
getWidgetById = widgetTestsTools.getWidgetById;

function keysLength( obj ) {
return CKEDITOR.tools.objectKeys( obj ).length;
}

function testDelKey( editor, keyName, range, shouldBeBlocked, msg ) {
range.select();

Expand Down Expand Up @@ -164,6 +168,52 @@
} );
},

'test #destroyEditable destroys nested widgets': function() {
var editor = this.editor;

editor.widgets.add( 'testmethods3', {
editables: {
foo: '#foo'
}
} );

this.editorBot.setData( '<div data-widget="testmethods3" id="w1"><p id="foo"><span data-widget="testmethods3" id="w2">x</span></p></div>', function() {
var w1 = getWidgetById( editor, 'w1' ),
w2 = getWidgetById( editor, 'w2' );

assert.areEqual( 2, keysLength( editor.widgets.instances ), '2 widgets were initialized' );

w1.destroyEditable( 'foo' );

assert.areEqual( 1, keysLength( editor.widgets.instances ), '1 widget reimained' );
assert.isNull( getWidgetById( editor, 'w2', true ), 'nested widget was destroyed' );
assert.isFalse( w2.element.getParent().hasAttribute( 'data-cke-widget-wrapper' ), 'widget was unwrapped' );
} );
},

// More precise tests can be found in widgetsrepoapi because this
// methods uses repo#destroyAll with specified container.
'test #destroyEditable in offline mode does not destroy nested widgets': function() {
var editor = this.editor;

editor.widgets.add( 'testmethods4', {
editables: {
foo: '#foo'
}
} );

this.editorBot.setData( '<div data-widget="testmethods4" id="w1"><p id="foo"><span data-widget="testmethods4" id="w2">x</span></p></div>', function() {
var w1 = getWidgetById( editor, 'w1' );

assert.areEqual( 2, keysLength( editor.widgets.instances ), '2 widgets were initialized' );

w1.destroyEditable( 'foo', true );

assert.areEqual( 2, keysLength( editor.widgets.instances ), '2 widgets reimained' );
assert.isNotNull( getWidgetById( editor, 'w2', true ), 'nested widget was not destroyed' );
} );
},

'test nestedEditable enter modes are limited by ACF': function() {
var editor = this.editor;

Expand Down Expand Up @@ -293,6 +343,44 @@
} );
},

// For performance reasons.
'test nestedEditable.setData - destroyAll(false,editable) is not called on first nestedEditable.setData': function() {
var editor = this.editor;

editor.widgets.add( 'testsetdata3', {} );

this.editorBot.setData(
'<div data-widget="testsetdata3" id="w1">' +
'<p id="foo"></p>' +
'</div>',
function() {
var w1 = getWidgetById( editor, 'w1' ),
ed = editor.document.getById( 'foo' );

ed.setHtml( '<span data-widget="testsetdata3" id="w2">x</span><span data-widget="testsetdata3" id="w3">x</span>' );

assert.areEqual( 1, keysLength( editor.widgets.instances ), '1 widget was initialized' );

var original = editor.widgets.destroyAll,
destroyAllCalls = 0,
revert = bender.tools.replaceMethod( editor.widgets, 'destroyAll', function( offline, container ) {
destroyAllCalls += 1;
original.call( this, offline, container );
} );

w1.initEditable( 'foo', { selector: '#foo' } );
assert.areSame( 0, destroyAllCalls, 'destroyAll is not called on initial nestedEditable.setData' );
assert.areEqual( 3, keysLength( editor.widgets.instances ), '3 widgets were initialized' );

w1.editables.foo.setData( '<span data-widget="testsetdata3" id="w2">x</span>' );

assert.areSame( 1, destroyAllCalls, 'destroyAll is called on 2nd+ nestedEditable.setData' );
assert.areEqual( 2, keysLength( editor.widgets.instances ), '2 widgets reimained' );
revert();
}
);
},

'test nestedEditable.getData - data processor integration': function() {
var editor = this.editor,
data = '<p>Foo</p><div data-widget="testgetdata1" id="w1"><p>A</p><p id="foo">B</p></div>';
Expand Down
42 changes: 42 additions & 0 deletions tests/plugins/widget/nestedwidgets.js
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,48 @@
assert.areSame( wn1.wrapper, wn1.dragHandlerContainer.getParent(), '1st nested widget\'s drag handler is directly in the wrapper' );
assert.areSame( wn2.wrapper, wn2.dragHandlerContainer.getParent(), '2nd nested widget\'s drag handler is directly in the wrapper' );
} );
},

'test all widgets are destroyed once when setting editor data': function() {
var editor = this.editors.editor,
bot = this.editorBots.editor,
destroyed = [];

bot.setData( generateWidgetsData( 1 ), function() {
for ( var id in editor.widgets.instances )
editor.widgets.instances[ id ].on( 'destroy', log );

bot.setData( '', function() {
assert.areSame( 'wn-0-0,wn-0-1,wp-0', destroyed.sort().join( ',' ), 'all widgets were destroyed' );
} );
} );

function log() {
destroyed.push( this.element.$.id );
}
},

'test all nested widgets are destroyed when setting nested editable data': function() {
var editor = this.editors.editor,
bot = this.editorBots.editor,
destroyed = [];

bot.setData( generateWidgetsData( 2 ), function() {
var w1 = getWidgetById( editor, 'wp-0' );

for ( var id in editor.widgets.instances )
editor.widgets.instances[ id ].on( 'destroy', log );

w1.editables.ned.setData( '<p>foo</p>' );
assert.areSame( 'wn-0-0,wn-0-1', destroyed.sort().join( ',' ), 'all widgets were destroyed' );

// Clean up widgets in this test, so next won't fire listeners added above.
editor.widgets.destroyAll();
} );

function log() {
destroyed.push( this.element.$.id );
}
}

} );
Expand Down
Loading

0 comments on commit 511070d

Please sign in to comment.