Created attachment 611438 [details] [diff] [review] v1
This doesn't quite seem right... In particular, wouldn't JS_HasProperty test true for a Window which has a <div id="XMLHttpRequest"> in it? We really want a JS_HasOwnProperty here. Maybe. At least in the window context that should work better. Modulo whatever happens when you reenter resolve hooks, of course.
Yup, it should be HasOwnProperty.
Created attachment 612696 [details] [diff] [review] v2 For some reason I can't make this fail in an xpcshell test :-/.
Comment on attachment 612696 [details] [diff] [review] v2 Actually, I forgot to test the id case.
And my question about resolve hook reentry stands, btw....
This needs to happen for fx14, since it breaks Adblock Plus if nothing else.
I stopped using Components.Constructor in Adblock Plus, new version should be released soon. Still, this is quite a bit of platform breakage - XMLHttpRequest should be the most common use of Components.Constructor.
(In reply to Boris Zbarsky (:bz) from comment #6) > And my question about resolve hook reentry stands, btw.... The JS engine already protects resolve hooks, see AutoResolving.
Yes, the question was what that protection will end up doing in this case.
Ah, I think it makes the JS_AlreadyHasOwnProperty not find the property.
Definitively an addon-compat worthy issue. CC'ing Jorge.
Created attachment 620297 [details] [diff] [review] v2 This doesn't change anything in the order of name resolution on windows from before the landing of the new DOM bindings. I think I've answered the questions about this patch, let me know if I didn't.
Comment on attachment 620297 [details] [diff] [review] v2 r=me
Comment on attachment 620297 [details] [diff] [review] v2 [Approval Request Comment] Regression caused by (bug #): bug 740069 User impact if declined: Broken addons (Addblock Plus was fixed, but this might affect others) Testing completed (on m-c, etc.): landed on m-c Risk to taking this patch (and alternatives if risky): low risk String changes made by this patch: None
(In reply to Peter Van der Beken [:peterv] from comment #18) > User impact if declined: Broken addons (Addblock Plus was fixed, but this > might affect others) It in fact does affect other add-ons, at least 3 of mine, having 2M+ ADU. addon-mxr shows a bunch of other users. Not sure how much are indeed affected, as this requires calling the constructor at least twice.
Comment on attachment 620297 [details] [diff] [review] v2 [Triage Comment] New regression in FF14 that has the potential to affect a large population. Approving for Aurora 14.
Mozilla/5.0 (X11; Linux i686; rv:14.0) Gecko/20100101 Firefox/14.0 Verified in F14 beta 8, on Ubuntu 12.04, Windows 7 and Mac OS 10.6 using attachment test case. Timestamp: 06/22/2012 02:57:09 PM Error: XMLHttpRequest objects created successfully Source File: jar:file:///email@example.com!/Module.js Line: 15