From 9e2cfbc54a1ec79041962639c03cdce661e55dec Mon Sep 17 00:00:00 2001 From: Kajetan Litwinowicz Date: Tue, 19 Mar 2019 12:01:01 +0100 Subject: [PATCH 01/25] Prevent adding same styles. --- plugins/widget/plugin.js | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/plugins/widget/plugin.js b/plugins/widget/plugin.js index d8660d4a38e..88667fd817a 100644 --- a/plugins/widget/plugin.js +++ b/plugins/widget/plugin.js @@ -3638,7 +3638,8 @@ function addCustomStyleHandler() { // Styles categorized by group. It is used to prevent applying styles for the same group being used together. - var styleGroups = {}; + var styleGroups = {}, + styleDefinitions = []; /** * The class representing a widget style. It is an {@link CKEDITOR#STYLE_OBJECT object} like @@ -3875,7 +3876,7 @@ // Save and categorize style by its group. function saveStyleGroup( style ) { var widgetName = style.widget, - group; + group, styleDef; if ( !styleGroups[ widgetName ] ) { styleGroups[ widgetName ] = {}; @@ -3887,7 +3888,13 @@ styleGroups[ widgetName ][ group ] = []; } - styleGroups[ widgetName ][ group ].push( style ); + styleDef = style.getDefinition(); + + // Don't push style if it's already stored (#589). + if ( CKEDITOR.tools.indexOf( styleDefinitions, styleDef ) === -1 ) { + styleDefinitions.push( styleDef ); + styleGroups[ widgetName ][ group ].push( style ); + } } } From 4343124972107e81164abcfb13a115f429245468 Mon Sep 17 00:00:00 2001 From: Kajetan Litwinowicz Date: Tue, 19 Mar 2019 12:01:16 +0100 Subject: [PATCH 02/25] Remove script listeners after load/error. --- core/loader.js | 2 ++ core/scriptloader.js | 9 +++++++++ 2 files changed, 11 insertions(+) diff --git a/core/loader.js b/core/loader.js index 2883848ca15..6931a90780e 100644 --- a/core/loader.js +++ b/core/loader.js @@ -158,6 +158,8 @@ if ( !CKEDITOR.loader ) { // Some browsers, such as Safari, may call the onLoad function // immediately. Which will break the loading sequence. (https://dev.ckeditor.com/ticket/3661) setTimeout( function() { + // Once script loaded remove listener, which might lead to memory leaks (#589). + script.onload = null; onScriptLoaded( scriptName ); }, 0 ); }; diff --git a/core/scriptloader.js b/core/scriptloader.js index a1d1e0ab53d..ddd226d374f 100644 --- a/core/scriptloader.js +++ b/core/scriptloader.js @@ -132,11 +132,13 @@ CKEDITOR.scriptLoader = ( function() { // Some browsers, such as Safari, may call the onLoad function // immediately. Which will break the loading sequence. (https://dev.ckeditor.com/ticket/3661) setTimeout( function() { + removeListeners( script ); onLoad( url, true ); }, 0 ); }; script.$.onerror = function() { + removeListeners( script ); onLoad( url, false ); }; } @@ -146,12 +148,19 @@ CKEDITOR.scriptLoader = ( function() { script.appendTo( CKEDITOR.document.getHead() ); CKEDITOR.fire( 'download', url ); // %REMOVE_LINE% + }; showBusy && CKEDITOR.document.getDocumentElement().setStyle( 'cursor', 'wait' ); for ( var i = 0; i < scriptCount; i++ ) { loadScript( scriptUrl[ i ] ); } + + function removeListeners( script ) { + // Once script loaded or failed remove listeners, which might lead to memory leaks (#589). + script.$.onload = null; + script.$.onerror = null; + } }, /** From 06af05b8289170e7f12c4a6a84ea53f41ed9c247 Mon Sep 17 00:00:00 2001 From: Kajetan Litwinowicz Date: Wed, 20 Mar 2019 10:17:02 +0100 Subject: [PATCH 03/25] Remove listener on editor destroy. --- plugins/image2/plugin.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/plugins/image2/plugin.js b/plugins/image2/plugin.js index 6abab9c4734..630f5e71e1c 100644 --- a/plugins/image2/plugin.js +++ b/plugins/image2/plugin.js @@ -1432,7 +1432,7 @@ if ( !editor.plugins.link ) return; - CKEDITOR.on( 'dialogDefinition', function( evt ) { + var listener = CKEDITOR.on( 'dialogDefinition', function( evt ) { var dialog = evt.data; if ( dialog.name == 'link' ) { @@ -1483,6 +1483,10 @@ }; } } ); + // Listener has to be removed due to leaking editor reference (#589). + editor.on( 'destroy', function() { + listener.removeListener(); + } ); // Overwrite default behaviour of unlink command. editor.getCommand( 'unlink' ).on( 'exec', function( evt ) { From 0b47c15c6488304a39ba910ad6c693360bee56d8 Mon Sep 17 00:00:00 2001 From: Kajetan Litwinowicz Date: Wed, 20 Mar 2019 11:27:00 +0100 Subject: [PATCH 04/25] Improved comparing stored styles with new ones. --- plugins/widget/plugin.js | 40 ++++++++++++++++++++++++++++++++-------- 1 file changed, 32 insertions(+), 8 deletions(-) diff --git a/plugins/widget/plugin.js b/plugins/widget/plugin.js index 88667fd817a..e3d316b7236 100644 --- a/plugins/widget/plugin.js +++ b/plugins/widget/plugin.js @@ -3876,24 +3876,48 @@ // Save and categorize style by its group. function saveStyleGroup( style ) { var widgetName = style.widget, - group, styleDef; + groupName, group; if ( !styleGroups[ widgetName ] ) { styleGroups[ widgetName ] = {}; } for ( var i = 0, l = style.group.length; i < l; i++ ) { - group = style.group[ i ]; - if ( !styleGroups[ widgetName ][ group ] ) { - styleGroups[ widgetName ][ group ] = []; + groupName = style.group[ i ]; + if ( !styleGroups[ widgetName ][ groupName ] ) { + styleGroups[ widgetName ][ groupName ] = []; } - styleDef = style.getDefinition(); + group = styleGroups[ widgetName ][ groupName ]; // Don't push style if it's already stored (#589). - if ( CKEDITOR.tools.indexOf( styleDefinitions, styleDef ) === -1 ) { - styleDefinitions.push( styleDef ); - styleGroups[ widgetName ][ group ].push( style ); + if ( !CKEDITOR.tools.array.find( group, getCompareFn( style ) ) ) { + group.push( style ); + } + } + + function getCompareFn( left ) { + return function( right ) { + return deepCompare( left.getDefinition(), right.getDefinition() ); + }; + + function deepCompare( left, right ) { + var leftKeys = CKEDITOR.tools.objectKeys( left ), + rightKeys = CKEDITOR.tools.objectKeys( right ); + + if ( leftKeys.length !== rightKeys.length ) { + return false; + } + + for ( var key in left ) { + var areSameObjects = typeof left[ key ] === 'object' && typeof right[ key ] === 'object' && deepCompare( left[ key ], right[ key ] ); + + if ( !areSameObjects && left[ key ] !== right[ key ] ) { + return false; + } + } + + return true; } } } From 7e2af54b1ff77bf760dcc775d14377f5bb25892d Mon Sep 17 00:00:00 2001 From: Kajetan Litwinowicz Date: Wed, 20 Mar 2019 12:32:40 +0100 Subject: [PATCH 05/25] Remove setHighlighter once it can't be used. --- plugins/codesnippet/plugin.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/plugins/codesnippet/plugin.js b/plugins/codesnippet/plugin.js index a9863984d9b..b956c5cc57e 100644 --- a/plugins/codesnippet/plugin.js +++ b/plugins/codesnippet/plugin.js @@ -46,6 +46,11 @@ editor._.codesnippet.langsRegex = new RegExp( '(?:^|\\s)language-(' + CKEDITOR.tools.objectKeys( langs ).join( '|' ) + ')(?:\\s|$)' ); }; + + editor.once( 'instanceReady', function() { + // Remove method once it can't be used, because it leaks editor reference (#589). + this.setHighlighter = null; + }, this ); }, onLoad: function() { From b64b6338ff254931805e3449d7e3ce8b39276a73 Mon Sep 17 00:00:00 2001 From: Kajetan Litwinowicz Date: Wed, 20 Mar 2019 12:53:17 +0100 Subject: [PATCH 06/25] Call autocomplete.destroy on editor.destroy. --- plugins/autocomplete/plugin.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/plugins/autocomplete/plugin.js b/plugins/autocomplete/plugin.js index 69249732ed5..e10cde47985 100644 --- a/plugins/autocomplete/plugin.js +++ b/plugins/autocomplete/plugin.js @@ -239,6 +239,8 @@ this.attach(); }, this ); } + + editor.on( 'destroy', this.destroy, this ); } Autocomplete.prototype = { From 954eaeee8a9495d63b78e3b14e9c2720dc2cfdac Mon Sep 17 00:00:00 2001 From: Kajetan Litwinowicz Date: Wed, 20 Mar 2019 13:42:44 +0100 Subject: [PATCH 07/25] Remove currentInstance if it's destroyed. --- core/ckeditor.js | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/core/ckeditor.js b/core/ckeditor.js index da079bd3265..1364a864f76 100644 --- a/core/ckeditor.js +++ b/core/ckeditor.js @@ -49,14 +49,19 @@ CKEDITOR.add = function( editor ) { } } ); - editor.on( 'blur', function() { + editor.on( 'blur', removeInstance ); + + // Remove currentInstance if it's destroyed (#589). + editor.on( 'destroy', removeInstance ); + + CKEDITOR.fire( 'instance', null, editor ); + + function removeInstance() { if ( CKEDITOR.currentInstance == editor ) { CKEDITOR.currentInstance = null; CKEDITOR.fire( 'currentInstance' ); } - } ); - - CKEDITOR.fire( 'instance', null, editor ); + } }; /** From e87a2c01aab44224bbc9cf4b4b331d8a83a34a9c Mon Sep 17 00:00:00 2001 From: Kajetan Litwinowicz Date: Wed, 20 Mar 2019 14:21:52 +0100 Subject: [PATCH 08/25] Remove stored node when editor is destroyed. --- core/skin.js | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/core/skin.js b/core/skin.js index d4e3f1d5b7f..a4c9a84848d 100644 --- a/core/skin.js +++ b/core/skin.js @@ -269,24 +269,32 @@ CKEDITOR.on( 'instanceLoaded', function( evt ) { // The chameleon feature is not for IE quirks. - if ( CKEDITOR.env.ie && CKEDITOR.env.quirks ) + if ( CKEDITOR.env.ie && CKEDITOR.env.quirks ) { return; + } var editor = evt.editor, showCallback = function( event ) { - var panel = event.data[ 0 ] || event.data; - var iframe = panel.element.getElementsByTag( 'iframe' ).getItem( 0 ).getFrameDocument(); + var panel = event.data[ 0 ] || event.data, + iframe = panel.element.getElementsByTag( 'iframe' ).getItem( 0 ).getFrameDocument(); // Add stylesheet if missing. if ( !iframe.getById( 'cke_ui_color' ) ) { var node = getStylesheet( iframe ); uiColorMenus.push( node ); + // Cleanup after destroying editor (#589). + editor.on( 'destroy', function() { + uiColorMenus = CKEDITOR.tools.array.filter( uiColorMenus, function( storedNode ) { + return node !== storedNode; + } ); + } ); + var color = editor.getUiColor(); // Set uiColor for new panel. - if ( color ) + if ( color ) { updateStylesheets( [ node ], CKEDITOR.skin.chameleon( editor, 'panel' ), [ [ uiColorRegexp, color ] ] ); - + } } }; From 3af7dcb05d314f87ea07ad040e33b1e125799761 Mon Sep 17 00:00:00 2001 From: Kajetan Litwinowicz Date: Wed, 20 Mar 2019 14:22:09 +0100 Subject: [PATCH 09/25] Remove element reference once it's removed. --- plugins/dialog/plugin.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/plugins/dialog/plugin.js b/plugins/dialog/plugin.js index 0d54a571310..738c73fa898 100644 --- a/plugins/dialog/plugin.js +++ b/plugins/dialog/plugin.js @@ -2228,6 +2228,9 @@ CKEDITOR.DIALOG_STATE_BUSY = 2; editor.focusManager.remove( currentCover ); var win = CKEDITOR.document.getWindow(); currentCover.hide(); + + // Remove current cover reference once it's removed (#589). + currentCover = null; win.removeListener( 'resize', resizeCover ); if ( CKEDITOR.env.ie6Compat ) { From d5bcd3d2ab1edfa79801596cf3397b5a3ca962df Mon Sep 17 00:00:00 2001 From: Kajetan Litwinowicz Date: Wed, 20 Mar 2019 15:36:39 +0100 Subject: [PATCH 10/25] Tests: added test for script load listeners. --- tests/core/loader.js | 34 ++++++++++++++++++++++++++++++++++ tests/core/scriptloader.js | 15 +++++++++++++++ 2 files changed, 49 insertions(+) create mode 100644 tests/core/loader.js diff --git a/tests/core/loader.js b/tests/core/loader.js new file mode 100644 index 00000000000..031459c194e --- /dev/null +++ b/tests/core/loader.js @@ -0,0 +1,34 @@ +/* bender-tags: editor */ +( function() { + 'use strict'; + + bender.test( { + // Script listeners should be removed because it might cause memory leaks (#589). + 'test script load listeners removed': function() { + var loader = CKEDITOR.loader, + scriptName = loader.loadedScripts.pop(); + + getScript().remove(); + + delete loader.loadedScripts[ 's:' + scriptName ]; + + loader.load( scriptName, false ); + + getScript().on( 'load', function() { + var script = this; + + setTimeout( function() { + resume( function() { + assert.isFalse( !!script.$.onreadystatechange ); + assert.isFalse( !!script.$.onload ); + } ); + }, 50 ); + } ); + + wait(); + function getScript() { + return CKEDITOR.document.findOne( 'script[src="' + CKEDITOR.getUrl( 'core/' + scriptName + '.js' ) + '"]' ); + } + } + } ); +} )(); diff --git a/tests/core/scriptloader.js b/tests/core/scriptloader.js index f0f0a03328a..c89bdfe06fa 100644 --- a/tests/core/scriptloader.js +++ b/tests/core/scriptloader.js @@ -104,6 +104,21 @@ var tests = { ] ); this.wait(); + }, + + // Script listeners should be removed because it might cause memory leaks (#589). + 'test script listeners removed when loaded': function() { + CKEDITOR.scriptLoader.load( '../_assets/sample.js', function() { + resume( function() { + var script = CKEDITOR.document.findOne( 'script[src="../_assets/sample.js"]' ); + + assert.isFalse( !!script.$.onreadystatechange ); + assert.isFalse( !!script.$.onload ); + assert.isFalse( !!script.$.onerror ); + } ); + } ); + + wait(); } }; From 0b8ba728ab0677d70d5982e8f67d41fb8a83136c Mon Sep 17 00:00:00 2001 From: Kajetan Litwinowicz Date: Wed, 20 Mar 2019 17:12:09 +0100 Subject: [PATCH 11/25] Tests: added unit test. --- tests/plugins/image2/dialoglistener.html | 1 + tests/plugins/image2/dialoglistener.js | 29 ++++++++++++++++++++++++ 2 files changed, 30 insertions(+) create mode 100644 tests/plugins/image2/dialoglistener.html create mode 100644 tests/plugins/image2/dialoglistener.js diff --git a/tests/plugins/image2/dialoglistener.html b/tests/plugins/image2/dialoglistener.html new file mode 100644 index 00000000000..afc6120991d --- /dev/null +++ b/tests/plugins/image2/dialoglistener.html @@ -0,0 +1 @@ + diff --git a/tests/plugins/image2/dialoglistener.js b/tests/plugins/image2/dialoglistener.js new file mode 100644 index 00000000000..375c0f847cd --- /dev/null +++ b/tests/plugins/image2/dialoglistener.js @@ -0,0 +1,29 @@ +/* bender-tags: editor,widget */ +/* bender-ckeditor-plugins: wysiwygarea, image2, dialog, link */ + +( function() { + 'use strict'; + + bender.test( { + 'test dialog listener removed after editor destroy': function() { + CKEDITOR.replace( 'editor', { + on: { + instanceReady: function() { + var listeners = CKEDITOR._.events.dialogDefinition.listeners, + listener = listeners[ 0 ]; + + this.on( 'destroy', function() { + resume( function() { + assert.areNotSame( listeners[ 0 ], listener, 'Listener is removed.' ); + } ); + } ); + + this.destroy(); + } + } + } ); + + wait(); + } + } ); +} )(); From ca059f7c107653471dc8d8ac816ed99b0b8d85b1 Mon Sep 17 00:00:00 2001 From: Kajetan Litwinowicz Date: Wed, 20 Mar 2019 17:25:06 +0100 Subject: [PATCH 12/25] Change event type to pluginsLoaded. --- plugins/codesnippet/plugin.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/codesnippet/plugin.js b/plugins/codesnippet/plugin.js index b956c5cc57e..6723f3763d2 100644 --- a/plugins/codesnippet/plugin.js +++ b/plugins/codesnippet/plugin.js @@ -47,7 +47,7 @@ CKEDITOR.tools.objectKeys( langs ).join( '|' ) + ')(?:\\s|$)' ); }; - editor.once( 'instanceReady', function() { + editor.once( 'pluginsLoaded', function() { // Remove method once it can't be used, because it leaks editor reference (#589). this.setHighlighter = null; }, this ); From 62a422a1bccf0aa6c7bbe4211f82c7a7ffd224e9 Mon Sep 17 00:00:00 2001 From: Kajetan Litwinowicz Date: Wed, 20 Mar 2019 17:37:39 +0100 Subject: [PATCH 13/25] Tests: added test case and updated tests after changes. --- tests/plugins/codesnippet/highlighter.js | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/tests/plugins/codesnippet/highlighter.js b/tests/plugins/codesnippet/highlighter.js index 2c9d92789dd..7330fb4a9b2 100644 --- a/tests/plugins/codesnippet/highlighter.js +++ b/tests/plugins/codesnippet/highlighter.js @@ -4,7 +4,18 @@ ( function() { 'use strict'; - bender.editor = true; + var setHighlighter; + + bender.editor = { + config: { + on: { + pluginsLoaded: function() { + // Normally setHighlighter can be used only during plugin init hooks, after that it is removed, so keep it for testing purposes. + setHighlighter = CKEDITOR.plugins.registered.codesnippet.setHighlighter; + } + } + } + }; var objToArray = bender.tools.objToArray, html = '
' +
@@ -20,6 +31,11 @@
 	}
 
 	bender.test( {
+		// (#589)
+		'test setHighlighter method is absent': function() {
+			assert.isNull( CKEDITOR.plugins.registered.codesnippet.setHighlighter );
+		},
+
 		'test highlighter: change highlighter': function() {
 			var editor = this.editor,
 				langs = {};
@@ -29,7 +45,7 @@
 				highlighter: function() {}
 			} );
 
-			editor.plugins.codesnippet.setHighlighter( highlighter );
+			setHighlighter( highlighter );
 
 			assert.areEqual( highlighter, getHighlighter( editor ), 'Highlighter has not been changed' );
 			assert.areEqual( langs, getLangs( this.editor ), 'Highlighter languages has not been changed' );
@@ -56,7 +72,7 @@
 				}
 			} );
 
-			editor.plugins.codesnippet.setHighlighter( highlighter );
+			setHighlighter( highlighter );
 
 			this.editorBot.setData( html, function() {
 				var widget = objToArray( editor.widgets.instances )[ 0 ];
@@ -97,7 +113,7 @@
 				}
 			} );
 
-			editor.plugins.codesnippet.setHighlighter( highlighter );
+			setHighlighter( highlighter );
 
 			this.editorBot.setData( html, function() {
 				var widget = objToArray( editor.widgets.instances )[ 0 ];

From d5102be72c3d79c056bbd0490a0a4f8ffbc30023 Mon Sep 17 00:00:00 2001
From: Kajetan Litwinowicz 
Date: Wed, 20 Mar 2019 18:23:22 +0100
Subject: [PATCH 14/25] Tests: added test cast.

---
 tests/plugins/autocomplete/autocomplete.html |  1 +
 tests/plugins/autocomplete/autocomplete.js   | 19 +++++++++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/tests/plugins/autocomplete/autocomplete.html b/tests/plugins/autocomplete/autocomplete.html
index 76d884149db..930396906e5 100644
--- a/tests/plugins/autocomplete/autocomplete.html
+++ b/tests/plugins/autocomplete/autocomplete.html
@@ -1 +1,2 @@
 
+ diff --git a/tests/plugins/autocomplete/autocomplete.js b/tests/plugins/autocomplete/autocomplete.js index ca76a94961e..e20cb6d557c 100644 --- a/tests/plugins/autocomplete/autocomplete.js +++ b/tests/plugins/autocomplete/autocomplete.js @@ -562,6 +562,25 @@ } ); } ); + wait(); + }, + + // (#589) + 'test autocomplete is destroyed with editor': function() { + var editor = CKEDITOR.replace( 'destroy' ), + ac = new CKEDITOR.plugins.autocomplete( editor, configDefinition ), + spy = sinon.spy( ac, 'destroy' ); + + editor.on( 'destroy', function() { + setTimeout( function() { + resume( function() { + assert.isTrue( spy.called ); + } ); + } ); + } ); + + editor.destroy(); + wait(); } } ); From 41a5ee340cf0a793c76629db1e859e0759e4fd0c Mon Sep 17 00:00:00 2001 From: Kajetan Litwinowicz Date: Wed, 20 Mar 2019 18:24:08 +0100 Subject: [PATCH 15/25] Fixed typeerror when destroy is called when view never was attached. --- plugins/autocomplete/plugin.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/plugins/autocomplete/plugin.js b/plugins/autocomplete/plugin.js index e10cde47985..b2edd6921b5 100644 --- a/plugins/autocomplete/plugin.js +++ b/plugins/autocomplete/plugin.js @@ -240,7 +240,9 @@ }, this ); } - editor.on( 'destroy', this.destroy, this ); + editor.on( 'destroy', function() { + this.destroy(); + }, this ); } Autocomplete.prototype = { @@ -361,7 +363,7 @@ this._listeners = []; - this.view.element.remove(); + this.view.element && this.view.element.remove(); }, /** From d31edb4500b2b02a016687c5602536f22a5a82e3 Mon Sep 17 00:00:00 2001 From: Kajetan Litwinowicz Date: Wed, 20 Mar 2019 18:31:41 +0100 Subject: [PATCH 16/25] Tests: added unit test. --- tests/core/editor/currentInstance.js | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 tests/core/editor/currentInstance.js diff --git a/tests/core/editor/currentInstance.js b/tests/core/editor/currentInstance.js new file mode 100644 index 00000000000..755795a449d --- /dev/null +++ b/tests/core/editor/currentInstance.js @@ -0,0 +1,27 @@ +/* bender-tags: editor */ +/* bender-ckeditor-plugins: wysiwygarea */ + +( function() { + 'use strict'; + + bender.editor = {}; + + bender.test( { + // (#589) + 'test currentInstance after editor destroy': function() { + this.editor.fire( 'focus' ); + this.editor.on( 'destroy', function() { + setTimeout( function() { + resume( function() { + assert.isNull( CKEDITOR.currentInstance ); + } ); + } ); + } ); + + assert.areSame( this.editor, CKEDITOR.currentInstance ); + + this.editor.destroy(); + wait(); + } + } ); +} )(); From 091a0e99e472443770c288d21047f19767469b64 Mon Sep 17 00:00:00 2001 From: Kajetan Litwinowicz Date: Fri, 15 Mar 2019 11:39:31 +0100 Subject: [PATCH 17/25] Moved backdoor property to private editor properties. --- plugins/magicline/plugin.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/magicline/plugin.js b/plugins/magicline/plugin.js index 5887681d008..ba3f8adb1ed 100644 --- a/plugins/magicline/plugin.js +++ b/plugins/magicline/plugin.js @@ -340,7 +340,7 @@ // This one allows testing and debugging. It reveals some // inner methods to the world. - this.backdoor = { + editor._.magiclineBackdoor = { accessFocusSpace: accessFocusSpace, boxTrigger: boxTrigger, isLine: isLine, From 595986390629a664c5405e1f657ab7070bef6d7c Mon Sep 17 00:00:00 2001 From: Kajetan Litwinowicz Date: Fri, 15 Mar 2019 11:39:41 +0100 Subject: [PATCH 18/25] Tests: updated after changes. --- tests/plugins/magicline/magicline.js | 2 +- tests/plugins/magicline/widgets.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/plugins/magicline/magicline.js b/tests/plugins/magicline/magicline.js index cb272e57675..dac56a68061 100644 --- a/tests/plugins/magicline/magicline.js +++ b/tests/plugins/magicline/magicline.js @@ -44,7 +44,7 @@ ); tc.resume( function() { - callback( editor, editable, editor.plugins.magicline.backdoor ); + callback( editor, editable, editor._.magiclineBackdoor ); } ); } } diff --git a/tests/plugins/magicline/widgets.js b/tests/plugins/magicline/widgets.js index bf30becb77b..9dceabd1ec5 100644 --- a/tests/plugins/magicline/widgets.js +++ b/tests/plugins/magicline/widgets.js @@ -149,7 +149,7 @@ editor = this.editorBot.editor; doc = editor.document; - var backdoor = editor.plugins.magicline.backdoor; + var backdoor = editor._.magiclineBackdoor; this.editorBot.setData( html, function() { if ( cfg.that.element ) @@ -184,7 +184,7 @@ editor = this.editorBot.editor; doc = editor.document; - var backdoor = editor.plugins.magicline.backdoor; + var backdoor = editor._.magiclineBackdoor; this.editorBot.setData( html, function() { var widget = cfg.widget(); From 14268659c81b942f878a156da737783afe1a5501 Mon Sep 17 00:00:00 2001 From: Kajetan Litwinowicz Date: Fri, 15 Mar 2019 11:47:41 +0100 Subject: [PATCH 19/25] Tests: added unit test. --- tests/plugins/magicline/memory.js | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 tests/plugins/magicline/memory.js diff --git a/tests/plugins/magicline/memory.js b/tests/plugins/magicline/memory.js new file mode 100644 index 00000000000..8765fd88a70 --- /dev/null +++ b/tests/plugins/magicline/memory.js @@ -0,0 +1,19 @@ +/* bender-tags: editor,magicline */ +/* bender-ckeditor-plugins: magicline */ + +( function() { + 'use strict'; + + bender.editor = {}; + + bender.test( { + // (#589) + 'test magicline backdoor reference is removed after editor.destroy': function() { + var editor = this.editor; + + editor.destroy(); + + assert.isUndefined( CKEDITOR.plugins.registered.magicline.backdoor ); + } + } ); +} )(); From 9200c0b1e3f970d308dd62101e6dff8659b60f56 Mon Sep 17 00:00:00 2001 From: Kajetan Litwinowicz Date: Thu, 21 Mar 2019 13:21:07 +0100 Subject: [PATCH 20/25] Changelog: added new entry. --- CHANGES.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGES.md b/CHANGES.md index f6186b58b53..3c0cf842e5b 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -5,6 +5,7 @@ Fixed Issues: +* [#589](https://github.com/ckeditor/ckeditor-dev/issues/589): Fixed: Editor has many sources of potential memory leaks. * [#1397](https://github.com/ckeditor/ckeditor-dev/issues/1397): Fixed: Using dialog to remove headers from the [table](https://ckeditor.com/cke4/addon/table) with one headers row only throws an error. * [#1479](https://github.com/ckeditor/ckeditor-dev/issues/1479): Fixed: [Justification](https://ckeditor.com/cke4/addon/justify) for styled content in BR mode is disabled. * [#2816](https://github.com/ckeditor/ckeditor-dev/issues/2816): Fixed: [Enhanced Image](https://ckeditor.com/cke4/addon/image2) resize handler is visible in [read-only mode](https://ckeditor.com/docs/ckeditor4/latest/guide/dev_readonly.html). From 80865ba97ced51302fa7672995a033b83f4d6aca Mon Sep 17 00:00:00 2001 From: Kajetan Litwinowicz Date: Fri, 22 Mar 2019 14:33:22 +0100 Subject: [PATCH 21/25] Copied absent CKEDITOR.tools.array.find into local function. --- plugins/widget/plugin.js | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/plugins/widget/plugin.js b/plugins/widget/plugin.js index e3d316b7236..a0d7e36cd20 100644 --- a/plugins/widget/plugin.js +++ b/plugins/widget/plugin.js @@ -3891,11 +3891,26 @@ group = styleGroups[ widgetName ][ groupName ]; // Don't push style if it's already stored (#589). - if ( !CKEDITOR.tools.array.find( group, getCompareFn( style ) ) ) { + if ( !find( group, getCompareFn( style ) ) ) { group.push( style ); } } + // Copied `CKEDITOR.tools.array` from major branch. + function find( array, fn, thisArg ) { + var length = array.length, + i = 0; + + while ( i < length ) { + if ( fn.call( thisArg, array[ i ], i, array ) ) { + return array[ i ]; + } + i++; + } + + return undefined; + } + function getCompareFn( left ) { return function( right ) { return deepCompare( left.getDefinition(), right.getDefinition() ); From 18a8fbe577bd93be18ff9c2e8dd39c6420cc1680 Mon Sep 17 00:00:00 2001 From: Kajetan Litwinowicz Date: Fri, 22 Mar 2019 15:58:37 +0100 Subject: [PATCH 22/25] Tests: ignore on built version. --- tests/core/loader.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/core/loader.js b/tests/core/loader.js index 031459c194e..a6440a44d5a 100644 --- a/tests/core/loader.js +++ b/tests/core/loader.js @@ -5,8 +5,14 @@ bender.test( { // Script listeners should be removed because it might cause memory leaks (#589). 'test script load listeners removed': function() { - var loader = CKEDITOR.loader, - scriptName = loader.loadedScripts.pop(); + var loader = CKEDITOR.loader; + + if ( !loader ) { + // All core modules are bundled in release version, so we can't and don't need to test it there. + assert.ignore(); + } + + var scriptName = loader.loadedScripts.pop(); getScript().remove(); From b0e5d10b9a9a72c0aa3a0d3d5603f447947d6f2f Mon Sep 17 00:00:00 2001 From: Kajetan Litwinowicz Date: Fri, 22 Mar 2019 15:58:51 +0100 Subject: [PATCH 23/25] Tests: fixed on built version. --- tests/plugins/image2/dialoglistener.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/plugins/image2/dialoglistener.js b/tests/plugins/image2/dialoglistener.js index 375c0f847cd..f1f49c5ec82 100644 --- a/tests/plugins/image2/dialoglistener.js +++ b/tests/plugins/image2/dialoglistener.js @@ -10,11 +10,11 @@ on: { instanceReady: function() { var listeners = CKEDITOR._.events.dialogDefinition.listeners, - listener = listeners[ 0 ]; + listener = listeners[ listeners.length - 1 ]; this.on( 'destroy', function() { resume( function() { - assert.areNotSame( listeners[ 0 ], listener, 'Listener is removed.' ); + arrayAssert.doesNotContain( listener, listeners, 'Listener is removed' ); } ); } ); From 214722c3844be0aded029db2a0bfff821c2672d6 Mon Sep 17 00:00:00 2001 From: Kajetan Litwinowicz Date: Mon, 25 Mar 2019 16:31:28 +0100 Subject: [PATCH 24/25] Tests: fixed on IE. --- tests/core/loader.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/core/loader.js b/tests/core/loader.js index a6440a44d5a..5a207273ee5 100644 --- a/tests/core/loader.js +++ b/tests/core/loader.js @@ -20,9 +20,9 @@ loader.load( scriptName, false ); - getScript().on( 'load', function() { - var script = this; + var script = getScript(); + script.on( script.$.onload ? 'load' : 'readystatechange', function() { setTimeout( function() { resume( function() { assert.isFalse( !!script.$.onreadystatechange ); @@ -32,6 +32,7 @@ } ); wait(); + function getScript() { return CKEDITOR.document.findOne( 'script[src="' + CKEDITOR.getUrl( 'core/' + scriptName + '.js' ) + '"]' ); } From 8e0dd657decf2e850fa06e4bbff1d73e2417abd2 Mon Sep 17 00:00:00 2001 From: Kajetan Litwinowicz Date: Mon, 25 Mar 2019 16:40:06 +0100 Subject: [PATCH 25/25] Changelog: rephrased entry. --- CHANGES.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index 3c0cf842e5b..5a617ef8588 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -5,7 +5,7 @@ Fixed Issues: -* [#589](https://github.com/ckeditor/ckeditor-dev/issues/589): Fixed: Editor has many sources of potential memory leaks. +* [#589](https://github.com/ckeditor/ckeditor-dev/issues/589): Fixed: Editor causes memory leaks in create / destroy cycles. * [#1397](https://github.com/ckeditor/ckeditor-dev/issues/1397): Fixed: Using dialog to remove headers from the [table](https://ckeditor.com/cke4/addon/table) with one headers row only throws an error. * [#1479](https://github.com/ckeditor/ckeditor-dev/issues/1479): Fixed: [Justification](https://ckeditor.com/cke4/addon/justify) for styled content in BR mode is disabled. * [#2816](https://github.com/ckeditor/ckeditor-dev/issues/2816): Fixed: [Enhanced Image](https://ckeditor.com/cke4/addon/image2) resize handler is visible in [read-only mode](https://ckeditor.com/docs/ckeditor4/latest/guide/dev_readonly.html).