Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean up further memory leaks around filters #15

Closed
wants to merge 10 commits into from
13 changes: 9 additions & 4 deletions core/ckeditor.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
}
};

/**
Expand Down
26 changes: 24 additions & 2 deletions core/editor.js
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,9 @@
}

function initConfig( editor, instanceConfig ) {
if( editor.status === 'destroyed' ) {
return;
}
// Setup the lister for the "customConfigLoaded" event.
editor.on( 'customConfigLoaded', function() {
if ( instanceConfig ) {
Expand Down Expand Up @@ -591,6 +594,12 @@

// Load all plugin specific language files in a row.
CKEDITOR.scriptLoader.load( languageFiles, function() {
// Do not do anything if the editor has been destroyed
// before being fully initialized
if( editor.status === 'destroyed' ) {
return;
}

// Initialize all plugins that have the "beforeInit" and "init" methods defined.
var methods = [ 'beforeInit', 'init', 'afterInit' ];
for ( var m = 0; m < methods.length; m++ ) {
Expand Down Expand Up @@ -755,14 +764,27 @@
* element with the instance content.
*/
destroy: function( noUpdate ) {
var filters = CKEDITOR.filter.instances,
self = this;

this.fire( 'beforeDestroy' );

!noUpdate && updateEditorElement.call( this );

this.editable( null );

this.filter.destroy();
delete this.filter;
if (this.filter) {
delete this.filter;
}

// Destroy filters attached to the editor (#1722).
CKEDITOR.tools.objectKeys( filters ).forEach(function( id ) {
var filter = filters[ id ];
if ( self === filter.editor ) {
filter.destroy();
}
} );

delete this.activeFilter;

this.status = 'destroyed';
Expand Down
7 changes: 4 additions & 3 deletions core/filter.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@
* @constructor Creates a filter class instance.
* @param {CKEDITOR.editor/CKEDITOR.filter.allowedContentRules} editorOrRules
*/
CKEDITOR.filter = function( editorOrRules ) {
CKEDITOR.filter = function( editorOrRules, rules ) {
/**
* Whether custom {@link CKEDITOR.config#allowedContent} was set.
*
Expand Down Expand Up @@ -164,7 +164,8 @@
// Register filter instance.
CKEDITOR.filter.instances[ this.id ] = this;

if ( editorOrRules instanceof CKEDITOR.editor ) {
var editor = this.editor = editorOrRules instanceof CKEDITOR.editor ? editorOrRules : null;
if ( editor && !rules ) {
var editor = this.editor = editorOrRules;
this.customConfig = true;

Expand All @@ -190,7 +191,7 @@
// Rules object passed in editorOrRules argument - initialize standalone filter.
else {
this.customConfig = false;
this.allow( editorOrRules, 'default', 1 );
this.allow( rules || editorOrRules, 'default', 1 );
}
};

Expand Down
2 changes: 2 additions & 0 deletions core/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,8 @@ if ( !CKEDITOR.loader ) {
// Some browsers, such as Safari, may call the onLoad function
// immediately. Which will break the loading sequence. (#3661)
setTimeout( function() {
// Once script loaded remove listener, which might lead to memory leaks (#589).
script.onload = null;
onScriptLoaded( scriptName );
}, 0 );
};
Expand Down
8 changes: 8 additions & 0 deletions core/scriptloader.js
Original file line number Diff line number Diff line change
Expand Up @@ -132,12 +132,14 @@ CKEDITOR.scriptLoader = ( function() {
// Some browsers, such as Safari, may call the onLoad function
// immediately. Which will break the loading sequence. (#3661)
setTimeout( function() {
removeListeners( script );
onLoad( url, true );
}, 0 );
};

// FIXME: Opera and Safari will not fire onerror.
script.$.onerror = function() {
removeListeners( script );
onLoad( url, false );
};
}
Expand All @@ -153,6 +155,12 @@ CKEDITOR.scriptLoader = ( function() {
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;
}
},

/**
Expand Down
7 changes: 7 additions & 0 deletions core/skin.js
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,13 @@
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 )
Expand Down
86 changes: 43 additions & 43 deletions dev/builder/build-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,59 +29,59 @@ var CKBUILDER_CONFIG = {
'tests'
],
plugins: {
a11yhelp: 1,
a11yhelp: 0,
about: 1,
basicstyles: 1,
bidi: 1,
blockquote: 1,
bidi: 0,
blockquote: 0,
clipboard: 1,
colorbutton: 1,
colordialog: 1,
contextmenu: 1,
dialogadvtab: 1,
div: 1,
elementspath: 1,
colorbutton: 0,
colordialog: 0,
contextmenu: 0,
dialogadvtab: 0,
div: 0,
elementspath: 0,
enterkey: 1,
entities: 1,
filebrowser: 1,
find: 1,
flash: 1,
filebrowser: 0,
find: 0,
flash: 0,
floatingspace: 1,
font: 1,
format: 1,
forms: 1,
horizontalrule: 1,
htmlwriter: 1,
iframe: 1,
image: 1,
font: 0,
format: 0,
forms: 0,
horizontalrule: 0,
htmlwriter: 0,
iframe: 0,
image: 0,
indentlist: 1,
indentblock: 1,
justify: 1,
indentblock: 0,
justify: 0,
link: 1,
list: 1,
liststyle: 1,
magicline: 1,
maximize: 1,
newpage: 1,
pagebreak: 1,
liststyle: 0,
magicline: 0,
maximize: 0,
newpage: 0,
pagebreak: 0,
pastefromword: 1,
pastetext: 1,
preview: 1,
print: 1,
removeformat: 1,
resize: 1,
save: 1,
selectall: 1,
showblocks: 1,
showborders: 1,
smiley: 1,
sourcearea: 1,
specialchar: 1,
stylescombo: 1,
tab: 1,
table: 1,
tabletools: 1,
templates: 1,
pastetext: 0,
preview: 0,
print: 0,
removeformat: 0,
resize: 0,
save: 0,
selectall: 0,
showblocks: 0,
showborders: 0,
smiley: 0,
sourcearea: 0,
specialchar: 0,
stylescombo: 0,
tab: 0,
table: 0,
tabletools: 0,
templates: 0,
toolbar: 1,
undo: 1,
wysiwygarea: 1
Expand Down
109 changes: 57 additions & 52 deletions dev/tasks/jscs-config.json
Original file line number Diff line number Diff line change
@@ -1,53 +1,58 @@
{
"requireCurlyBraces": [
"switch", "try", "catch"
],
"requireSpaceBeforeKeywords": true,
"requireSpaceAfterKeywords": [
"do", "for", "if", "else", "switch", "case", "try", "catch", "void", "while", "with", "return", "typeof"
],
"requireSpaceBeforeBlockStatements": true,
"requireParenthesesAroundIIFE": true,
"requireSpacesInConditionalExpression": {
"afterTest": true,
"beforeConsequent": true,
"afterConsequent": true,
"beforeAlternate": true
},
"requireSpacesInFunction": {
"beforeOpeningCurlyBrace": true
},
"disallowSpacesInFunction": {
"beforeOpeningRoundBrace": true
},
"disallowSpacesInCallExpression": true,
"requireBlocksOnNewline": true,
"requireSpacesInsideObjectBrackets": "all",
"requireSpacesInsideArrayBrackets": "all",
"requireSpacesInsideParentheses": "all",
"disallowSpaceAfterObjectKeys": true,
"requireSpaceBeforeObjectValues": true,
"requireCommaBeforeLineBreak": true,
"requireOperatorBeforeLineBreak": true,
"disallowSpaceAfterPrefixUnaryOperators": true,
"disallowSpaceBeforePostfixUnaryOperators": true,
"requireSpaceBeforeBinaryOperators": true,
"requireSpaceAfterBinaryOperators": true,
"disallowKeywords": [
"with"
],
"disallowMultipleLineStrings": true,
"disallowMixedSpacesAndTabs": true,
"disallowTrailingWhitespace": true,
"maximumLineLength": 200,
"requireDotNotation": true,
"requireDotNotation": true,
"disallowYodaConditions": true,
"disallowNewlineBeforeBlockStatements": true,
"validateLineBreaks": "LF",
"validateQuoteMarks": {
"mark": "'",
"escape": true
},
"validateIndentation": "\t"
}
"es3": true,
"requireCurlyBraces": [
"switch", "try", "catch"
],
"requireSpaceBeforeKeywords": true,
"requireSpaceAfterKeywords": [
"do", "for", "if", "else", "switch", "case", "try", "catch", "void", "while", "with", "return", "typeof"
],
"requireSpaceBeforeBlockStatements": true,
"requireParenthesesAroundIIFE": true,
"requireSpacesInConditionalExpression": {
"afterTest": true,
"beforeConsequent": true,
"afterConsequent": true,
"beforeAlternate": true
},
"requireSpacesInFunction": {
"beforeOpeningCurlyBrace": true
},
"disallowSpacesInFunction": {
"beforeOpeningRoundBrace": true
},
"disallowSpacesInCallExpression": true,
"requireBlocksOnNewline": true,
"requireSpacesInsideObjectBrackets": "all",
"requireSpacesInsideArrayBrackets": "all",
"requireSpacesInsideParentheses": "all",
"disallowSpaceAfterObjectKeys": true,
"requireSpaceBeforeObjectValues": true,
"requireCommaBeforeLineBreak": true,
"requireOperatorBeforeLineBreak": true,
"disallowSpaceAfterPrefixUnaryOperators": true,
"disallowSpaceBeforePostfixUnaryOperators": true,
"requireSpaceBeforeBinaryOperators": true,
"requireSpaceAfterBinaryOperators": true,
"disallowKeywords": [
"with"
],
"disallowMultipleLineStrings": true,
"disallowMixedSpacesAndTabs": true,
"disallowTrailingWhitespace": true,
"maximumLineLength": 200,
"requireDotNotation": {
"allExcept": [ "keywords" ]
},
"disallowYodaConditions": true,
"disallowNewlineBeforeBlockStatements": true,
"validateLineBreaks": "LF",
"validateQuoteMarks": {
"mark": "'",
"escape": true
},
"validateIndentation": {
"value": "\t",
"allExcept": [ "comments" ]
}
}
18 changes: 9 additions & 9 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,16 @@
"version": "4.4.8",
"description": "The development version of CKEditor - JavaScript WYSIWYG web text editor.",
"devDependencies": {
"benderjs": "~0.2.3",
"benderjs-jquery": "~0.3.0",
"benderjs-sinon": "~0.2.1",
"benderjs-yui": "~0.2.3",
"grunt": "~0.4.5",
"grunt-contrib-imagemin": "~0.8.1",
"grunt-jscs": "~1.5.0",
"grunt-contrib-jshint": "~0.11.0",
"benderjs": "^0.4.1",
"benderjs-jquery": "^0.3.0",
"benderjs-sinon": "^0.2.1",
"benderjs-yui": "^0.3.2",
"grunt": "^0",
"grunt-contrib-imagemin": "^0",
"grunt-jscs": "^2.0.0",
"grunt-contrib-jshint": "^0",
"grunt-githooks": "~0.3.1",
"shelljs": "~0.3.0"
"shelljs": "^0"
},
"scripts": {
"test": "echo \"Error: no test specified\" && exit 1"
Expand Down
Loading