You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by Andrew Grieve <ag...@google.com> on 2012/10/25 16:52:39 UTC
Re: android commit: Fix exception when plugin returns a null string.
Hey Bryce,
I don't think this change is quite correct. Looks like it's converting null
into an empty string. How did you run into this? Mobile spec? If it's
something we need to support (looks like it is, iOS handles it at least),
it'll need JS changes as well, so will need another re-tag. Worth it?
On Wed, Oct 24, 2012 at 2:37 PM, <bc...@apache.org> wrote:
> Updated Branches:
> refs/heads/master 7d3afcab9 -> cba0d5902
>
>
> Fix exception when plugin returns a null string.
>
>
> Project:
> http://git-wip-us.apache.org/repos/asf/incubator-cordova-android/repo
> Commit:
> http://git-wip-us.apache.org/repos/asf/incubator-cordova-android/commit/cba0d590
> Tree:
> http://git-wip-us.apache.org/repos/asf/incubator-cordova-android/tree/cba0d590
> Diff:
> http://git-wip-us.apache.org/repos/asf/incubator-cordova-android/diff/cba0d590
>
> Branch: refs/heads/master
> Commit: cba0d59021a2562af6888050a171d4ba3cdbe47e
> Parents: 7d3afca
> Author: Bryce Curtis <cu...@gmail.com>
> Authored: Wed Oct 24 12:36:30 2012 -0600
> Committer: Bryce Curtis <cu...@gmail.com>
> Committed: Wed Oct 24 12:36:30 2012 -0600
>
> ----------------------------------------------------------------------
> .../org/apache/cordova/NativeToJsMessageQueue.java | 13 ++++++++++---
> 1 files changed, 10 insertions(+), 3 deletions(-)
> ----------------------------------------------------------------------
>
>
>
> http://git-wip-us.apache.org/repos/asf/incubator-cordova-android/blob/cba0d590/framework/src/org/apache/cordova/NativeToJsMessageQueue.java
> ----------------------------------------------------------------------
> diff --git a/framework/src/org/apache/cordova/NativeToJsMessageQueue.java
> b/framework/src/org/apache/cordova/NativeToJsMessageQueue.java
> index 55fa205..03a69b1 100755
> --- a/framework/src/org/apache/cordova/NativeToJsMessageQueue.java
> +++ b/framework/src/org/apache/cordova/NativeToJsMessageQueue.java
> @@ -398,7 +398,12 @@ public class NativeToJsMessageQueue {
> ret += 1 + pluginResult.getMessage().length();
> break;
> case PluginResult.MESSAGE_TYPE_STRING: // s
> - ret += 1 + pluginResult.getStrMessage().length();
> + if (pluginResult.getStrMessage() == null) {
> + ret += 1;
> + }
> + else {
> + ret += 1 + pluginResult.getStrMessage().length();
> + }
> break;
> case PluginResult.MESSAGE_TYPE_JSON:
> default:
> @@ -433,8 +438,10 @@ public class NativeToJsMessageQueue {
> .append(pluginResult.getMessage());
> break;
> case PluginResult.MESSAGE_TYPE_STRING: // s
> - sb.append('s')
> - .append(pluginResult.getStrMessage());
> + sb.append('s');
> + if (pluginResult.getStrMessage() != null) {
> + sb.append(pluginResult.getStrMessage());
> + }
> break;
> case PluginResult.MESSAGE_TYPE_JSON:
> default:
>
>
Re: android commit: Fix exception when plugin returns a null string.
Posted by Andrew Grieve <ag...@chromium.org>.
Cool. Yeah, probably worth fixing then since it's not hard to fix & test.
I'll fix and re-tag. All the JS changes will be android-specific so other
platforms will not need to update their JS after the re-tag.
On Thu, Oct 25, 2012 at 11:04 AM, Simon MacDonald <simon.macdonald@gmail.com
> wrote:
> Hey Andrew,
>
> It is quite easy to reproduce. If you PluginResult is constructed like
> this:
>
> String result = null;
>
> return new PluginResult(status, result);
> Obviously, that is a contrived example but the actual place we ran into
> this was getting a shared preference that wasn't set so it returned a null
> which we passed back to the JS side and that's where things fell down.
>
> Simon Mac Donald
> http://hi.im/simonmacdonald
>
>
> On Thu, Oct 25, 2012 at 10:52 AM, Andrew Grieve <ag...@google.com>
> wrote:
>
> > Hey Bryce,
> >
> > I don't think this change is quite correct. Looks like it's converting
> null
> > into an empty string. How did you run into this? Mobile spec? If it's
> > something we need to support (looks like it is, iOS handles it at least),
> > it'll need JS changes as well, so will need another re-tag. Worth it?
> >
> >
> >
> >
> > On Wed, Oct 24, 2012 at 2:37 PM, <bc...@apache.org> wrote:
> >
> > > Updated Branches:
> > > refs/heads/master 7d3afcab9 -> cba0d5902
> > >
> > >
> > > Fix exception when plugin returns a null string.
> > >
> > >
> > > Project:
> > > http://git-wip-us.apache.org/repos/asf/incubator-cordova-android/repo
> > > Commit:
> > >
> >
> http://git-wip-us.apache.org/repos/asf/incubator-cordova-android/commit/cba0d590
> > > Tree:
> > >
> >
> http://git-wip-us.apache.org/repos/asf/incubator-cordova-android/tree/cba0d590
> > > Diff:
> > >
> >
> http://git-wip-us.apache.org/repos/asf/incubator-cordova-android/diff/cba0d590
> > >
> > > Branch: refs/heads/master
> > > Commit: cba0d59021a2562af6888050a171d4ba3cdbe47e
> > > Parents: 7d3afca
> > > Author: Bryce Curtis <cu...@gmail.com>
> > > Authored: Wed Oct 24 12:36:30 2012 -0600
> > > Committer: Bryce Curtis <cu...@gmail.com>
> > > Committed: Wed Oct 24 12:36:30 2012 -0600
> > >
> > > ----------------------------------------------------------------------
> > > .../org/apache/cordova/NativeToJsMessageQueue.java | 13
> ++++++++++---
> > > 1 files changed, 10 insertions(+), 3 deletions(-)
> > > ----------------------------------------------------------------------
> > >
> > >
> > >
> > >
> >
> http://git-wip-us.apache.org/repos/asf/incubator-cordova-android/blob/cba0d590/framework/src/org/apache/cordova/NativeToJsMessageQueue.java
> > > ----------------------------------------------------------------------
> > > diff --git
> a/framework/src/org/apache/cordova/NativeToJsMessageQueue.java
> > > b/framework/src/org/apache/cordova/NativeToJsMessageQueue.java
> > > index 55fa205..03a69b1 100755
> > > --- a/framework/src/org/apache/cordova/NativeToJsMessageQueue.java
> > > +++ b/framework/src/org/apache/cordova/NativeToJsMessageQueue.java
> > > @@ -398,7 +398,12 @@ public class NativeToJsMessageQueue {
> > > ret += 1 + pluginResult.getMessage().length();
> > > break;
> > > case PluginResult.MESSAGE_TYPE_STRING: // s
> > > - ret += 1 + pluginResult.getStrMessage().length();
> > > + if (pluginResult.getStrMessage() == null) {
> > > + ret += 1;
> > > + }
> > > + else {
> > > + ret += 1 +
> > pluginResult.getStrMessage().length();
> > > + }
> > > break;
> > > case PluginResult.MESSAGE_TYPE_JSON:
> > > default:
> > > @@ -433,8 +438,10 @@ public class NativeToJsMessageQueue {
> > > .append(pluginResult.getMessage());
> > > break;
> > > case PluginResult.MESSAGE_TYPE_STRING: // s
> > > - sb.append('s')
> > > - .append(pluginResult.getStrMessage());
> > > + sb.append('s');
> > > + if (pluginResult.getStrMessage() != null) {
> > > + sb.append(pluginResult.getStrMessage());
> > > + }
> > > break;
> > > case PluginResult.MESSAGE_TYPE_JSON:
> > > default:
> > >
> > >
> >
>
Re: android commit: Fix exception when plugin returns a null string.
Posted by Simon MacDonald <si...@gmail.com>.
Hey Andrew,
It is quite easy to reproduce. If you PluginResult is constructed like this:
String result = null;
return new PluginResult(status, result);
Obviously, that is a contrived example but the actual place we ran into
this was getting a shared preference that wasn't set so it returned a null
which we passed back to the JS side and that's where things fell down.
Simon Mac Donald
http://hi.im/simonmacdonald
On Thu, Oct 25, 2012 at 10:52 AM, Andrew Grieve <ag...@google.com> wrote:
> Hey Bryce,
>
> I don't think this change is quite correct. Looks like it's converting null
> into an empty string. How did you run into this? Mobile spec? If it's
> something we need to support (looks like it is, iOS handles it at least),
> it'll need JS changes as well, so will need another re-tag. Worth it?
>
>
>
>
> On Wed, Oct 24, 2012 at 2:37 PM, <bc...@apache.org> wrote:
>
> > Updated Branches:
> > refs/heads/master 7d3afcab9 -> cba0d5902
> >
> >
> > Fix exception when plugin returns a null string.
> >
> >
> > Project:
> > http://git-wip-us.apache.org/repos/asf/incubator-cordova-android/repo
> > Commit:
> >
> http://git-wip-us.apache.org/repos/asf/incubator-cordova-android/commit/cba0d590
> > Tree:
> >
> http://git-wip-us.apache.org/repos/asf/incubator-cordova-android/tree/cba0d590
> > Diff:
> >
> http://git-wip-us.apache.org/repos/asf/incubator-cordova-android/diff/cba0d590
> >
> > Branch: refs/heads/master
> > Commit: cba0d59021a2562af6888050a171d4ba3cdbe47e
> > Parents: 7d3afca
> > Author: Bryce Curtis <cu...@gmail.com>
> > Authored: Wed Oct 24 12:36:30 2012 -0600
> > Committer: Bryce Curtis <cu...@gmail.com>
> > Committed: Wed Oct 24 12:36:30 2012 -0600
> >
> > ----------------------------------------------------------------------
> > .../org/apache/cordova/NativeToJsMessageQueue.java | 13 ++++++++++---
> > 1 files changed, 10 insertions(+), 3 deletions(-)
> > ----------------------------------------------------------------------
> >
> >
> >
> >
> http://git-wip-us.apache.org/repos/asf/incubator-cordova-android/blob/cba0d590/framework/src/org/apache/cordova/NativeToJsMessageQueue.java
> > ----------------------------------------------------------------------
> > diff --git a/framework/src/org/apache/cordova/NativeToJsMessageQueue.java
> > b/framework/src/org/apache/cordova/NativeToJsMessageQueue.java
> > index 55fa205..03a69b1 100755
> > --- a/framework/src/org/apache/cordova/NativeToJsMessageQueue.java
> > +++ b/framework/src/org/apache/cordova/NativeToJsMessageQueue.java
> > @@ -398,7 +398,12 @@ public class NativeToJsMessageQueue {
> > ret += 1 + pluginResult.getMessage().length();
> > break;
> > case PluginResult.MESSAGE_TYPE_STRING: // s
> > - ret += 1 + pluginResult.getStrMessage().length();
> > + if (pluginResult.getStrMessage() == null) {
> > + ret += 1;
> > + }
> > + else {
> > + ret += 1 +
> pluginResult.getStrMessage().length();
> > + }
> > break;
> > case PluginResult.MESSAGE_TYPE_JSON:
> > default:
> > @@ -433,8 +438,10 @@ public class NativeToJsMessageQueue {
> > .append(pluginResult.getMessage());
> > break;
> > case PluginResult.MESSAGE_TYPE_STRING: // s
> > - sb.append('s')
> > - .append(pluginResult.getStrMessage());
> > + sb.append('s');
> > + if (pluginResult.getStrMessage() != null) {
> > + sb.append(pluginResult.getStrMessage());
> > + }
> > break;
> > case PluginResult.MESSAGE_TYPE_JSON:
> > default:
> >
> >
>