842012 - break add line command is broken

Status: RESOLVED FIXED

(DevTools :: Debugger, defect, P2)

Description

[Victor Porof [:vporof][:vp]]

IIRC the "break add line" command should provide an autocompletion list with all the sources. It doesn't seem to work in Nightly.

STR:
1. Go to htmlpad.org/debugger, open debugger and gcli
2. Type break add line
3. Anything you type after that result in JS file URI, Can't use ".

Comment 1

[Josh Marchán]

I noticed this one and have been spending a lot of time poking around the break command code. Can I give it a shot?

Comment 2

[Josh Marchán]

Attachment: fix 'break add line' command

The command broke after https://github.com/joewalker/devtools-window/pull/324

This patch is kind of an icky way of getting it to work again. I'm not sure about adding args and execution context to the appropriate lookup() and data() calls in gcli. If that's the better way to do it, though, I can probably figure it out.

Comment 3

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

Comment on attachment 723771 [details] [diff] [review]
fix 'break add line' command

Victor found this so, finders, keepers!

Comment 4

[Victor Porof [:vporof][:vp]]

Comment on attachment 723771 [details] [diff] [review]
fix 'break add line' command

Review of attachment 723771 [details] [diff] [review]:
-----------------------------------------------------------------

Does the patch depend on some other bug? I couldn't apply this, and the changes in getPanel make me feel dumb :)
Can you rebase and ask me again for r?

::: browser/devtools/commandline/BuiltinCommands.jsm
@@ +380,5 @@
>          name: "file",
>          type: {
>            name: "selection",
> +          data: function() {
> +            let dbg = getPanel(HUDService.currentContext(), "jsdebugger");

Hmm, this looks extremely fishy to me. Why aren't you using getPanel here?

@@ +381,5 @@
>          type: {
>            name: "selection",
> +          data: function() {
> +            let dbg = getPanel(HUDService.currentContext(), "jsdebugger");
> +            return dbg?dbg.panelWin.DebuggerView.Sources.values:[];

Uber nit: spaces around ? and : please.

Comment 5

[Joe Walker [:jwalker]]

Bug 657595 is green in GCLI, but needs some integration with Firefox. I'm working on that.

Comment 6

[Josh Marchán]

I haven't forgotten about this bug -- I've been meaning to look into bug 657595 to see if it did anything wrt context/data for the data/lookup functions for SelectionType. I really don't like the original way I approached this patch. I really think the best thing to do here is to make sure the various types get access to the same context that the command's exec uses.

I remember we talked about it on IRC. I'll have to dig into my logs to remember the details of that conversation.

Comment 7

[Joe Walker [:jwalker]]

The problem, I think is that the "file" parameter needs to know about the current debugger in order to know what files are available. Until recently the args and context params to the data function were just not there. I *think* that they are now.
So maybe this will just work? I'm thinking aloud here!

Comment 8

[Victor Porof [:vporof][:vp]]

This doesn't seem to be automagically fixed, unfortunately :(

Comment 9

[Rob Campbell [:rc] (:robcee)]

still broken. Would love to have this fixed. Marking P2 because broken and assigned.

Comment 10

[Maria Grazia Alastra [:mga]]

This bug was introduced on Nightly 20.0 on 15 December 2012, 06:34.

User agent Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:20.0) Gecko/20121215 Firefox/20.0, Build ID: 20121215030840.

Here is the list of changes made between the Nightly build of the 14 December and the build of 15 December: http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2012-12-14+06%3A40&enddate=2012-12-15+05%3A40

There are a couple of changes that might be interesting, but this one got my attention: http://hg.mozilla.org/mozilla-central/rev/72cc10ffa8e2

However an issue a lot like this one (if you're interested I have some screenshots) was reproducible also in Firefox 17.0b1, 17.0 and 17.0.1, but was fixed in 18.0, probably by chance.

Comment 11

[Joe Walker [:jwalker]]

This should be fixed by the patch in bug 834230.

Comment 12

[Joe Walker [:jwalker]]

Marking RESOLVED|FIXED. Technically right now bug 834230 is only [fixed-in-fx-team] but this bug is fixed in that patch so we're not likely to lose track.