You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by Ian Clelland <ic...@chromium.org> on 2014/10/24 17:41:29 UTC
Re: [cordova-js] replaceNavigator
I'm sorry that I missed this, and that it never got the review that it
deserved -- thanks for at least taking the right step and giving the whole
dev community the chance to review it first, though.
According to a bug report this morning (
https://issues.apache.org/jira/browse/CB-7868), it looks like this code
breaks on Android, from versions 2.3 to 4.0.3. I haven't verified it, but
it looks likely. (Not sure whether 4.0.4 would be affected as well)
If I get a chance this afternoon, I'll see if I can come up with a
cross-platform solution for it. Otherwise, if anyone sees an obvious fix
before I get to it, feel free to jump in (Joe? Andrew?)
Ian
On Wed, Sep 10, 2014 at 12:09 PM, Shazron <sh...@gmail.com> wrote:
> PR: https://github.com/apache/cordova-js/pull/80
> Issue: https://issues.apache.org/jira/browse/CB-6911
>
> May I humbly request a review of the pull request above. Since it affects
> all platforms, I thought it would be prudent.
>
> Note the deprecation "error" is just a warning (not sure why Safari Web
> Inspector shows it with an error icon) since it doesn't really affect the
> app (no exception), but it's better to fix it now rather than later.
>
> replaceNavigator proxies window.navigator functions so that cordova can
> override/clobber window.navigator functions, this patch enables it to proxy
> properties as well, removing the error message.
>
Re: [cordova-js] replaceNavigator
Posted by Shazron <sh...@gmail.com>.
Yup, we could use those functions. I'll check them out.
On Mon, Oct 27, 2014 at 12:51 PM, Ian Clelland <ic...@chromium.org>
wrote:
> On Mon Oct 27 2014 at 3:41:48 PM Shazron <sh...@gmail.com> wrote:
>
> > Ouch, sorry about this :(
> >
>
> Don't be -- you did the right thing, and nobody actually tried it out on
> the devices it's failing on.
>
> I was talking to Andrew about it earlier, and he reminded me of the utility
> functions we have for this in cordova.js -- defineGetterSetter might be the
> one; I don't remember specifically. Would it be possible to use the utility
> methods instead? They're pretty well tested on all platforms.
>
> Ian
>
>
> > On Fri, Oct 24, 2014 at 8:41 AM, Ian Clelland <ic...@chromium.org>
> > wrote:
> >
> > > I'm sorry that I missed this, and that it never got the review that it
> > > deserved -- thanks for at least taking the right step and giving the
> > whole
> > > dev community the chance to review it first, though.
> > >
> > > According to a bug report this morning (
> > > https://issues.apache.org/jira/browse/CB-7868), it looks like this
> code
> > > breaks on Android, from versions 2.3 to 4.0.3. I haven't verified it,
> but
> > > it looks likely. (Not sure whether 4.0.4 would be affected as well)
> > >
> > > If I get a chance this afternoon, I'll see if I can come up with a
> > > cross-platform solution for it. Otherwise, if anyone sees an obvious
> fix
> > > before I get to it, feel free to jump in (Joe? Andrew?)
> > >
> > > Ian
> > >
> > > On Wed, Sep 10, 2014 at 12:09 PM, Shazron <sh...@gmail.com> wrote:
> > >
> > > > PR: https://github.com/apache/cordova-js/pull/80
> > > > Issue: https://issues.apache.org/jira/browse/CB-6911
> > > >
> > > > May I humbly request a review of the pull request above. Since it
> > affects
> > > > all platforms, I thought it would be prudent.
> > > >
> > > > Note the deprecation "error" is just a warning (not sure why Safari
> Web
> > > > Inspector shows it with an error icon) since it doesn't really affect
> > the
> > > > app (no exception), but it's better to fix it now rather than later.
> > > >
> > > > replaceNavigator proxies window.navigator functions so that cordova
> can
> > > > override/clobber window.navigator functions, this patch enables it to
> > > proxy
> > > > properties as well, removing the error message.
> > > >
> > >
> >
>
Re: [cordova-js] replaceNavigator
Posted by Ian Clelland <ic...@chromium.org>.
On Mon Oct 27 2014 at 3:41:48 PM Shazron <sh...@gmail.com> wrote:
> Ouch, sorry about this :(
>
Don't be -- you did the right thing, and nobody actually tried it out on
the devices it's failing on.
I was talking to Andrew about it earlier, and he reminded me of the utility
functions we have for this in cordova.js -- defineGetterSetter might be the
one; I don't remember specifically. Would it be possible to use the utility
methods instead? They're pretty well tested on all platforms.
Ian
> On Fri, Oct 24, 2014 at 8:41 AM, Ian Clelland <ic...@chromium.org>
> wrote:
>
> > I'm sorry that I missed this, and that it never got the review that it
> > deserved -- thanks for at least taking the right step and giving the
> whole
> > dev community the chance to review it first, though.
> >
> > According to a bug report this morning (
> > https://issues.apache.org/jira/browse/CB-7868), it looks like this code
> > breaks on Android, from versions 2.3 to 4.0.3. I haven't verified it, but
> > it looks likely. (Not sure whether 4.0.4 would be affected as well)
> >
> > If I get a chance this afternoon, I'll see if I can come up with a
> > cross-platform solution for it. Otherwise, if anyone sees an obvious fix
> > before I get to it, feel free to jump in (Joe? Andrew?)
> >
> > Ian
> >
> > On Wed, Sep 10, 2014 at 12:09 PM, Shazron <sh...@gmail.com> wrote:
> >
> > > PR: https://github.com/apache/cordova-js/pull/80
> > > Issue: https://issues.apache.org/jira/browse/CB-6911
> > >
> > > May I humbly request a review of the pull request above. Since it
> affects
> > > all platforms, I thought it would be prudent.
> > >
> > > Note the deprecation "error" is just a warning (not sure why Safari Web
> > > Inspector shows it with an error icon) since it doesn't really affect
> the
> > > app (no exception), but it's better to fix it now rather than later.
> > >
> > > replaceNavigator proxies window.navigator functions so that cordova can
> > > override/clobber window.navigator functions, this patch enables it to
> > proxy
> > > properties as well, removing the error message.
> > >
> >
>
Re: [cordova-js] replaceNavigator
Posted by Shazron <sh...@gmail.com>.
Ouch, sorry about this :(
On Fri, Oct 24, 2014 at 8:41 AM, Ian Clelland <ic...@chromium.org>
wrote:
> I'm sorry that I missed this, and that it never got the review that it
> deserved -- thanks for at least taking the right step and giving the whole
> dev community the chance to review it first, though.
>
> According to a bug report this morning (
> https://issues.apache.org/jira/browse/CB-7868), it looks like this code
> breaks on Android, from versions 2.3 to 4.0.3. I haven't verified it, but
> it looks likely. (Not sure whether 4.0.4 would be affected as well)
>
> If I get a chance this afternoon, I'll see if I can come up with a
> cross-platform solution for it. Otherwise, if anyone sees an obvious fix
> before I get to it, feel free to jump in (Joe? Andrew?)
>
> Ian
>
> On Wed, Sep 10, 2014 at 12:09 PM, Shazron <sh...@gmail.com> wrote:
>
> > PR: https://github.com/apache/cordova-js/pull/80
> > Issue: https://issues.apache.org/jira/browse/CB-6911
> >
> > May I humbly request a review of the pull request above. Since it affects
> > all platforms, I thought it would be prudent.
> >
> > Note the deprecation "error" is just a warning (not sure why Safari Web
> > Inspector shows it with an error icon) since it doesn't really affect the
> > app (no exception), but it's better to fix it now rather than later.
> >
> > replaceNavigator proxies window.navigator functions so that cordova can
> > override/clobber window.navigator functions, this patch enables it to
> proxy
> > properties as well, removing the error message.
> >
>