GNOME Bugzilla – Bug 780252
GNOME Shell 3.24 shows NY Weather if location privacy is enabled
Last modified: 2017-03-23 23:58:26 UTC
Created attachment 348239 [details] screenshot of location privacy settings Ubuntu GNOME 17.04 Beta GNOME 3.23.92 In Settings>Privacy, I have Location turned On but I turned it Off for gnome-shell. GNOME Shell's calendar dropdown menu continued to show my weather from the last location I had shared with it. After a few days, I looked again and now it's giving me (apparently) New York City weather. I'm in Florida now and it's definitely not 25° F now like GNOME Shell is indicating. What I expect to happen is for GNOME Shell to immediately stop showing me Weather when I've turned Location off for GNOME Shell. I'm attaching a screenshot of my location privacy settings.
(In reply to Jeremy Bicha from comment #0) > What I expect to happen is for GNOME Shell to immediately stop showing me > Weather when I've turned Location off for GNOME Shell. With https://cgit.freedesktop.org/geoclue/commit/?id=a4cef6c0ad08, GNOME Shell won't appear at all in the list. What we should do instead is following the configuration for GNOME Weather, just as we do for other settings.
Created attachment 348277 [details] [review] weather: Track whether a location was set Setting GWeatherInfo:location to null helpfully doesn't mean "no location", but "NYC". This obviously isn't what we want to show users, so track the location validity separately and consider it when updating the label shown to users.
Created attachment 348278 [details] [review] weather: Disentangle _useAutoLocation from Weather setting We currently use automatic location for weather forecasts if the corresponding Weather setting is set, however we should take other factors into account as well: - whether location services are enabled at all - whether Weather has been authorized to use them In preparation of these changes, track the setting's value in a separate property and make _useAutoLocation a getter, so we can extend it with additional conditions easily.
Created attachment 348279 [details] [review] weather: Take global location switch into account The setting to globally disable location settings altogether isn't handled by the geoclue service itself, but by the authorization agent. This means that: - it doesn't apply to system components (which gnome-shell is now considered[0]) - it doesn't apply once the geoclue connection has been authorized However users can reasonably expect that we won't use location services after they disabled them, so handle the setting explicitly. [0] https://cgit.freedesktop.org/geoclue/commit/?id=a4cef6c0ad08
Created attachment 348280 [details] [review] location: Split out PermissionStore It doesn't make sense to tie the proxy code for flatpak's permission store to the location indicator, just because that was the first component to use it, so split it into a separate module.
Created attachment 348281 [details] [review] weather: Follow GNOME Weather's location permissions Our weather integration should follow GNOME Weather as closely as possible, which means that we should respect its location permission rather than using our own or none at all (which we can as a "system" component and as geoclue's authorization agent).
Review of attachment 348277 [details] [review]: right
Review of attachment 348278 [details] [review]: ++
Review of attachment 348279 [details] [review]: makes sense
Review of attachment 348280 [details] [review]: ok
Review of attachment 348281 [details] [review]: didn't double check the DBus API, but the logic seems sound ::: js/misc/weather.js @@ +38,3 @@ + + this._permStore.LookupRemote('gnome', 'geolocation', (res, error) => { + let [perms, data] = error ? [{}, null] : res; maybe log this error too?
Created attachment 348331 [details] [review] weather: Track whether a location was set Changed string to show when we don't have a valid location to "Select location..." after feedback on #gnome-design.
Attachment 348278 [details] pushed as 7c9f769 - weather: Disentangle _useAutoLocation from Weather setting Attachment 348279 [details] pushed as 74e1058 - weather: Take global location switch into account Attachment 348280 [details] pushed as 9cc1e6b - location: Split out PermissionStore Attachment 348281 [details] pushed as c3428f1 - weather: Follow GNOME Weather's location permissions Attachment 348331 [details] pushed as d393ca4 - weather: Track whether a location was set
Let me know if I should open a different bug instead. I built gnome-shell 3.24 with these 5 patches (as applied to git master) cherry-picked. Now gnome-shell crashes when I either enable or disable sharing my Location (using either the GNOME Shell status menu switch or in Settings>Privacy).
Bug 780278?
(In reply to Florian Müllner from comment #15) > Bug 780278? Thanks. That patch helps a lot. (It still managed to crash the first time I enabled Location but it hasn't crashed since.) Anyway, I'll close this bug again.