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

Marker's icon is not always present #172

Closed
tmcw opened this issue Aug 7, 2013 · 4 comments
Closed

Marker's icon is not always present #172

tmcw opened this issue Aug 7, 2013 · 4 comments

Comments

@tmcw
Copy link
Contributor

tmcw commented Aug 7, 2013

In this bit of code: https://github.com/Leaflet/Leaflet.draw/blob/master/src/edit/handler/EditToolbar.Edit.js#L135-L153 Leaflet.draw makes the assumption that the L.Marker layers in the map will always have a _icon private member. This assumption is broken in some cases, including (I think) when markers are offscreen or added to the map without an icon specified. In this case, a TypeError is thrown and not caught.

@jacobtoye
Copy link
Member

I can't think of how this could happen? The icon should be initialized when it is added to the map and (besides CircleMarker's) as far as I know markers are not removed from the map when offscreen.

Do you have a case where you are hitting the error? If you could run up a jsfiddle that would be great!

@tmcw
Copy link
Contributor Author

tmcw commented Aug 8, 2013

Will try to get a more precise test case today! This is coming in through exceptions caught on geojson.io so I didn't experience it directly, just saw the code path.

@jacobtoye
Copy link
Member

@tmcw could you please check to see if you are still seeing the error with the latest build. I've added a small check to see if _icon exists. This could likely cause other issue if the marker is added to the map at a later point without firing the layeradd event.

@tmcw
Copy link
Contributor Author

tmcw commented Sep 3, 2013

Testcase: this happens when your draw:edited event handler clears the items in the layer you're drawing into: https://dl.dropboxusercontent.com/u/68059/tmp/Leaflet.draw/examples/basic.html - this is what www.geojson.io does until Leaflet supports toGeoJSON uniformly. - to replicate (with your console open) add a marker, click edit, move the marker, save. you'll see the 'icon is undefined' issue.

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

No branches or pull requests

2 participants