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

Fix some memory leaks in 3.1.0 admin #2340

Merged
merged 4 commits into from
Aug 20, 2013

Conversation

hafriedlander
Copy link
Contributor

No description provided.

Hamish Friedlander added 4 commits August 20, 2013 15:17
We werent calling tinyMCE.Editor.destroy, which is needed to
clean up event bindings. The advanced theme also wasnt cleaning
up after itself on destroy properly
@@ -753,8 +753,11 @@ Sizzle is good for finding elements for a selector, but not so good for telling
* Add focusin and focusout support to bind and live for browers other than IE. Designed to be usable in a delegated fashion (like $.live)
* Copyright (c) 2007 Jörn Zaefferer
*/
$.support.focusInOut = !!($.browser.msie);
if (!$.support.focusInOut) {
if ($.support.focusinBubbles === undefined) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't typeof $.support.focusinBubbles === 'undefined' safer?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I use undefined by itself a lot in Entwine. Strictly typeof X === 'undefined' is safer, but lots of things are going to break if you define undefined, not just this line :P

@chillu
Copy link
Member

chillu commented Aug 20, 2013

It still leaks 1MB in Chrome and 5MB in IE8 for me when simply switching back and forth between two page edit views in the standard install. But then again, a single reference is enough to cause this behaviour, and Hamish's work clearly fixes some leaks which should make further debugging work easier. Behat tests pass against this branch (no JS errors, on Firefox), I've done a bit of manual testing in IE8, happy to merge.

chillu added a commit that referenced this pull request Aug 20, 2013
Fix some memory leaks in 3.1.0 admin
@chillu chillu merged commit a2f9af5 into silverstripe:3.1.0 Aug 20, 2013
@mateusz
Copy link
Contributor

mateusz commented Aug 20, 2013

Nice, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants