Looks good Fil - good to get the iOS only cruft out of there. Minor point but there's a way to hide the private vars in the implementation file since users won't need to really see them in the public headers, through class extensions: http://stackoverflow.com/questions/5826345/how-to-declare-instance-variables-and-methods-not-visible-or-usable-outside-of-t Not that prevents access, just visibility, and we are open source after all...
2012/3/28 Filip Maj <[email protected]>: > I've sent a pull request to both apache/cordova-js and apache/cordova-ios > for the geo stuff (side note: github pull requests might be broken again > :/) > > https://github.com/apache/incubator-cordova-ios/pull/8 > > > The above gets the native iOS geolocation plugin lined up to the new > interface imposed by cordova-js. > > The JS file is bundled in the branch so you don't need to rebuild > cordova-js to see these improvements. > > Shaz and anyone else keen on the iOS platform: can you guys take a look? > My tests on my end look good but would love another set of eyes. > > On 3/27/12 5:00 PM, "Filip Maj" <[email protected]> wrote: > >>So we'll keep it around (even though we won't override the implementation >>that we get for free in the WebView), and thus we'll need to update the >>native implementation. Gotcha. I will update the JIRA issue as such then. >> >>On 3/27/12 4:57 PM, "Joe Bowser" <[email protected]> wrote: >> >>>We should axe it/move it to its own plugins repo. Someone may want it, >>>but >>>I don't want to make promises for it. >>> >>>On Tue, Mar 27, 2012 at 4:55 PM, Filip Maj <[email protected]> wrote: >>> >>>> I am going to assume then that will be merged in and we'll be making >>>>the >>>> necessary native tweaks across the platforms we want to support. >>>> >>>> ANDROID peeps: should we axe native geo code, or should we keep it >>>>around >>>> and thus update the implementation to follow this new approach? >>>> >>>> On 3/27/12 3:33 PM, "Shazron" <[email protected]> wrote: >>>> >>>> >Apple should be following W3C, although I don't know if they fixed >>>> >this bug yet, it's still unresolved for me in Radar: >>>> >http://openradar.appspot.com/radar?id=1160403 but based on what my >>>> >test on 5.1, they've fixed it. >>>> > >>>> >Sure ping me on email/jabber let's set up a time. >>>> > >>>> >On Tue, Mar 27, 2012 at 3:07 PM, Filip Maj <[email protected]> wrote: >>>> >> Assuming that the native WebView implementations across whatever >>>> >>platforms >>>> >> adhere to the W3C Geo spec, then these native changes would line up >>>>our >>>> >> implementation with what users are expecting in their browser. >>>> >> >>>> >> I can help with tweaking the implementation on iOS, but would love >>>>if >>>> >>you >>>> >> could once-over it, Shaz, and perhaps jump on a quick remote hack >>>>sesh >>>> >> with me for 15-20 mins to make sure we are looking good. >>>> >> >>>> >> On 3/27/12 2:46 PM, "Shazron" <[email protected]> wrote: >>>> >> >>>> >>>Thanks Fil - I'm all for fixing geolocation in iOS. There's several >>>> >>>jira issues for it, and I've been attempting to fix it as best I >>>>can, >>>> >>>but users are still reporting problems with it since it doesn't >>>>match >>>> >>>the native implementation of UIWebView. >>>> >>> >>>> >>>On Mon, Mar 26, 2012 at 4:00 PM, Filip Maj <[email protected]> wrote: >>>> >>>> Hey all, >>>> >>>> >>>> >>>> The past week or so I've been working on revamping the geolocation >>>> >>>>tests according to what is laid out by the W3C API [1]. Been >>>>tracking >>>> >>>>progress and whatnot in a JIRA issue [2]. >>>> >>>> >>>> >>>> Good news: I've got the tests implemented plus cordova-js passing >>>>said >>>> >>>>tests (compare view to see diff available @ [3]). >>>> >>>> >>>> >>>> Bad news: we've been doing it wrong in our native implementations >>>>forŠ >>>> >>>>well, ever, it seems. >>>> >>>> >>>> >>>> Moving forward would like to hear suggestions from everyone. >>>> >>>> >>>> >>>> Breaking down what we didn't do in the past that the spec >>>>mandates: >>>> >>>> >>>> >>>> * Properly implementing a timeout. It is one of the available >>>> >>>>options that you can pass into getCurrentPosition / watchPosition. >>>> >>>>However, we've been using it to date as essentially a "frequency" >>>> >>>>parameter for watchPosition, I.e. "give me position updates every >>>> >>>><options.timeout> milliseconds". This is wrong. According to the >>>>spec, >>>> >>>>the timeout option defines how long after invoking a >>>>watch/getCurrent >>>> >>>>the error callback should wait before it fires with a TIMEOUT >>>> >>>>PositionError object. >>>> >>>> * There is no control over how often watchPosition should fire >>>> >>>>success callbacks. Instead, the spec says: "In step 5.2.2 of the >>>>watch >>>> >>>>process, the successCallback is only invoked when a new position is >>>> >>>>obtained and this position differs signifficantly from the >>>>previously >>>> >>>>reported position. The definition of what consitutes a significant >>>> >>>>difference is left to the implementation." >>>> >>>> * I've also added tests + control of comparing the "maximumAge" >>>> >>>>parameter on the JS side, and keeping a reference to the last >>>> >>>>successful >>>> >>>>position retrieved from the native framework and comparing its >>>> >>>>timestamp >>>> >>>>together with maximumAge. This should implement proper caching of >>>> >>>>positioning on the WebView side and hopefully save some native >>>>round >>>> >>>>trips. >>>> >>>> >>>> >>>> All of this means the the API on the native side for geolocation >>>>will >>>> >>>>change (sorry iOS!). Basically we have three actions that the >>>> >>>>Geolocation plugin should listen for: >>>> >>>> >>>> >>>> * getLocation, which takes as parameters enableHighAccuracy >>>> >>>>(boolean) and maximumAge (int as milliseconds). >>>> >>>> * addWatch, parameter: only the usual callbackID required. >>>> >>>> * clearWatch, parameter: only the usual callbackID required. >>>> >>>> >>>> >>>> getLocation should require very little changing (other than not >>>> >>>>needing >>>> >>>>the timeout parameter anymore, since that is handled on the JS side >>>>in >>>> >>>>my patch). >>>> >>>> >>>> >>>> addWatch should keep a list of callback Ids, and, as soon as we >>>>have >>>> >>>>one watch started, the native framework should start watching the >>>> >>>>position for a "significant position difference". Once that >>>>happens, it >>>> >>>>should fire the success callback(s) for all stored watch callback >>>>Ids. >>>> >>>>If there is an issue retrieving position, it should fire the error >>>> >>>>callback(s) for all stored watch callback Ids. >>>> >>>> >>>> >>>> I commented out a bunch of iOS-specific code that already did a >>>> >>>>"distance filter" type of approach to position acquisition, but was >>>> >>>>only >>>> >>>>available if you provided undocumented parameters in the options >>>> >>>>object. >>>> >>>>Not sure about how feasible a distance filter is in Android, or >>>>Windows >>>> >>>>Phone, or our other supported platforms. >>>> >>>> >>>> >>>> One final point of discussion worth bringing up about this issue. >>>> >>>>BlackBerry and Android use the default implementation of >>>>geolocation >>>> >>>>abilities in their respective WEbViews. Because of this I would >>>>mandate >>>> >>>>removal of any Geolocation java code from the Android + BlackBerry >>>> >>>>implementations. Saves some bytes. Originally we had the Android >>>>plugin >>>> >>>>classes in there for support for devices before 2.0. Since we are >>>>only >>>> >>>>supporting 2.0 and above, this is no longer needed. Are there any >>>> >>>>issues >>>> >>>>with this? >>>> >>>> >>>> >>>> Appreciate you guys looking this over. >>>> >>>> >>>> >>>> Cheers, >>>> >>>> Fil >>>> >>>> >>>> >>>> [1] http://dev.w3.org/geo/api/spec-source.html#api_description >>>> >>>> [2] https://issues.apache.org/jira/browse/CB-359 >>>> >>>> [3] >>>> >>>> >>>> https://github.com/filmaj/incubator-cordova-js/compare/master...geotest >>>> >>>>s >>>> >> >>>> >>>> >> >
