1132443 - eval code generated by the debugger shown in sources list

Status: RESOLVED WORKSFORME

(DevTools :: Debugger, defect)

Description

[Hallvord R. M. Steen [:hallvors]]

With the new eval support, the eval code the debugger creates to watch properties here http://mxr.mozilla.org/mozilla-central/source/browser/devtools/debugger/debugger-controller.js#1065 is piped back to the debugger, which thinks it is stuff running in the web page.

There is presumably a way to tell the debugger that this is not the web site's code - for example typing into the console works OK, so somehow the debugger knows that strings evaluated in the page because you type in the console are not page code.

It massively clutters the source list to have new eval entries for each watched variable each time you step or get a variable popup

This is a regression from 905700 and should be handled before that fix is released.

(And besides it breaks my fix for 967853 because the debugger thinks a script is being run in the page each time the variable popup opens, and closes it immediately. So I can't check in that fix until this gets fixed. The test failures mentioned in recent comments in that bug are due to this issue.)

Comment 1

[Panos Astithas (he/him) [:past] (please ni?)]

James may already have thought about this, but the controller generates the clientEvaluate request and keeps an account of pending requests, so it should be able to skip updating the source list in these cases.

Comment 2

[Codacoder]

See https://bugzilla.mozilla.org/show_bug.cgi?id=1111369
and https://bugzilla.mozilla.org/show_bug.cgi?id=1124106

Comment 3

[James Long (:jlongster)]

Ok. This may be the straw that breaks the camel's back (what a weird expression). I saw we hide eval sources that don't have a sourceURL pragma for now. We will uplift it all the way to beta because that's currently where eval debugging lives.

Comment 4

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

(In reply to Panos Astithas [:past] from comment #1)
> James may already have thought about this, but the controller generates the
> clientEvaluate request and keeps an account of pending requests, so it
> should be able to skip updating the source list in these cases.

+1, clientEvaluate can set a gensym'd sourceURL and a flag on the thread actor that onNewScript can test and use to ignore the appropriate scripts.

Comment 5

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

* and/or a flag

Comment 6

[Hallvord R. M. Steen [:hallvors]]

James: if you fix this by "hiding eval sources that don't have a sourceURL pragma", will the debugger still fire "thread-resumed" events on DebuggerController._toolbox.target each time one of those "watch" variable scripts is evaluated? That event is actually the underlying bug that is breaking my bug 967853 fix.

It sounds like hiding those entries from the scripts list but still firing confusing "thread-resumed" while standing at a breakpoint would be a pretty limited cosmetic fix leaving a serious source of confusion underneath.

Comment 7

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

This should be implemented on the server side. Although it could conceivably be done on the client, it would be inefficient and inelegant.

Comment 8

[Hallvord R. M. Steen [:hallvors]]

(Perhaps a fix here would also solve bug 1082612?? - another pet annoyance of mine ;))

Comment 9

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

(In reply to Hallvord R. M. Steen [:hallvors] from comment #8)
> (Perhaps a fix here would also solve bug 1082612?? - another pet annoyance
> of mine ;))

We have to do an RDP round trip to display the result of the watch expressions. Unrelated to this bug.

Comment 10

[James Long (:jlongster)]

Since bug 1124106 landed, we hide all unnamed eval scripts, so this should be fixed.