Hallvord R. M. Steen

Prototype findOrStore considered harmful

Saturday, 7. July 2007, 11:08:40

prototype, compatibility, coding, decompile

Prototype 1.5.0 completely breaks CNN on certain mobile versions of Opera. All the text disappears.

We've dug up the source and it appears to be code from the common prototype.js that causes problems, CNN's prototype.js is here:

Element.extend.cache = {
  findOrStore: function(value) {
    return this[value] = this[value] || function() {
      return value.apply(null, [this].concat($A(arguments)));
    }
  }
};

Problem arises when the "value" argument is a function object. Then this[value] = this[value] requires decompilation, in order to create a string for the property name.

This is a bad idea because

Decompilation support is an optional feature. We disable it on many limited platforms for performance
Decompiling a function is in any case not a reliable way to create a unique hash key since functions can have exactly the same source code but live in different scopes or different closures, so this code as-is will create obscure bugs that are very hard to track down.

I hope the excellent Prototype devs can find a better approach. Unfortunately this code is still there in the latest version.

(Due to impatient family members I can't bug it in their tracker right now, post a link to the bug in comments if you do.)

DOMStorage and securityInteresting DOMContentLoaded question

Comments

HeroreV # 7. July 2007, 17:16

What would be a good solution to this problem? They can't use a function as the map key unless they implement a map class themselves. There is no way I know of to get an address, ID, hash, etc for an object. Should JavaScript be extended to solve this problem?

I guess I'd use something like this:
function getID(obj) {
if( obj === null ) return 0;
if( obj === undefined ) return 1;
if( ! obj.__id__ )
obj.__id__ = randNumGreaterThan1();
return obj.__id__;
}
And call it when I wanted a map key.
this[getID(value)] = this[value] || function() {...}

hallvors # 7. July 2007, 22:30

Yes, something like that is probably what I would do too.

Regarding your question "should JavaScript be extended..": quoting a somewhat related discussion on whether to add some form of unique object ID to ECMAScript 4 (quote is heavily snipped):

> What jumps out at me is the potential for developers to assume that a
> hashcode is the same as a unique ID (especially if some
> implementations enforce uniqueness while others don't), thus leading
> to subtle bugs. For that reason, I'd like to weigh in with the
> opinion that requiring uniqueness for hashcodes would be really
> nice. (Alternatively a separate mechanism for generating unique
> object IDs would be nice.)
<X>

If an implementation is going to guarantee uniqueness then it probably
needs to maintain a global table of all live objects whose hash codes
have been obtained, to avoid generating duplicate codes when the
global hash code counter wraps around (intrinsic::hashcode returns a
uint).
<X>
IMO we'd be better off following Java here and just spell out (more)
clearly that you can't use the hash code as an object ID.

Posted to ES4-discuss by Lars Thomas Hansen (sorry, listmembers-only archive but AFAIK anyone can join the list.)

hallvors # 7. July 2007, 22:50

Bug now filed:
http://dev.rubyonrails.org/ticket/8903

hallvors # 8. July 2007, 12:35

And it has been fixed already :-D

deanedwards # 14. July 2007, 08:34

How do we detect if decompilation is disabled? What does Function.prototype.toString() return instead? Does it error?

d.i.z. # 14. July 2007, 11:03

hallvors explained this in bug report:

Originally posted by hallvors:

Decompilation support is an optional feature of ECMAScript. Opera disables it on limited platforms (mobiles, devices) for performance. When the output of Function.prototype.toString() is "[ecmascript code]" this is the case.

hallvors # 20. July 2007, 14:39

Dean, good question - because I don't think we've been completely consistent in the past. It never throws an error but I seem to remember that some versions have returned the mixed case string "[ECMAScript code]" and some the all-lower case form..