Skip to content
This repository has been archived by the owner on Feb 29, 2020. It is now read-only.

Remove listeners will not actually remove listeners #55

Closed
k88hudson opened this issue Feb 5, 2016 · 2 comments
Closed

Remove listeners will not actually remove listeners #55

k88hudson opened this issue Feb 5, 2016 · 2 comments
Assignees
Labels
Milestone

Comments

@k88hudson
Copy link
Contributor

I just noticed right now in ActivityStreams we have:

  _setupListeners() {
    PlacesProvider.links.on("deleteURI", this._handlePlacesChanges.bind(this));
    ...
  _removeListeners() {
    PlacesProvider.links.on("deleteURI", this._handlePlacesChanges);
    ...

I think this.fn !== this.fn.bind(this), so we'll need to use a different strategy for this

@oyiptong
Copy link
Contributor

oyiptong commented Feb 8, 2016

You are correct.

event-emitter.js uses a Map to hold the listeners: https://dxr.mozilla.org/mozilla-central/source/devtools/shared/event-emitter.js#65

Also, this.fn.bind(this) !=== this.fn.bind(this)

@k88hudson k88hudson modified the milestone: 3. Beta Feb 22, 2016
@tspurway tspurway assigned oyiptong and rlr and unassigned oyiptong Feb 29, 2016
rlr added a commit that referenced this issue Mar 1, 2016
rlr added a commit that referenced this issue Mar 1, 2016
@rlr
Copy link
Contributor

rlr commented Mar 1, 2016

landed

@rlr rlr closed this as completed Mar 1, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants