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 Wed, 28 Mar 2012 23:54:14 GMT
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
View raw message