One last question: assuming that I get the Android native tweaks in order and that's all working fine, you cool with us merging that in once the geo changes make their way into cordova-js?
On 3/28/12 4:26 PM, "Shazron" <[email protected]> wrote: >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-variabl >es-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...geotes >>>>>t >>>>> >>>>s >>>>> >> >>>>> >>>>> >>> >>
