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

Nick Alexander nalexander at mozilla.com
Fri May 23 10:43:26 PDT 2014


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