incubator-callback-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Filip Maj <...@adobe.com>
Subject Re: Geolocation: Adhering to W3C Specification
Date Thu, 29 Mar 2012 19:56:03 GMT
Updated my branch with that change now.

On 3/29/12 11:31 AM, "Filip Maj" <fil@adobe.com> wrote:

>Good catch Becky, thanks. I'll patch that shortly.
>
>On 3/29/12 11:02 AM, "Becky Gibson" <gibson.becky@gmail.com> wrote:
>
>>I think there is an issue in the LocationManager: didUpdateToLocation:
>>fromLocation method.  The NSFastEnumeration protocol for NSDictionary
>>iterates over the keys and based how watchCallbacks is created I think
>>you
>>want the object.
>>
>>Thus, I think this:
>>
>>for (NSString *callbackId in self.locationData.watchCallbacks) {
>>            [self returnLocationInfo:callbackId];
>>        }
>>
>>should be:
>>
>>for (NSString *timerId in self.locationData.watchCallbacks) {
>>            [self returnLocationInfo:[self.locationData.watchCallbacks
>>objectForKey: timerId ]];
>>        }
>>
>>-becky
>>
>>On Wed, Mar 28, 2012 at 9:37 PM, Shazron <shazron@gmail.com> wrote:
>>
>>> Never mind - saw that NetworkStatus commit was already in the mainline
>>>as
>>> well
>>>
>>> 2012/3/28 Shazron <shazron@gmail.com>:
>>> > Oh - when you merge it in, don't squash your commits in this case
>>> > unless you list all the changes in the squash. Good to know about the
>>> > "Network Status" change since I needed to know about this in the
>>> > updated guides I'm writing.
>>> >
>>> > 2012/3/28 Filip Maj <fil@adobe.com>:
>>> >> Thanks for looking it over Shaz, much appreciated - the link helps
>>>too.
>>> I
>>> >> am slowly getting over my aversion of invoking functions using
>>>square
>>> >> brackets and accepting objective-c for what it is.
>>> >>
>>> >> On 3/28/12 4:26 PM, "Shazron" <shazron@gmail.com> 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-varia
>>>b
>>>l
>>> >>>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 <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...geotes
>>> >>>>>>>t
>>> >>>>>>> >>>>s
>>> >>>>>>> >>
>>> >>>>>>>
>>> >>>>>>>
>>> >>>>>
>>> >>>>
>>> >>
>>>
>


Mime
View raw message