1. Open the Web Console on a page 2. In the JS term, enter: [] instanceof Array 3. Hit enter You would expect to see True returned, but instead see False. This is likely because we are running the console's through evalInSandbox and don't have full access to the global scope. Same applies to ({}) instanceof Object and probably several other examples.

You might also look at bug 597872

stealing this bug as I have a fix. Patch also invokes evalInSandbox with JS 1.8.

removes with() as the helper functions are already applied to the sandbox. Removed .wrappedJSObject from _window. needs a test.

Discussed this on IRC, but just adding here for the benefit of non-Robs: looks like bug 581069 removed JSTerm helper test coverage (http://hg.mozilla.org/mozilla-central/rev/6160fe29dd37#l3.7), so we should probably add that back before landing this.

(In reply to comment #2) > stealing this bug as I have a fix. > > Patch also invokes evalInSandbox with JS 1.8. Why the "default" -> "1.8" change? Is that beneficial? I don't think people will expect |var let;| to throw. Interaction with the page may be affected as well...

adding dependency to bug 605565 as I've updated those tests and they provide a convenient place to attach a new testcase. Natch: the addition of "1.8" was based on feedback from developers. Maybe we should make it an option?

(In reply to comment #6) > Natch: the addition of "1.8" was based on feedback from developers. Ah, in that case I am wrong :) > should make it an option? The reviewer will probably decide that. |var let;| doesn't throw now in the error console code input, but this is quite different.

ok, now that we have jsterm helper tests again, this patch breaks them. Back to the drawing board!

WIP patch. Currently fails the jshelper tests due to "Console not defined".

now with the patch flag!

Changed the way we're invoking evalInSandbox. Removed withs, added helper functions directly to the sandbox object. Using new constructor method for Sandbox. Added tests for instanceof and checked for return of XrayWrappers. This should block as it's a much better implementation of the command-line execution.

almost forgot, removed all instances of unwrap() in the helper methods.

I ran this through try last night. 3 spurious warnings, lots of green.

just to add a little more context to the patch that the summary of this bug doesn't relate: This fixes: * removes nested with() calls from the command line (with() is bad!). * removes all instances of XPCNativeObject.unwrap() from the command line. * adds the helper functions directly to the sandbox object. * uses the new constructor mechanism for Sandbox objects. This fixes the instanceof problem mentioned above as well as eliminates some kludgey code from the js command line. I think it's important, makes the code cleaner and more understandable, and is based on feedback from mrbkap about the proper way to use Sandboxes.

Can you generate a diff -w of this, I think it might make it easier to review.

Now with -w for easier digestion.

Comment on attachment 486087 [details] [diff] [review] [checked-in] v3 Looks good

I guess this can go into b7; I'm not sure I understand the rush, but I also don't believe that there's any significant risk.

it's low-risk and I think people seeing the words "object XrayWrapper" in front of every object they evaluate will be confusing and weird. Plus, I want the console to work correctly and don't think it should wait.

Comment on attachment 486087 [details] [diff] [review] [checked-in] v3 http://hg.mozilla.org/mozilla-central/rev/bcfefd30c2a6