1464164 - Users should be able to declare variables when paused in strict mode

Status: NEW

(DevTools :: Console, defect, P2)

Description

[Jason Laster [:jlast]]

This is a follow up on https://bugzilla.mozilla.org/show_bug.cgi?id=1375300.

Users currently cannot declare new variables in the console when paused in strict mode.

STR:

1. go to http://devtools-html.github.io/debugger-examples/examples/todomvc/
2. add a breakpoint in todo-view.js#34
3. add a todo
4. type `var a = 2` in the console

Firefox GIF: http://g.recordit.co/fTlqaahJrV.gif
NOTE: it is possible to declare variables when paused in sloppy mode.


I am a big fan of Chrome's current UX. Variable declarations are added in the global scope and become properties of the window. I think this approach has several benefits:

1. users can add variables locally
2. the variables are around on the next pause (in the same frame or other frames)
3. the variables are around when the debugger resumes
4. local variables will shadow the console’s variables

Chrome GIF: http://g.recordit.co/99ptD4YqY8.gif

Edge Cases:
- `const` and `let` do not need be supported. 
- should set all declarations in the global scope `var a = 2, b = 2` 
- should not leak local variables into the global scope `() => {var a = 2; }`

Comment 1

[Ted Campbell [:tcampbell]]

I'm wondering if the straight-forward way to achieve this is to consistently use DebugEnvironmentProxy for all items in the environment chain (rather than just debug hollows). Then I would make JSObject::isUnqualifiedVarObj()/isQualifiedVarObj() return false for DebugEnvironmentProxies, resulting in the global being used for new bindings. This would only need to apply to debugger evals (and maybe is a flag when we create the DebugEnvironmentProxy?).

Jason, any thoughts one way or another here? The current situation of hitting the sloppy-eval case when the user runs |var x = 42;| in console seems a little crazy.

Comment 2

[Jason Laster [:jlast]]

Following up from some prior discussions, it looks like the Debugger API might have tried to handle the case where we're paused and want to evaluate a console expression in the global context:

https://developer.mozilla.org/en-US/docs/Tools/Debugger-API/Debugger.Object

debugger.executeInGlobal(code, [options])
debugger.executeInGlobalWithBindings(code, bindings, [options])
frame.eval(code, [options])
frame.evalWithBindings(code, bindings, [options])

It's also possible that we are missing the appropriate semantics. I ran into Shu last night at the TC39 community event and he mentioned that executeInGlobal with this intention. Ideally the semantics would be similar to an inline script tag.

Comment 3

[Jason Laster [:jlast]]

We landed a client-side fix here: https://github.com/devtools-html/debugger.html/pull/5983

The solution leverages babel to transform the expression into a property assignment e.g. `var b = 3;` => `self.b = 3`

Comment 4

[Jason Laster [:jlast]]

We would still like to improve the engine so that we do not need the transform, but this option helps us better understand the problem while we finish other work on run to completion bugs

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

Redirect a needinfo that is pending on an inactive user to the triage owner.
:nchevobbe, since the bug has high priority, could you have a look please?

For more information, please visit auto_nag documentation.