Add support for approximate geolocation - #195
Conversation
|
For readability as you iterate on this proposal it's okay for this PR to directly change the specification text however to land this change the changes need to be enclosed in the correct candidate additions/corrections/deletions syntax. |
|
@reillyeon if it's ok, let's move this to CR first (i.e., let's not waste time with the ins/dels). We are close to publishing as CR again. |
99d9b03 to
3fa8869
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for approximate location positioning to the Geolocation API specification. It introduces a privacy-preserving alternative to precise location sharing, allowing applications to request coarse-grained location data when high accuracy is not needed.
Key changes:
- Introduces
accuracyModeoption with "precise" (default) and "approximate" values - Adds new "geolocation-approximate" permission alongside existing "geolocation" permission
- Implements separate caching for precise and approximate positions
- Updates permission request flows to allow users to choose between precise and approximate location sharing
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
3fa8869 to
8658f5a
Compare
2354ca8 to
d3a4d47
Compare
antosart
left a comment
There was a problem hiding this comment.
A few comments wrt the permission handling.
d3a4d47 to
b0d0589
Compare
Clarifies that exposing the "geolocation-approximate" permission is not strictly enforced, and implementations may choose to only make "geolocation" queryable.
|
@marcoscaceres / @antosart / @tomvangoethem , |
| <a>"geolocation-approximate"</a> changes: | ||
| </p> | ||
| <ol class="algorithm"> | ||
| <li>If the [=permission state=] of <a>"geolocation"</a> is changed to |
There was a problem hiding this comment.
I think we should also add:
If the permission state of "geolocation" is granted and the permission state of "geolocation-approximate" is changed to prompt, set the permission state of "geolocation" to prompt.
If the permission state of "geolocation-approximate" is denied and the permission state of "geolocation" is changed to prompt, set the permission state of "geolocation-approximate" to prompt.
There was a problem hiding this comment.
I think current algorithm give the basic coherence between two permissions seems good to me. I am not sure whether should we add more coherence here given that it might bring more questions and different agents might want to implement this differently. Maybe let's add it later from a new issue so we can discuss further on finer permission control for this feature?
There was a problem hiding this comment.
Exactly... what @alvinjiooo said.
Maybe we can add a note or issues here saying we are still trying to figure this out. And we can link to the issue via <aside class="issue" data-number="??">
There was a problem hiding this comment.
This should be handled by the Permissions spec, as these changes are bound to the event loop.
If anything, changes state needs to "queue at task on the ??? task source to ..." so the permission state can change at the appropriate time at the turn of the event loop (so the PermissionState can do what it needs to do).
There was a problem hiding this comment.
The point is that the PR as currently written assumes that the combined permission states are in one of the 6 allowed states. The algorithms break otherwise. Given that, I do believe that we should explicitly listing the 6 allowed states (see comment above) and also write that implementations must make sure that everything stays consistent at all times. If we list things that implementors must do, I say either we list all of them or we skip all.
There was a problem hiding this comment.
I’ll have another look. If they as look up tables for state, the yes: make sense to keep them. I didn’t get that sense tho, as the permission state is maintained independently by the permission store (as permissions might change at any time for any reason… they expire, users changes them, os level changes etc. and the change might not always be observable by script).
There was a problem hiding this comment.
@marcoscaceres ,
I discussed with @antosart and decided to remove the new/old permission coherence description from this PR and added a aside tag with issue #231 to track the follow up discussion. Currently we still keep the table for the permission state combos which we believe you also think that is reasonable to have it in this PR. Please take a look at the latest version of the PR and let us know if this looks good to you.
Thanks!
This update correctly links to the Permissions API using ReSpec citing conventions and resolves a missing definition error by using the correct 'query a permission' algorithm term.
Hi @marcoscaceres , |
marcoscaceres
left a comment
There was a problem hiding this comment.
Mostly just nits now... but question around the permissions table.
Otherwise looking pretty good.
- Use <dl>, <dt>, and <dd> for precise and approximate position definitions. - Convert informative note on permission masking to a normative statement. - Tidy index.html.
There was a problem hiding this comment.
I updated for most comments but leave the permission states combination table since @antosart and @marcoscaceres still having discussion. For other comments I have them updated and resolved.
|
Is there any reason why there should be a separate geolocation-approximate permission instead of doing |
- Transition from dual permission model to single 'geolocation' permission. - Introduce GeolocationPermissionDescriptor with accuracyMode member. - Simplify getCurrentPosition and watchPosition algorithms to determine effectiveAccuracy before acquisition. - Remove redundant Permission coherence section. - Tidy index.html.
@saschanaz, Thanks for great suggestion, through email discussion Hi @marcoscaceres, |
|
Note that a custom Please also see the Permissions issue w3c/permissions#459 I opened. People who chimed in so far seem to agree that permission aspects are probably a bad choice for booleans, and I personally tend to agree. |
|
@alvinjiooo I think the main problem with the current version of the PR is that this is not backwards compatible. When requesting a position, you fall back to checking the approximate permission if the precise permission is not granted However, you got rid of the overridden permission query algorithm. This means that you would end up with |
I wonder when you mention |
Hi @marcoscaceres , |
|
@marcoscaceres I went ahead and drafted w3c/permissions#477. Would something like that be helpful to address your concerns wrt exposing the new permission? |
Yes, in your current version, the spec mandates for Regardless, I would suggest to go back to a version of this PR that reflects the explainer, since it's otherwise difficult to understand what we are trying to spec. |
Actually currently version of PR didn't define how permissions.query should behave wrt to GeolocationPermissionDescriptor, that is actually up to user agent to decide what to return when The algorithm at Request a position first check permission with Let me know if you still feel the previous version of PR ("geolocation" + "geolocation-approximate" permission name) should be the way to moving forward. Thanks. |
The Permissions spec does mandate the return value of |
…tor" This reverts commit 1f5e304.
Thanks for pointing this out. Since previous PR also need custom permission query and custom algorithm because the new permission "geolocation-approximate". I would imagine I will first revert the PR back to "geolocation-approximate" for now to where we start to invite Mozilla to review, but I am not sure if it is meaningful to continue to update it before we get clear idea what is acceptable model to everyone. |
Hey @marcoscaceres. I was wondering if you had time to consider the proposal of having this PR together with something like w3c/permissions#477 (which we could iterate upon and improve) to clarify that browsers can mask the queried permission state in some cases (as WebKit already does for |
|
I tried to write a version of this PR which does not define a different permission name or permission model and instead follows the proposed WebKit model. Since the discussion on this PR has been already quite long, I created a separate PR #233. |
Closes #182
This commit introduces the ability for users and developers to request a less precise, privacy-preserving "approximate" location.
Key changes include:
The following tasks have been completed:
Implementation commitment (and no objections):
Documentation (new feature):
For documentation, either create an issue or pull request in MDN's Content repo - providing as much information as you can. PR is prefered.
Approximate Geolocation explainer
Preview | Diff