[psa] JSONObject.optString doesn't do what you think it does

Jim Chen nchen at mozilla.com
Fri May 23 11:23:04 PDT 2014


I think a good compromise between correctness and practicality would be
to treat null and undefined as equivalent in NativeJSObject. If people
are expecting and using them as equivalents, we can just treat them as
such. This is really simple to do (one-line patch), and will make opt*
return the fallback value for any null properties. It will also let you
use NativeJSObject.has() in place of isNull().

Jim


On 5/23/14 1:43 PM, Nick Alexander wrote:
> On 2014-05-22, 8:50 PM, Jim Chen wrote:
>> I think the problem comes down to null being an object type in JS (i.e.
>> typeof null === typeof {} === 'object'). Unlike Java where a String
>> variable can be null to indicate absence of value, in JS there's no such
>> thing as a "null string", because null is not related to strings.
> 
>> That's why NativeJSObject will throw an IllegalArgumentException if you
>> call obj.getString("a") on obj={a:null}. It thinks "a" is an object, and
>> you're trying to treat it as a string. I think the correct approach here
>> is to use undefined to mark the absence of value instead of null. In
>> short, use undefined for a JS string where you would use null for a Java
>> String.
> 
> 
> This is technically true, but not relevant in practice.  We have a great
> deal of JS, not all of it in mobile/android (!), that treats undefined
> and null as equivalent.  Shaving that yak is not in the cards.
> 
>> And NativeJSObject is not an exact drop-in replacement for JSONObject.
> 
> This is a fine thing to assert, but code that was subtly wrong using
> JSONObject was wholesale converted to NativeJSObject.  That's a bug, yes
> -- but shows that even when the differences are most likely to be
> apparent, namely right when the new structure is introduced, the
> differences are not accounted for.  I think hoping that this will
> improve over time is optimistic in the extreme.  For better or worse,
> we're stuck with JSONObject's semantics.
> 
>> For example, the API for accessing arrays is different, and
>> NativeJSObject never does type conversion for you. With these
>> differences in mind, I don't think NativeJSObject has to be
>> bug-compatible with JSONObject either. We're still early enough in the
>> conversion process to make NativeJSObject behave in more reasonable
>> ways. It may take more effort now, but I think the possibility of
>> avoiding gotchas/subtle bugs down the road is worth it.
> 
> If we elect to do this, then we're making any *new* use of
> NativeJSObject better.  But we've just shown that any *conversion* from
> JSONObject is tough.  We have thousands of uses of JSONObject in the
> tree already.  Are we really going to turn a mechanical process into a
> manual review?
> 
> Nick


More information about the mobile-firefox-dev mailing list