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

vjs.one() event listening cannot be turned off with vjs.off() #658

Closed
delner opened this issue Jul 26, 2013 · 2 comments
Closed

vjs.one() event listening cannot be turned off with vjs.off() #658

delner opened this issue Jul 26, 2013 · 2 comments

Comments

@delner
Copy link

delner commented Jul 26, 2013

Binding an event handler using one() cannot be removed using off().

Scenario:

var callback = new function() { alert('Do not trigger me!') };
var component = new Component(); // Any CoreObject has the one/off functions
component.one('myEvent', callback);
component.off('myEvent', callback);

component.trigger('myEvent'); // Should not raise alert, but will anyways.

Cause

The vjs.one() function wraps the function passed with an anonymous function:

vjs.one = function(elem, type, fn) {
  vjs.on(elem, type, function(){
    vjs.off(elem, type, arguments.callee);
    fn.apply(this, arguments);
  });
};

When vjs.on() is called, it adds a guid to the function, so it can identify and remove it later. When vjs.off() is called, it seeks the function with that guid and removes it. Because callback in my example gets wrapped with an anonymous function, it does not assign the guid to callback, rather to the anonymous function. When vjs.off(callback) is called, it doesn't find a matching guid and thus fails to unbind the event handler.

Proposed Solution

"Sync" the guid on the anonymous function and fn, so they both have the same guid. This will allow event handlers set with vjs.one() to be 'cancelled.'

Sample code:

vjs.one = function(elem, type, fn) {
  var func = function(){
    vjs.off(elem, type, arguments.callee);
    fn.apply(this, arguments);
  };

  // If on() is called before one() is
  // Inherit the ID of fn, so that one() may be cancelled.
  if(fn.guid) { func.guid = fn.guid; }

  vjs.on(elem, type, func); // This function will set func.guid if it wasn't previously set.

  // If one() is called before on()
  // Set the ID of fn, so that on() may be cancelled.
  if(!fn.guid) { fn.guid = func.guid; }
};

This code works on my fork.

@heff
Copy link
Member

heff commented Jul 29, 2013

Thanks for the find + fix. Could probably be simplified like so.

func.guid = fn.guid = fn.guid || vjs.guid++;

The bind() inside on() will keep an existing guid.

Want to do a pull request and get credit?

@delner
Copy link
Author

delner commented Jul 29, 2013

Sure thing, I'll set it up.

@heff heff closed this as completed in 334ff59 Jul 29, 2013
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants