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

Include accuracy property in 'locationfound' events. #1889

Closed

Conversation

k-anon
Copy link

@k-anon k-anon commented Jul 19, 2013

I am submitting a patch adding back the accuracy property to locationfound events originating from Map.Geolocation. I believe it is desirable that a plugin bolstering Leaflet's geolocation wrapper, such as the otherwise currently broken domoritz/leaflet-locatecontrol, would have access to this property in some way.

@@ -63,11 +63,12 @@ L.Map.include({
},

_handleGeolocationResponse: function (pos) {
var lat = pos.coords.latitude,
var accuracy = parseFloat(pos.coords.accuracy),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the parseFloat here? In the spec this should be a numeric just like the rest. Is there some implementation that diverges?

@mourner
Copy link
Member

mourner commented Sep 17, 2013

Yeah, if it's really a number, it should have been fixed here #1755

@mourner mourner closed this Nov 4, 2013
@radicalbiscuit
Copy link
Contributor

In Chrome 31.0.1650.57 on Windows 7, I'm encountering a pos.coords.accuracy whose type is string.

This might be due to the Chrome extension Manual Geolocation, which I'm using to test. It should also be noted that I'm using this with leaflet-locatecontrol.

At any rate, I place a breakpoint in _handleGeolocationResponse so I can inspect pos on each handling of geolocation. They're all numeric until I use Manual Geolocation to change the coordinates. All subsequent events then use a string pos.coords.accuracy. What's more, the latitude and longitude are also strings.

So, once again, I believe this to be related to Manual Geolocation, but it's worth noting that there is at least one implementation that allows non-numeric geolocation properties, and if that's the case, perhaps it might be warranted to explicitly include accuracy parsed or otherwise (as in #584), or do a wholesale inclusion of the geolocation properties.

@danzel
Copy link
Member

danzel commented Nov 20, 2013

Sounds more like a bug in manual geolocation.

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

5 participants