Mailing-List: contact callback-dev-help@incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: callback-dev@incubator.apache.org
Received-SPF: pass (nike.apache.org: domain of fil@adobe.com designates
 64.18.1.37 as permitted sender)
From: Filip Maj <fil@adobe.com>
To: "callback-dev@incubator.apache.org" <callback-dev@incubator.apache.org>
Date: Thu, 29 Mar 2012 12:56:03 -0700
Subject: Re: Geolocation: Adhering to W3C Specification
Thread-Topic: Geolocation: Adhering to W3C Specification
Thread-Index: Ac0N5daIUgg4uxEQSI+q2o/maqVEIQ==
Message-ID: <CB9A0C5C.32E1%fil@adobe.com>
In-Reply-To: <CB99F89A.3289%fil@adobe.com>
Accept-Language: en-US
Content-Language: en-US
user-agent: Microsoft-MacOutlook/14.14.0.111121
acceptlanguage: en-US
Content-Type: text/plain; charset="iso-8859-2"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0

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=3D1160403 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=A9
>>> >>>>>>> >>>>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
>>> >>>>>>> >>
>>> >>>>>>>
>>> >>>>>>>
>>> >>>>>
>>> >>>>
>>> >>
>>>
>