incubator-callback-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Shazron <shaz...@gmail.com>
Subject Re: Geolocation: Adhering to W3C Specification
Date Wed, 28 Mar 2012 23:26:53 GMT
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 <fil@adobe.com>:
> 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" <fil@adobe.com> 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" <bowserj@gmail.com> 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 <fil@adobe.com> 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" <shazron@gmail.com> 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 <fil@adobe.com> 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" <shazron@gmail.com> 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 <fil@adobe.com>
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
>>>> >>
>>>>
>>>>
>>
>

Mime
View raw message