At least for Linux/*BSD/freedesktop.org we need a Geolocation implementation. The candidates include gpsd, gypsy and GeoClue.
Created attachment 24824 [details] Add configure.ac support for geo location
Created attachment 24825 [details] Add stubs for GeolocationServiceGtk Stubs and makefile changes
Created attachment 24826 [details] Implementation using the gypsy library An attempt at using the gypsy library. This mostly compiles and links but is completely untested, it is uploaded for reference. The error handling and mixing of querying the information and listening to signals probably needs debugging.
Created attachment 24833 [details] configure.ac changes for Geolocation support Add a new configure option for geolocation
Created attachment 24834 [details] Add stubs and GNUmakefile.am changes Add stubs for a GeolocationServiceGtk, make the idl generation use ENABLE_GEOLOCATION=1, compile with ENABLE_GEOLOCATION=1.
Created attachment 24835 [details] GeolocationServiceGtk implementation using GeoClue Implement GeolocationServiceGtk with GeoClue. This is mostly untested, the velocity, suspend, resume handling is missing (not implementable ATM), the accuracy information might not be accurate and there is no API for browser developers yet.
Created attachment 24843 [details] GelocationServiceGtk implementation using GeoClue Fixed a couple of crashes in the startUpdating method. PositionOptions* can be null, it can fail to create a GeoClueMasterClient...
Comment on attachment 24833 [details] configure.ac changes for Geolocation support Looks sane. r=me Please add a ChangeLog entry before committing.
Comment on attachment 24834 [details] Add stubs and GNUmakefile.am changes Looks good. r=me Please add a ChangeLog entry before committing.
Comment on attachment 24833 [details] configure.ac changes for Geolocation support Clearing the review flag. The patch was landed.
Comment on attachment 24834 [details] Add stubs and GNUmakefile.am changes Clearing the review flag. The patch was landed.
Comment on attachment 24843 [details] GelocationServiceGtk implementation using GeoClue Looks sane. r=me.
Hi Holger, Just out of curiosity, should we be implementing GeolocationServiceClient instead of having our own GeolocationService impl?
Created attachment 25980 [details] tested patch with geoclue here is a new version of the patch with some minor bugs fixed (also tested with geoclue)
Where is the difference to the not landed patch? Could you generate a patch just showing the additional changes?
One thing that held me back to land the patch is privacy. There is currently no API that can allow/disable access to Geo data...
Created attachment 25984 [details] added only delta between not landed patch and the one tested the reason I had to use GEOCLUE_ACCURACY_LEVEL_DETAILED as default was because the options object being passed was null even when I created one
Comment on attachment 25984 [details] added only delta between not landed patch and the one tested > diff -Naur a/WebCore/page/PositionError.h b/WebCore/page/PositionError.h > - TIMEOUT = 3 > + TIMEOUT = 3, > + LOCATION_PROVIDER_ERROR = 4, > + POSITION_NOT_FOUND_ERROR = 5 > + > }; This is plain wrong. The enum is matching the PositionError of the w3c geolocation API. You may not just add values to it... > > static PassRefPtr<PositionError> create(ErrorCode code, const String& message) { return adoptRef(new PositionError(code, message)); } > diff -Naur a/WebCore/page/PositionError.idl b/WebCore/page/PositionError.idl > --- a/WebCore/page/PositionError.idl 2008-11-24 08:36:41.000000000 -0500 > +++ b/WebCore/page/PositionError.idl 2008-12-12 15:26:40.000000000 -0500 > @@ -35,6 +35,9 @@ > const unsigned short PERMISSION_DENIED = 1; > const unsigned short POSITION_UNAVAILABLE = 2; > const unsigned short TIMEOUT = 3; > + const unsigned short LOCATION_PROVIDER_ERROR = 4; > + const unsigned short POSITION_NOT_FOUND_ERROR = 5; > + > }; wrong, for the same reason. If you think this should be there then please participate in the W3C geolocation discussion?! > } > diff -Naur a/WebCore/platform/gtk/GeolocationServiceGtk.cpp b/WebCore/platform/gtk/GeolocationServiceGtk.cpp > --- a/WebCore/platform/gtk/GeolocationServiceGtk.cpp 2008-12-12 15:24:56.000000000 -0500 > +++ b/WebCore/platform/gtk/GeolocationServiceGtk.cpp 2008-12-12 15:26:40.000000000 -0500 > @@ -88,7 +88,7 @@ > return false; > } > > - GeoclueAccuracyLevel accuracyLevel = GEOCLUE_ACCURACY_LEVEL_LOCALITY; > + GeoclueAccuracyLevel accuracyLevel = GEOCLUE_ACCURACY_LEVEL_DETAILED; hmm, could you explain that part? It feels odd to treat a non existing option like a request for a high accuracy... > > void GeolocationServiceGtk::stopUpdating() > @@ -168,16 +168,16 @@ > if (error) { > setError(PositionError::POSITION_NOT_FOUND_ERROR, error->message); > return; > - } else if (fields & GEOCLUE_POSITION_FIELDS_LATITUDE && fields & GEOCLUE_POSITION_FIELDS_LONGITUDE) { > + } else if (!(fields & GEOCLUE_POSITION_FIELDS_LATITUDE && fields & GEOCLUE_POSITION_FIELDS_LONGITUDE)) { oops. > > - > + // don't update now, wait for Geoclue to trigger > // The m_altitudeAccuracy accuracy is likely to be wrong > - GeoclueAccuracyLevel level; > - geoclue_accuracy_get_details(accuracy.get(), &level, &m_accuracy, &m_altitudeAccuracy); > - updatePosition(); > + //GeoclueAccuracyLevel level; > + //geoclue_accuracy_get_details(accuracy.get(), &level, &m_accuracy, &m_altitudeAccuracy); > + //updatePosition(); oh well, could you point me to the gurantee that we will get a signal? I could not find anything in the GeoClue API doing that. And in general we do not comment out code...we remove it. > + if (!(fields & GEOCLUE_POSITION_FIELDS_LATITUDE && fields & GEOCLUE_POSITION_FIELDS_LONGITUDE)) { yes.
(In reply to comment #18) > (From update of attachment 25984 [details] [review]) > > diff -Naur a/WebCore/page/PositionError.h b/WebCore/page/PositionError.h > > > - TIMEOUT = 3 > > + TIMEOUT = 3, > > + LOCATION_PROVIDER_ERROR = 4, > > + POSITION_NOT_FOUND_ERROR = 5 > > + > > }; > > This is plain wrong. The enum is matching the PositionError of the w3c > geolocation API. You may not just add values to it... Correct. These are up to the current spec, available at: http://dev.w3.org/geo/api/spec-source.html Please do not add these values to the enum.
Created attachment 26052 [details] updated patch I’m sending this new patch which hopefully addresses Holger’s and Greg comments. >This is plain wrong. The enum is matching the PositionError of the w3c >geolocation API. You may not just add values to it... agreed >wrong, for the same reason. If you think this should be there then please >participate in the W3C geolocation discussion?! agreed. Is there any reason to have PositionError::LOCATION_PROVIDER_ERROR and PositionError::POSITION_NOT_FOUND_ERROR in the first place? >hmm, could you explain that part? It feels odd to treat a non existing option >like a request for a high accuracy... agreed. I was trying to get around bug 22847 >oh well, could you point me to the gurantee that we will get a signal? I could >not find anything in the GeoClue API doing that. And in general we do not >comment out code...we remove it in geoclue/example/master-pos-example.c position_changed_cb is triggered periodically
(In reply to comment #20) > >wrong, for the same reason. If you think this should be there then please > >participate in the W3C geolocation discussion?! > > agreed. Is there any reason to have PositionError::LOCATION_PROVIDER_ERROR and > PositionError::POSITION_NOT_FOUND_ERROR in the first place? Mostly because the JS client will not know what to do with them. You can find the discussion on the Geolocation mailing list of how these names and values came to be. The Thread Subject is "PositionError Requests".
I did land the original patch and the delta in r40441 and 40442.