Android null result #427
Android null result #427
Conversation
| @@ -396,7 +396,7 @@ internal fun JsonSerializable.pluginResult(): PluginResult { | |||
| val json = this.toJsonValue() | |||
|
|
|||
| return when { | |||
| json.isNull -> PluginResult(PluginResult.Status.OK) | |||
| json.isNull -> PluginResult(PluginResult.Status.OK, json.string) | |||
There was a problem hiding this comment.
instead of json.string should it just be null ?
There was a problem hiding this comment.
null is not supported by cordova
There was a problem hiding this comment.
oh, this is literally passing "null"
There was a problem hiding this comment.
Looks like we use to do an empty string for null - https://github.com/urbanairship/urbanairship-cordova/blob/14.11.0/urbanairship-cordova/src/android/UAirshipPlugin.java#L794
There was a problem hiding this comment.
Yes, an empty string is fine too
There was a problem hiding this comment.
I am sure your change is good but let me double check the difference between iOS and Android
There was a problem hiding this comment.
There is also https://documentation.opencms.org/javadoc/core/org/opencms/json/JSONObject.Null.html that we can try
There was a problem hiding this comment.
It looks like it does support null - https://github.com/apache/cordova-android/blob/master/framework/src/org/apache/cordova/PluginResult.java#L42C46-L42C63
try (String)null
There was a problem hiding this comment.
Here is the obj-c one where instead of the message from the status its just nil. I doubt cordova will change now but this should be an upstream bug IMO - https://github.com/apache/cordova-ios/blob/master/CordovaLib/Classes/Public/CDVPluginResult.m#L115
What do these changes do?
Standardise null result on android and ios
Why are these changes necessary?
Should return a "null" result on Android instead of "OK"
How did you verify these changes?
Run demo app
Verification Screenshots:
iPhone
Anything else a reviewer should know?