incubator-callback-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Becky Gibson <gibson.be...@gmail.com>
Subject Re: Geolocation: Adhering to W3C Specification
Date Thu, 29 Mar 2012 18:02:42 GMT
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-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 <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
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message