GNOME Bugzilla – Bug 736479
location: Translate accuracy level for geoclue
Last modified: 2014-09-11 16:49:36 UTC
See patch.
Created attachment 285911 [details] [review] location: Translate accuracy level for geoclue The enums values of geoclue have gaps in them (so more levels could be added in future) but enum values of settings don't have such gaps so we gotta translate between them. Since desrt says that enums as integers in gsettings are bad, we now treat accuracy level settings as string. This fixes the recent regression of geoclue only allowing geiop level accuracy to apps.
See bug#736272 for why we are not simply assigning values to enums in the settings.
Review of attachment 285911 [details] [review]: ::: js/ui/status/location.js @@ +206,1 @@ return 0; What does 0 mean in the context of Geoclue? If it means disabled, I'd add that to the declaration of GeoclueAccuracyLevel above.
Created attachment 285920 [details] [review] location: Translate accuracy level for geoclue v2: NONE=0 enum added to declaration.
Review of attachment 285920 [details] [review]: "Since desrt says that enums as integers in gsettings are bad, we now treat accuracy level settings as string." enums in settings aren't really the issue here, mapping between integer (G_DESKTOP_LOCATION_ACCURACY_LEVEL_EXACT) and string ("exact") is built-in, so you can use either option just fine. The problem is Geoclue's DBus API using magic numbers instead of descriptive string values, but that's nothing we can change now ... ::: js/ui/status/location.js @@ +203,3 @@ + return GeoclueAccuracyLevel.CITY; + else if (level == 'country') + return GeoclueAccuracyLevel.COUNTRY; You could do something fancy like: let level = this._settings.get_string(MAX_ACCURACY_LEVEL); return GeoclueAccuracyLevel[level.toUpperCase()] || GeoclueAccuracyLevel.NONE; here. If you do prefer a big if-else block, please write it as: let level = this._settings.get_enum(MAX_ACCURACY_LEVEL); if (level == GDesktopEnums.LocationAccuracyLevel.EXACT) return GeoclueAccuracyLevel.EXACT; else if (...) @@ +207,1 @@ return 0; GeoclueAccuracyLevel.NONE
Review of attachment 285920 [details] [review]: > enums in settings aren't really the issue here, mapping between integer (G_DESKTOP_LOCATION_ACCURACY_LEVEL_EXACT) and string ("exact") is built-in, so you can use either option just fine. Not according to desrt, unless I misunderstood him.
Created attachment 285932 [details] [review] location: Translate accuracy level for geoclue v3: Less code. I didn't get to test this cause all of a sudden I'm getting this error from shell: (gnome-shell:27339): Gjs-CRITICAL **: JS ERROR: SyntaxError: missing : after property id @ resource:///org/gnome/shell/ui/endSessionDialog.js:143 ** Message: Execution of main.js threw exception: JS_EvaluateScript() failed
(In reply to comment #7) > I didn't get to test this cause all of a sudden I'm getting this error from > shell: My fault, fixed.
Review of attachment 285932 [details] [review]: OK
Attachment 285932 [details] pushed as 7653175 - location: Translate accuracy level for geoclue I tested it a but before pushing.