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 18:31:53 GMT
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-variab
>>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