Bug 53497 – Web Inspector: do not hide scope variables with command line api.

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector (Deprecated)

[Pavel Feldman]

1. Stop on a breakpoint with "dir" or "$" variables in scope.
2. Eval "dir" in the console.

Expected: dir value
Actual: dir function from the command line API.

The reason is that we eval "with (commandLineApi) { expression }". It does not seem feasible to support both: command line api and proper eval in scope on a breakpoint. Options are:
a. do not inject command line api while on a breakpoint (pros: no confusion, cons: no inspect methods and other command line goodness)
b. inject an object with the API, so that user could type foo.inspect()
c. inject missing functions into the console object.

I am leaning toward (a) for now.

[Patrick Mueller]

So, with (a), you won't get the command line api at a breakpoint, but you will get it when you're NOT stopped at a breakpoint.  Seems confusing.  

Can we query the scope and only expose properties that won't shadow something on the stack?

In any case, it seems like having all the API in an object like "console" would be a nice capability, that we could support no matter what else we do?

Wonder what FireBug does - they would at least have a problem of not making shadowable API available - if they don't have the same problem we do.  Would be nice to have both debuggers have the same capability (in terms of adding all the API to console, or whatever).

[Jerome Duval]

If the fact that a value can be shadowed can be detected, then I think we could ask for disambiguation from the user (with a little popup menu?).

Stuffing the API in some object just makes it harder to have a collision, not impossible ;)

[Patrick Mueller]

(In reply to comment #2)
> If the fact that a value can be shadowed can be detected, then I think we could ask for disambiguation from the user (with a little popup menu?).
> 
> Stuffing the API in some object just makes it harder to have a collision, not impossible ;)

If the user shadows "console" with their own variable, they have bigger problems than shadowed functions from the console command-line.  eg, they won't be able to console.log().  "console" seems like a pretty safe namespace to hang some more debug API on.

The only thing I don't like about it, is that it make the name of something like "keys()" longer - "console.keys()".  But since it's something needed only for shadowed command-line APIs, don't think the long-form would get used much; nice to know it's there - but I will likely forget.  :-)

I wonder also about maybe having some kind of command-line prefix that would change the behavior.  For instance, if the first char in the command-line input is a ".", then don't use the command-line API when invoking the command.  Or something.

[Pavel Feldman]

(In reply to comment #1)
> So, with (a), you won't get the command line api at a breakpoint, but you will get it when you're NOT stopped at a breakpoint.  Seems confusing.  
> 
> Can we query the scope and only expose properties that won't shadow something on the stack?
> 
> In any case, it seems like having all the API in an object like "console" would be a nice capability, that we could support no matter what else we do?
> 
> Wonder what FireBug does - they would at least have a problem of not making shadowable API available - if they don't have the same problem we do.  Would be nice to have both debuggers have the same capability (in terms of adding all the API to console, or whatever).

I think FireBug does not export Command line API while on a breakpoint. Anyways, I was going to remove our support for it, but you gave me an idea:
  When command line API is needed, before every evaluation on a callframe, go through the command line api properties (log, inspect, etc.) and hide the ones that match any of the scope roots (local, closures, with) or global object properties. It looks like it should work and should not be expensive: we already have scope roots, checking for ("log" in window) should not be bad either. It will still hide non-enumerable properties, but that should be fine (window.foo) will still work as a workaround.

[Pavel Feldman]

Created attachment 81485 [details]
[PATCH] Proposed change.

[Yury Semikhatsky]

Comment on attachment 81485 [details]
[PATCH] Proposed change.

View in context: https://bugs.webkit.org/attachment.cgi?id=81485&action=review

> LayoutTests/inspector/console-api-on-call-frame.html:49
> +on a call frame.

Please add a link to the bug

[Pavel Feldman]

Committed r77924: <http://trac.webkit.org/changeset/77924>

[Csaba Osztrogonác]

(In reply to comment #7)
> Committed r77924: <http://trac.webkit.org/changeset/77924>

8-10 inspector tests fail on all bot because of this change.

[WebKit Review Bot]

http://trac.webkit.org/changeset/77924 might have broken Qt Linux Release
The following tests are not passing:
http/tests/inspector/extensions-headers.html
http/tests/inspector/extensions-resources-redirect.html
inspector/extensions-api.html
inspector/extensions-audits-api.html
inspector/extensions-audits.html
inspector/extensions-eval.html
inspector/extensions-events.html
inspector/extensions-resources.html

[Pavel Feldman]

(In reply to comment #9)
> http://trac.webkit.org/changeset/77924 might have broken Qt Linux Release
> The following tests are not passing:
> http/tests/inspector/extensions-headers.html
> http/tests/inspector/extensions-resources-redirect.html
> inspector/extensions-api.html
> inspector/extensions-audits-api.html
> inspector/extensions-audits.html
> inspector/extensions-eval.html
> inspector/extensions-events.html
> inspector/extensions-resources.html

Re-applied as r77929.