Opened 9 years ago

Closed 9 years ago

#5181 closed Task (fixed)

Use callFunction when loading the wysiwyg data

Reported by: Frederico Caldeira Knabben Owned by: Garry Yao
Priority: Normal Milestone: CKEditor 3.3
Component: General Version: SVN (CKEditor) - OLD
Keywords: Confirmed Review+ Cc:

Description

We're currently using an ugly and unclear solution to notify the data load in the wysiwyg plugin.

While we still need the activation script, we could make it a bit clearer by using the CKEDITOR.tools.callFunction.

Attachments (2)

5181.patch (1.3 KB) - added by Frederico Caldeira Knabben 9 years ago.
5181_2.patch (1.7 KB) - added by Garry Yao 9 years ago.

Download all attachments as: .zip

Change History (7)

Changed 9 years ago by Frederico Caldeira Knabben

Attachment: 5181.patch added

comment:1 Changed 9 years ago by Frederico Caldeira Knabben

Keywords: Review? added
Owner: set to Frederico Caldeira Knabben
Status: newassigned

comment:2 Changed 9 years ago by Alfonso Martínez de Lizarrondo

Keywords: Review- added; Review? removed

Looking at the patch, I wonder if the usage pattern of tools.callFunction is correct or it can lead to memory leaks: in this patch, it adds a function with a closure to the editor object, and even if the editor is destroyed, the function remains in the tools array of the tools.js closure, so that should mean that the editor object remains forever in memory.

If this assumption is correct, then we should review all the usages of tools.addFunction to make sure that somehow those functions are removed when they aren't no longer needed (in this case the function should auto-remove itself) or when the editor is destroyed.

Maybe instead of storing everything in a global array, each editor should have its own addFunction, so that the array is cleaned up on destroy.

Changed 9 years ago by Garry Yao

Attachment: 5181_2.patch added

comment:3 Changed 9 years ago by Garry Yao

Keywords: Review? added; Review- removed
Owner: changed from Frederico Caldeira Knabben to Garry Yao
Status: assignednew

Leak-free version of attachment:5181.patch

comment:4 Changed 9 years ago by Frederico Caldeira Knabben

Keywords: Review+ added; Review? removed

comment:5 Changed 9 years ago by Garry Yao

Resolution: fixed
Status: newclosed

Fixed with [5450].

Note: See TracTickets for help on using tickets.
© 2003 – 2019 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy