User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b11pre) Gecko/20110127 Firefox/4.0b11pre Build Identifier: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b11pre) Gecko/20110127 Firefox/4.0b11pre On the recently updated Apple.com, many effects are added and there's this search field thing: upon focused and unfocused it changes color and width. And then some strange behaviors show up. For one the color transition seems not to be working, but that's not the case discussing here so we'll just pass on that. When the width animation is happening, it gets really choppy which I thought was just another performance issue, but then I noticed that it actually has something to do with certain mouse actions. More specifically, clicking results in choppy animations, while clicking and then holding yields nice smooth animation. Follow the screencast to test it for yourself. Reproducible: Always

Tested with the same outcome on latest Mac and Win build.

Not sure if it's with Javascript or CSS though.

It looks like it's done with JS. It also looks like they start the animation on mousedown (or focus?), then do _something_ on mouseup that takes a bunch of time (which is why you don't see the problem if you don't mouse up). The "something" is spending almost all of its time under fun_toString calling JS_DecompileFunction.

The relevant script is the obfuscated hell that is http://images.apple.com/global/metrics/js/s_code_h.js The toString calls come by way of valueOf, while doing a JSOP_EQ. The function that's at the top of the JS stack when this happens looks like it was created like so: s.manageVars = new Function("c", "l", "f", "var s=this,vl,la,vla;l=l?l:'';f=f?f:1 ;if(!s[c])return false;vl='pageName,purchaseID,channel,server,pageType,campaign,state,zip,events,products,transactionID';for(var n=1;n<76;n++){vl+=',prop'+n+',eVar'+n+',hier'+n;}if(l&&(f==1||f==2)){if(f==1){vl=l;}if(f==2){la=s.split(l,',');vla=s.split(vl,',');vl='';for(x in la){for(y in vla){if(la[x]==vla[y]){vla[y]='';}}}for(y in vla){vl+=vla[y]?','+vla[y]:'';}}s.pt(vl,',',c,0);return true;}else if(l==''&&f==1){s.pt(vl,',',c,0);return true;}else{return false;}"); and when it's called 'f' is 2, 'l' is a string, 'c' is a string. The JSOP_EQ in question has the string from 'l' as lval, and a function as rval, as far as I can tell.

OK, so the relevant compare is this one: if(la[x]==vla[y]) Now la and vla are both returns values from split, hence arrays.... But for/in on an array will enumerate things other than the indexed properties, if they're user-set. In this case, looks like vla[y] is a function object. y seems to have the value "each". If I look at the JSScript for vla[y] it claims to live inside http://images.apple.com/global/scripts/lib/prototype.js (the line number it claims is 550, which seems to be wrong, but I think there's little doubt that the issue is this little gem on line 828 of that file: Object.extend(Array.prototype, Enumerable); Enumerable has an "each" method. It has a bunch of other methods too, so we're doing at least 30-40 function-stringification operations per element of |la|. And |la| has a number of elements in it. I can't say how many times this code is called, but in all every click on the page calls fun_toString 10568 times. Not a typo there. 5-digit number. Which is why it takes long enough to be user-perceptible. None of this obviously browser-sniffed, so I think we just need to make this faster. :( Obvious options are: 1) Cache the result of fun_toString in the function object. 2) Make JSOP_EQ do something clever when doing toString and fun_toString is involved. Other ideas?

sfink points out that there's only one call to this code, probably. |vl| has 240 elements, and 240*44 is about the total number of fun_toString calls here.

Yeah, that fixes things.

Comment on attachment 507792 [details] [diff] [review] apple sucks ok, if it bounces at all for anything ever, it's FF5.

CC'ing Christian. We should reach out to Apple and tell them their website does this kind of insanity.

For what it's worth, I filed radar item 8929169 back when I finished sorting out what was going on here. Now the only problem is them noticing it... ;)

Sent to the try server, since I really want this to stick.

We sure other browsers behave the same? Or did we break something such that we are enumerating functions where others don't? /be

Executing this on apple.com: javascript:var x = ""; for (var __ in []) x += __ + ","; alert(x); Indicates that all browsers I have enumerate the function properties on arrays on that page (and that there are 45 such properties in Safari, 46 in Chrome, 40 in Opera and Firefox). What other browsers do differently from us is not decompile to stringify functions.

Thanks, bz. Nice fast work, Andreas! /be

clear() doesn't free the memory allocated for the hash table, like in bug 614155 comment 7.

Probably ok. Compartments go away. Any that do what Apple did here deserve the temporary memory bloat. /be

It seems like nobody noticed the somewhat obvious fact that prototype.js is a commonly used javascript framework and all blame was put on Apple. Apple is using an old version of Prototype and the problematic Object.extend(Array.prototype, Enumerable); line has been removed the the past two releases of Prototype so Apple does deserve some blame, but there's many other sites out there using old versions of the framework too.

Thanks for the detailed report and screencast, by the way. The patch seems to stick so this will be fixed in FF4.

Should we make a patch for 1.9.2? I would definitely leave 1.9.1 behind, its really time to update, but 1.9.2 might be worth it. Drivers?

cavanc@hotmail.com the blame is on apple because using |for..in| to iterate over arrays is a known "don't ever do this" kind of thing, precisely because it will iterate everything enumerable on Array.prototype and Object.prototype in addition to the things that are actually in the array. The person who wrote that code is lucky they just ended up with a performance hit instead of a security bug...

cdleary-bot mozilla-central merge info: http://hg.mozilla.org/mozilla-central/rev/e201f9d09799