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

Fix(Map): call stopLocate on map.remove #5893

Merged
merged 1 commit into from Oct 31, 2017

Conversation

ghybs
Copy link
Collaborator

@ghybs ghybs commented Oct 31, 2017

Hi,

Following SO post Leaflet .locate watch option breaks .locate after changing tab, this PR proposes to automatically call map.stopLocate() as part of map.remove() process, so that app developer does not need to remember about having used the Geolocation with watch option or not.

Otherwise, Geolocation handlers (namely _handleGeolocationResponse and _handleGeolocationError) are still called regulary and try to access DOM elements which have been previously cleared by map.remove().

Check for _locationWatchId is not perfect (watch may have been already stopped), but calling stopLocate again does not harm. It prevents trying to access the Geolocation API if locate had never been called, even though doing so should not harm either.

so that Geolocation handlers (namely _handleGeolocationResponse and _handleGeolocationError) do not try to access DOM elements which have been cleared by map.remove().
Of course developer should currently call map.stopLocate() manually before map.remove(), but it looks like he/she may forget it.
Check for _locationWatchId is not perfect (watch may have been already stopped), but calling stopLocate again does not harm. It prevents trying to access the Geolocation API if locate had never been called, even though doing so should not harm either.
@mourner mourner merged commit cf518ff into Leaflet:master Oct 31, 2017
@ghybs ghybs deleted the mapStopLocateOnRemove branch October 31, 2017 17:14
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

Successfully merging this pull request may close these issues.

None yet

2 participants