unbindPopup appears to clear click event listener

[rickhall]

I am using Leaflet marker cluster since I'm dealing with lots of data. I do something like this to listen for clicks on markers:

this._cluster = new L.MarkerClusterGroup();
this._cluster.on(
    "click",
    bind(this._onMarkerClick, this)
);

And something like this to listen for popup close events:

this._map = new L.Map($map[0], {})
    .on(
        "popupclose",
        bind(this._onPopupClose, this)
    );

To avoid creating excessive popups, I am trying to dynamically bind/unbind popups to the markers, like this:

_onMarkerClick: function(event) {
    var $content = this._createPopupContent(event.layer._myObject);
    event.layer.bindPopup($content[0], { }).openPopup();
},
_onPopupClose: function(event) {
    if (event.popup._source) {
            event.popup._source.unbindPopup();
    }
},

This works the initial time a marker is clicked, but subsequent clicks on the marker do not trigger the event and thus the popup is not displayed. This only affects the clicked marker, but any time I click another marker and dismiss its popup, then it will no longer receive click events as well. If I do not unbind in the close event, but instead just close the popup then I continue to receive click events and my popup is displayed.

From my poking around, it appears that invoking unbindPopup() in this scenario is the equivalent to doing an "off click" for that specific marker. Perhaps I am missing something, but this doesn't really seem to be an intuitive behavior. I am simply trying to reduce memory consumption by disposing of unwanted popups.

Perhaps I am missing the more obvious way to do this, but I couldn't find anything.

[mourner]

Check out #3109 — it will probably make your performance hacks for popups unnecessary.

[rickhall]

Yes, that sounds like it would do the work for me that I'm trying to do manually. Thanks. In the meantime, is there something I am doing wrong above that can be corrected to get the behavior that I want?

[danzel]

Or instead of binding a popup on click, just create a new popup and show it.
See the second example on: http://leafletjs.com/reference.html#popup

If unbindPopup is killing the click handler though that would be a bug.

[mourner]

@rickhall could you reproduce the click handler issue in a minimal JSFiddle test case?

[rickhall]

Here is a fiddle: http://jsfiddle.net/fphonku5/1/

It is entirely possible I am doing something incorrectly, but to see the issue simply run and click on the marker to see the popup, click off of the popup to make it go away, then click on the marker again and you will see the popup doesn't appear again.

[iH8]

Your experiencing this problem because you are attaching the event to the your instance of L.MarkerClusterGroup not to your instance of L.Marker. I'm not seeing this problem when attaching it directly to the instance of L.Marker I've went ahead and made a Plunker (I'm not too big on JSfiddle) to demonstrate.

cluster.on(): http://plnkr.co/edit/WFhpjZ?p=preview
marker.on(): http://plnkr.co/edit/jNZa36?p=preview

Now from what i understand from the README.MD of markercluster the click event on the cluster returns an event with a property called layer which contains the marker instance thus it should in fact be the exact same thing. I'm kind of baffled by this but i'm guessing the problem lies with L.MarkerClusterGroup not with Leaflet core.

On a sidenote: You don't need to use bind to pass the context when attaching your eventhandlers, you can simple use the third parameter of the on method which is for context. See: http://leafletjs.com/reference.html#events-addeventlistener

[rickhall]

@iH8, thanks for the response.

I bound the event to L.MarkerClusterGroup because I may have literally thousands and thousands of markers and didn't want to attach event listeners to all of them. Perhaps that isn't the right approach, but it seems like it should be.

It could be a bug in L.MarkerClusterGroup, but from my poking around, it sort of looked like the issue was with Leaflet itself. I'm sure that someone with more Leaflet expertise than I have could figure it out in no time from my fiddle, though.

[iH8]

I completely understand your reasoning and share your opinion as to that binding to the cluster should be the way to go. I was merely pointing out that the problem doesn't arise when binding to the actual marker.

I think it wouldn't be a bad idea to raise this issue over on the tracker of the actual plugin: https://github.com/Leaflet/Leaflet.markercluster/issues Maybe they already have experience with the behaviour we're getting and are overseeing something or doing something stupid. (although i could find anything with a quick search) I'de give it a shot, you never know.

[rickhall]

Yeah, you could be right. However, did you see this part of my original message: "If I do not unbind in the close event, but instead just close the popup then I continue to receive click events and my popup is displayed." Due to that and some of my poking around, it seemed like the issue was with Leaflet.

Regardless, I'll do as you suggest and open an issue over there too. Thanks.

[mourner]

Could this problem be replicated with just a FeatureGroup rather than MarkerClusterGroup (which it inherits from)? Just to have a more minimal test case. If it can, you can close Leaflet/Leaflet.markercluster#455.

[rickhall]

I'll try to see if I can minimize it.

[rickhall]

My intuition after poking around originally was that this was a core issue, perhaps this fiddle will help narrow it down:

http://jsfiddle.net/fphonku5/6/

It looks like I see the same behavior with just a FeatureGroup, unless I am doing something wrong.

[janatjak]

IMHO, in class Events method hasEventListeners trying test "click_idx_len" in array this.events[ "_leaflet_events" ](great then zero), but this value is zero, because removeEventListener decrement it (after calling method unbindPopup).

[rickhall]

Yeah, I was seeing something like that too when I was poking around, which is why originally contended (and still believe) that the issue was in the core.

[janatjak]

If I test problem with dev-branch, it works.
janatjak@97ee683

[IvanSanchez]

@mourner Try to fix stable, or close and wait for 1.0 release?

[mourner]

Pretty minor so I think close.

[rickhall]

When might 1.0 get released or another stable? Is .8-dev reasonably stable? Is there updated documentation for it online? Thanks.