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

clearLayers() doesn't always work on geojson layergroups #1512

Closed
brianherbert opened this issue Mar 11, 2013 · 6 comments · Fixed by #1514
Closed

clearLayers() doesn't always work on geojson layergroups #1512

brianherbert opened this issue Mar 11, 2013 · 6 comments · Fixed by #1514
Labels

Comments

@brianherbert
Copy link
Contributor

I'm having a hard time reproducing an issue I'm having when I clearLayers() on a geojson layer group. I create a layer group like so:

geojson = {some geojson here}
var geojsonLayer = L.geoJson(geojson, {
...
}).addTo(map);

Then later, I want to refresh the layer by removing it's contents and repopulating it:

geojsonLayer.clearLayers();
geojsonLayer.addData(geojson);

However, when clearLayers executes, I'm seeing an error saying that an event doesn't exist.

I changed line 405 https://github.com/Leaflet/Leaflet/blob/master/dist/leaflet-src.js#L405 to check and make sire it exists before attempting to delete it, like so:

if (events[objKey] !== undefined && events[objKey][contextId] !== undefined) {
    delete events[objKey][contextId];
}

This fixes my issue, but I cannot figure out why it's doing this. Any pointers would be great since I don't want to mess with the core library or submit a pull request if I don't know what's wrong.

I created this simple jsfiddle, which works (much to my frustration): http://jsfiddle.net/3Sqvq/5/

@iirvine
Copy link
Contributor

iirvine commented Mar 11, 2013

Hey there - sounds like the issue I've been running into with FeatureGroup events in #1495. It's related to the way Leaflet.Events is indexing listeners in the current master - it's not breaking in your jsfiddle because you're pointed at the last release, 0.5.1, not at master.

I've not really dug into the problem too deep yet - though it seems like ensuring your group has a _leaflet_id by calling L.stamp() on it will at least keep events from throwing that error.

describe("FeatureGroup Events Bug", function() {
    it("When a layer is removed from a FeatureGroup, Leaflet.Events throws error", function() {
        var container = document.createElement('div'),
            map = new L.Map(container).setView([0, 0], 0);

        var group = new L.FeatureGroup();
        var marker = L.marker([0,0]);

        group.addLayer(marker);
        map.addLayer(group);

        expect(function(){
            group.clearLayers();
        }).not.toThrow();
    });
    it("Doesn't throw when a group has been stamped", function() {
        var container = document.createElement('div'),
            map = new L.Map(container).setView([0,0], 0);

        var group = new L.FeatureGroup();
        var marker = L.marker([0,0]);

        L.stamp(group)

        group.addLayer(marker);
        map.addLayer(group);

        expect(function(){group.clearLayers();}).not.toThrow();
    });
});

@danzel
Copy link
Member

danzel commented Mar 12, 2013

Thanks guys, will take a look at this and #1495 soon.

@danzel
Copy link
Member

danzel commented Mar 12, 2013

@brianherbert Check out #1514 for a fix.

@danzel
Copy link
Member

danzel commented Mar 28, 2013

I think this should be closed, fixed AFAIK.

@danzel danzel closed this as completed Mar 28, 2013
@MacWeber
Copy link

MacWeber commented Sep 9, 2013

It is still not working for me.
I'm coming from this issue #1416

@MacWeber
Copy link

MacWeber commented Sep 9, 2013

I realize I was not on HEAD. layer.clearLayers(); is working fine.

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

Successfully merging a pull request may close this issue.

4 participants