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

Already on GitHub? Sign in to your account

autogrow._handleShow should only bind to browser's transition end event (memory leak) #6697

Closed
nonplus opened this Issue Nov 4, 2013 · 2 comments

Comments

Projects
None yet
3 participants

nonplus commented Nov 4, 2013

Right now, the autoshow._handleShow() function binds once to three flavors of the transition end event: one("transitionend webkitTransitionEnd oTransitionEnd"). Since a browser is only going to fire (at most) one of these events (IE will fire none of them), the other event subscriptions won't ever be released.

The following code is the culprit:

if ( event.type !== "popupbeforeposition" ) {
    this.element
        .addClass( "ui-textinput-autogrow-resize" )
        .one( "transitionend webkitTransitionEnd oTransitionEnd",
            $.proxy( function() {
                this.element.removeClass( "ui-textinput-autogrow-resize" );
            }, this ) );
}

The fix should be to bind to the specific transition end event (if the browser has one). For example, the following is an example of a fixed implementation:

var transitionEndEvent = 'WebkitTransition' in window ? 'webkitTransitionEnd'
    : 'OTransition' in window ? 'oTransitionEnd'
    : 'MozTransition' in window || 'transition' in window ? 'transitionend'
    : undefined;

if ( event.type !== "popupbeforeposition" && transitionEndEvent) {
    this.element
        .addClass( "ui-textinput-autogrow-resize" )
        .one( "transitionend webkitTransitionEnd oTransitionEnd",
            $.proxy( function() {
                this.element.removeClass( "ui-textinput-autogrow-resize" );
            }, this ) );
}

However, there is probably a better place to configure the transitionEndEvent variable so that it can be reused by other widgets (e.g. panel.js)

@ghost ghost assigned gabrielschulhof Nov 5, 2013

Contributor

gabrielschulhof commented Nov 5, 2013

$.support might be a good place for storing the value of that conditional.

nonplus commented Nov 5, 2013

I assume you mean $.**mobile.**support. I'm not sure what the correct naming conventions are and it seems that most of the support properties are Boolean rather than string values (though since an event name is truthy, that shouldn't be a problem).

In this case, would the name be $.mobile.support.transition.end?

@ghost ghost assigned arschmitz Jan 17, 2014

@arschmitz arschmitz added a commit that referenced this issue Jan 22, 2014

@arschmitz arschmitz AnimationComplete: Add transition, fallbacks and remove memory leaks
Also no its own module including support tests for animations and
transitions

Fixes gh-5156
Fixes gh-6816
Fixes gh-6697
Fixes gh-6895
Fixes gh-6148
f4fbff2

@arschmitz arschmitz added a commit that referenced this issue Feb 12, 2014

@arschmitz arschmitz AnimationComplete: Add transition, fallbacks and remove memory leaks
Also no its own module including support tests for animations and
transitions

Fixes gh-5156
Fixes gh-6816
Fixes gh-6697
Fixes gh-6895
Fixes gh-6148
Closes gh-7001
0793219

@arschmitz arschmitz closed this in 749c78e Feb 12, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment