User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; ru; rv:1.8.0.4) Gecko/20060508 Firefox/1.5.0.4 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1) Gecko/20061001 BonEcho/2.0 Test case: chrome.manifest: overlay chrome://browser/content/browser.xul chrome://test/content/overlay.xul chrome://test/content/overlay.xul: <?xml version="1.0"?> <overlay id="testOverlay" xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"> <script type="application/x-javascript"> <![CDATA[ Function.prototype.f=function(){}; Function.prototype.s='string'; function dumpF(s){ dump('\n'+s+':Function.prototype.f='+(typeof Function.prototype.f)); } function dumpS(s){ dump('\n'+s+':Function.prototype.s='+(typeof Function.prototype.s)); } dumpF('without timeout'); dumpS('without timeout'); setTimeout("dumpF('0ms')",0); setTimeout("dumpS('0ms')",0); setTimeout("dumpF('1000ms')",1000); setTimeout("dumpS('1000ms')",1000); setTimeout("dumpF('5000ms')",5000); setTimeout("dumpS('5000ms')",5000); setTimeout("dumpF('10000ms')",10000); setTimeout("dumpS('10000ms')",10000); ]]> </script> </overlay> After several seconds "typeof Function.prototype.f" becomes "undefined" instead of being "function". Reproducible: Always Steps to Reproduce: 1. Apply overlay mentioned above to chrome://browser/content/browser.xul. 2. Set browser.dom.window.dump.enabled=true in about:config. (You can download and install a test extension which does items 1 and 2: http://xsms.nm.ru/temp/gecko/function_prototype/testxpi.xpi) 3. Run Firefox with "-console" flag. 4. Look into console for results Actual Results: without timeout:Function.prototype.f=function without timeout:Function.prototype.s=string 0ms:Function.prototype.f=function 0ms:Function.prototype.s=string 1000ms:Function.prototype.f=function 1000ms:Function.prototype.s=string 5000ms:Function.prototype.f=undefined 5000ms:Function.prototype.s=undefined 10000ms:Function.prototype.f=undefined 10000ms:Function.prototype.s=undefined Expected Results: without timeout:Function.prototype.f=function without timeout:Function.prototype.s=string 0ms:Function.prototype.f=function 0ms:Function.prototype.s=string 1000ms:Function.prototype.f=function 1000ms:Function.prototype.s=string 5000ms:Function.prototype.f=function 5000ms:Function.prototype.s=string 10000ms:Function.prototype.f=function 10000ms:Function.prototype.s=string This happens only with Firefox 2.0 nightly builds. There is no such problem in Firefox 1.5.0.x.

This is a regression from bug 343417, tested via local backout. http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/browser/components/safebrowsing/content/sb-loader.js&rev=1.10&mark=76,77#75 is what causes Function.prototype.f to be cleared - that component loads http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/toolkit/components/url-classifier/content/js/lang.js&rev=1.2#272 , which plays with Function.prototype. The timeouts are only needed because that component is instantiated on a timeout from the browser window load.

Bug 343417 was landed on branch over 2 months ago, and this testcase is the first report of this bug. That makes me wonder how badly (if at all) it will affect users if we release with it. Brendan/jst: gavin tells me you're investigating. Please let us know what you think the deleterious effects of this bug might be, and the risk involved in fixing it. Backing out bug 343417 seems like the cure might be worse than the disease.

(In reply to comment #2) > Brendan/jst: gavin tells me you're investigating. Please let us know what you > think the deleterious effects of this bug might be, and the risk involved in > fixing it. Backing out bug 343417 seems like the cure might be worse than the > disease. It looks related to overlays, but last night jst and I saw stuff debugging that was moderately alarming. Need to hear from jst today about what he learned. If only XUL overlays provoke the bug, perhaps we can wait for 1.8.1.1. We need to understand what's going on first to be sure. The alarming stuff has to-do with split window interactions with the JS standard environment initialization. /be

How does this test out? /be

(In reply to comment #4) > Created an attachment (id=241145) [edit] > patch based on jst's investigation and suggestion > > How does this test out? I tried testing this, but applying the patch and rebuilding in js/src resulted in a browser that would close immediately after being launched (it doesn't seem to crash).

Comment on attachment 241145 [details] [diff] [review] patch based on jst's investigation and suggestion Unfortunately this doesn't do it. For one, we end up getting here for globals with no inner, and even when I changed that to just use the context's global it still doesn't fix the problem. Still digging (had hardly any time for this tonight) :(

(In reply to comment #3) > was moderately alarming. Need to hear from jst today about what he learned. > If only XUL overlays provoke the bug, perhaps we can wait for 1.8.1.1. We I'm going to assume that course of action until we hear differently. We're triaging every day, so just renominate if this turns out to be a showstopper.

(In reply to comment #2) > Bug 343417 was landed on branch over 2 months ago, and this testcase is the > first report of this bug. Not really, the same bug affected the Custom Buttons extensions (http://adblockplus.org/development-builds/adding-filter-subscriptions-with-two-clicks#c000054). Unfortunately I didn't have time to investigate this issue propertly when I got this report, and Custom Buttons was easily fixed by avoiding using function prototypes.

This may be related to bug 357947, in which properties added to Object and Function are disappearing in windows opened with window.open.

Need to run, so this patch will need to explain itself for now. Mainly for mrbkap to review and test out, but should be what we want AFAICT.

Bug 358289 seems to be referring to the same issue (prototype.js library produces the error "(function(fragment){}).bind is not a function" with the testcase).

Comment on attachment 243737 [details] [diff] [review] Alternate approach. Testing this is going to be hard for me, but I like the patch.

Comment on attachment 243737 [details] [diff] [review] Alternate approach. Cool, thanks for reviewing mrbkap. As far as testing goes, I've checked and this does fix the testcase in this bug and also the testcase in bug 357947. Wladimir, any chance you could revert your changes to the Custom Buttons extension and see if this patch fixes that as well? As for what this patch does, the primary fix is to make js_FindClassObject innerize the object before looking up the class. But for that to work I had to tweak how the innerObject hook works in nsWindowSH. With this change it returns the outer if the outer is frozen, and with this patch an outer window is frozen on creation and remains frozen until the first document is loaded in it. That way we don't forward anything to the inner or create dummy inner objects while we're initializing the outer, and after that we do what we've always done. The bulk of this change is tweaking the freeze/thaw code to make it work as described above, the rest is the fixes in the innerObject hook and js_FindClassObject().

Sorry, recompiling Firefox is a little of a problem for me right now, you will have to test yourself. I reverted my changes, you can download the extension that still has this problem from http://adblockplus.org/trash/custombuttons-buggy.xpi. After you install put this URL into the location bar: custombutton://Options%5D%5Bdata%3Aimage/png%3Bbase64%2CiVBORw0KGgoAAAANSUhEUgAAABAAAAAQCAYAAAAf8/9hAAADHUlEQVR4Xl3DS2hcVRzA4d+55869M5NMppOEyctoxlgnsRgxRrSKpaQ+arG4kkJEBQmu1YUaoXQluNEuRUQ3xShiKdpiKSFpNRAosbQaY3yUGqOpcZI0Zh7JzJx7zt9FyaYffKrp0Gl2iBMkERsSJwOqWJsUZwtUTb/zvTnxlHH1CKxFRNjhcRPKurtVLcpHTelPtkcGP47yrZ/adHiy9kT+gsskjlKLwDlu5QHgZLA20HHa9GcnzGB7j7svS/Tknkc5/MDTHNyTckN3jDntHUN7RxC6AVA3q9TBr8Hqt0uH9r/DcAKKhl3f/EXjPxuoQKE9H9uZohQTNjHocz99oTYqI4JYAA/nEGe79aUNqEDDzDqt26Xr8Q7/VNjiz/jNmIZqmdvDgGyuC5eK97rt7dDV67iawRNR+01f+4vykAdFIblV/zXeknjcC/0jKu49Fe9KPRbetWvKyzhykaFpqG/AZpvHURxAgSfKv6fe05t0fe1gFFaH6WhFF1wpNIkgWcbJRS/QzyV2Zy4ke2K0PdIbY/Tws9Ld8R4mavPFRpGaM8ga0AultnR7tlp7NZ30jzZkkkirRu0Obuh08IavmE4uSag2I2S9soyi7Clnvg+u//CtXpgrsGIwQZx/W5vHYg2xsZiWprjEaCn4JLbUbNKq2XpFIcv/Qbm0LNaGqmH4FBJFOGvfre478CadLWAhJTVySTPVmdFnUil/Lkj4+WtV9frlRe6sGuDyPExPj6r43s/AWd92tX1n9g3vxSpIAEVQAolQaIxT3q6pxkoVHEAOmP4Nzk48o8KHxyGyzTaXnYieHx7k6iY0K+jMwBWgAvhADIgAU4W1eZiZO8lG8RUVPHiCmxi2udZxVotnqFptR+5/CT+vWAL6gXuBq8AVA1+e+Jut4iC+v+ohAiLgZEr/vDyg1iqjFDZfVucWPuc2oMPAR18t8OHkCnkDNxagXjuO1qsAHjsUSBAriAeiEda31vjjF1icLbP45wtM/v4Wx87Oc37mNax5H6UA8LmFWIsoBcWt43wwPUU9MsSDS9joRy5eO09ML6E9dvwPdX9rZvPeL7sAAAAASUVORK5CYII%3D%5D%5BopenPreferences%28%29%3B%5D%5B You will get an Options button in your Customize dialog, add it to a toolbar and restart Firefox. The current Firefox version shows the error mentioned above on startup.

Comment on attachment 243737 [details] [diff] [review] Alternate approach. Cool, I especially like the removal of the ad-hoc Freeze/Thaw around the JS_NewArrayObject -- any more like that? Prolly not. As jst and I were discussing in person, the followup bug for this one should ask why a scope chain ends in an outer window object, such that js_FindClassObject needs to innerize. The premise of the split window work was that no scope chain ends in an outer window. /be

Wladimir, unless I missed something obvious that works both with and without this patch, so I'm not sure what to make of that. I'm planning to land this change real soon now, so maybe you can test it out once it's in (trunk) and see if you can prove that problem fixed as well.

(In reply to comment #16) > (From update of attachment 243737 [details] [diff] [review] [edit]) > Cool, I especially like the removal of the ad-hoc Freeze/Thaw around the > JS_NewArrayObject -- any more like that? Prolly not. I didn't find any other ones like that. And in fact on the trunk that one doesn't exist any more either, which lead me and mrbkap to thinking that the bug this fixed has regressed on the trunk. But not so any more now that this is in on the trunk. I'll attach the trunk version of this patch too...

This one's in on the trunk now.

Johnny, I don't see this issue with CustomButtons on trunk either, it only occurs with Firefox 2.

(In reply to comment #16) > As jst and I were discussing in person, the followup bug for this one should > ask why a scope chain ends in an outer window object There is no scope chain in this case -- that's why we're using the context's globalObject which is, by definition, the outer window (since the outer window owns the context).

(In reply to comment #21) > (In reply to comment #16) > > As jst and I were discussing in person, the followup bug for this one should > > ask why a scope chain ends in an outer window object > > There is no scope chain in this case -- that's why we're using the context's > globalObject which is, by definition, the outer window (since the outer window > owns the context). Oh, right. In that case, the OBJ_INNER_OBJECT should be moved up into the else clause, and then then clause should assert that the last object is not an outer. /be

(In reply to comment #20) > Johnny, I don't see this issue with CustomButtons on trunk either, it only > occurs with Firefox 2. Wladimir, I did all my testing with the 1.8 branch, aka Firefox 2.

I can reproduce issue with CustomButtons in both Firefox 2.0 and current 1.8.1 branch nightly. I'll test it again when you land the patch.

*** Bug 357947 has been marked as a duplicate of this bug. ***

(In reply to comment #22) > Oh, right. In that case, the OBJ_INNER_OBJECT should be moved up into the else > clause, and then then clause should assert that the last object is not an > outer. That does fix the testcase in bug 357947, but not the testcase in this bug. I have yet to sit down and figure out exactly what's causing us to use an outer as a scope in the testcase in this bug...

*** Bug 359253 has been marked as a duplicate of this bug. ***

So what is the status of this making it to a release?

(In reply to comment #28) > So what is the status of this making it to a release? Look at the nomination flags, the blocking1.8.1.1? in particular. That should turn into blocking1.8.1.1+ very soon. The Mozilla and Gecko 1.8.1.1 version lines up with Firefox 2.0.0.1, the first patch release. jst, hope you don't mind my giving you this, since you patched. Can it be marked FIXED now? /be

Comment on attachment 243737 [details] [diff] [review] Alternate approach. approved for 1.8 branch, a=dveditz for drivers

This patch was checked into the trunk, assuming it supposed to be FIXED.

Fix landed on the 1.8 branch.

FYI: According to http://bugzilla.mozdev.org/show_bug.cgi?id=15700 this affects web pages as well.

*** Bug 360267 has been marked as a duplicate of this bug. ***

Verified in Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1) Gecko/20061115 BonEcho/2.0 - the problematic CustomButtons version from above works, test cases in bug 357947 and http://bugzilla.mozdev.org/show_bug.cgi?id=15700 don't show any more problems either.

*** Bug 358289 has been marked as a duplicate of this bug. ***

This patch caused bug 358755 (which is basically a regression of bug 348990 and bug 323641).