After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 722623 - Set 'DesktopId' and 'RequestedAccuracyLevel' on Geoclue client proxy
Set 'DesktopId' and 'RequestedAccuracyLevel' on Geoclue client proxy
Status: RESOLVED FIXED
Product: gnome-settings-daemon
Classification: Core
Component: datetime
3.11.x
Other Linux
: Normal normal
: ---
Assigned To: Kalev Lember
gnome-settings-daemon-maint
Depends on:
Blocks:
 
 
Reported: 2014-01-20 17:34 UTC by Zeeshan Ali
Modified: 2014-02-03 18:37 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
datetime: Provide desktop id to geoclue (1.10 KB, patch)
2014-02-03 13:42 UTC, Kalev Lember
committed Details | Review
datetime: Specify the accuracy level for geoclue (1.66 KB, patch)
2014-02-03 18:13 UTC, Kalev Lember
committed Details | Review

Description Zeeshan Ali 2014-01-20 17:34:17 UTC
For security reasons, Apps must provide id of their desktop id (name of desktop file minus '.desktop' extension). Also, apps should now specify the level of geolocation accuracy they'd prefer.

Mandatory docs link:

http://www.freedesktop.org/software/geoclue/docs/gdbus-org.freedesktop.GeoClue2.Client.html#gdbus-property-org-freedesktop-GeoClue2-Client.DesktopId

No link for AccuracyLevel yet (as its not in a release) but here is the relavent commits/code:

http://cgit.freedesktop.org/geoclue/commit/?id=bcd2a531ec0af4b3ae746400142e380b7ac81603
http://cgit.freedesktop.org/geoclue/commit/?id=f0509858c101e2860bc1125cb76797ee19d03419
Comment 1 Bastien Nocera 2014-01-21 06:55:06 UTC
Note that the desktop ID should be gnome-datetime-panel.desktop.

Can you attach a patch that you've tested?
Comment 2 Zeeshan Ali 2014-01-21 12:35:36 UTC
(In reply to comment #1)
> Note that the desktop ID should be gnome-datetime-panel.desktop.
> 
> Can you attach a patch that you've tested?

Kalev promised to take care of this. If he doesn't, I'll provide a patch.
Comment 3 Kalev Lember 2014-02-03 13:42:54 UTC
Created attachment 267946 [details] [review]
datetime: Provide desktop id to geoclue
Comment 4 Kalev Lember 2014-02-03 13:48:55 UTC
The patch above sets the desktop ID, but I am not so sure if we need to set the RequestedAccuracyLevel. It seems that the geoclue default matches with what we need here -- the city level. Should we still explicitly set it?

Geoclue doesn't install the headers where the accuracy level enums are defined, and we'd need to copy the enum definitions. I'd rather avoid doing that if the defaults work fine.
Comment 5 Bastien Nocera 2014-02-03 13:57:48 UTC
Review of attachment 267946 [details] [review]:

A little explanation as to why it's needed would be appreciated in the commit message.
Looks fine otherwise.
Comment 6 Kalev Lember 2014-02-03 15:01:54 UTC
Comment on attachment 267946 [details] [review]
datetime: Provide desktop id to geoclue

Attachment 267946 [details] pushed as 501bad0 - datetime: Provide desktop id to geoclue
Comment 7 Zeeshan Ali 2014-02-03 16:57:08 UTC
(In reply to comment #4)
> The patch above sets the desktop ID, but I am not so sure if we need to set the
> RequestedAccuracyLevel. It seems that the geoclue default matches with what we
> need here -- the city level. Should we still explicitly set it?

Yes, the user of geoclue shouldn't assume what default level geoclue will use.

> Geoclue doesn't install the headers where the accuracy level enums are defined,
> and we'd need to copy the enum definitions. I'd rather avoid doing that if the
> defaults work fine.

Its a small enum and you can only define the value you are setting to make it even smaller.

Ideally geoclue should provide a client library but I just never got around to doing that and nobody provided patches: https://bugs.freedesktop.org/show_bug.cgi?id=68658
Comment 8 Kalev Lember 2014-02-03 18:13:56 UTC
Created attachment 267981 [details] [review]
datetime: Specify the accuracy level for geoclue

This sets the accuracy level to the same value as the current geoclue
default. As per the discussion in the ticket, we shouldn't rely on the
default staying the same in the future and have to explicitly set it.
Comment 9 Zeeshan Ali 2014-02-03 18:16:09 UTC
Review of attachment 267981 [details] [review]:

ack
Comment 10 Kalev Lember 2014-02-03 18:37:56 UTC
Attachment 267981 [details] pushed as 41d81bf - datetime: Specify the accuracy level for geoclue