Should have a shortcut key to focus the input line in the web console.

ctrl/cmd-shift-L ?

Input from UX team wanted here.

Per a discussion with Kevin I'm going to propose ⌘⇧I (for "Insert"?)

I believe this breaks string freeze, so marking as [strings].

I proposed cmd/ctrl-shift-I for "input" :) how does this break string freeze? it needs to appear on a menu somewhere?

This just adds new key bindings, no actual strings. You won't display "Insert" on any UI, correct?

Well, I thought keybindings generally went in properties files. For example, all the keybindings in browser/ that I can see are in .dtd files.

they do, and it is a new key that'll have to be translated, so yes, I think it does break string freeze.

Fixed in the patch for bug 612253.

Er, make that bug 612252.

mass change: filter on PRIORITYSETTING

Kevin's suggestion of cmd/ctrl-shift-I seems good, and while this bug is old it looks like there's still no shortkey. Can we get a patch for this?

We use this shortcut for the Inspector.

One idea would be to make the web console shortcut key (Ctrl-Shift-K/Cmd-Opt-K) have a slightly modified state-dependent action: (state: action) - console closed: opens the console and focuses the command line - console open and command line focused: closes the console - console open and command line not focused: focuses the command line Would that work?

(In reply to Panos Astithas [:past] from comment #14) > One idea would be to make the web console shortcut key > (Ctrl-Shift-K/Cmd-Opt-K) have a slightly modified state-dependent action: > > (state: action) > - console closed: opens the console and focuses the command line > - console open and command line focused: closes the console > - console open and command line not focused: focuses the command line > > Would that work? I would love to get such behavior. But for the people who don't understand / don't know about the fact that the console can be open but not have the focus, this is going to be weird ("why doesn't ctrl-shift-k sometimes close the console and sometimes it doesn't?") What if: - console close: ctrl-shift-k: opens the console and focus the command line - console open and command line not focused: ctrl-shift-k: focus the command line - web console focused: ESCAPE: close the console (like the Inspector) If we want to do such a think, we would need to have a special visual style for when the command line is focused.

I'd note that not having the console shortcut be a toggle would be in stark contrast with the rest of our devtools, as well as the competition's. I'm thinking that keeping it a toggle, even if it needs to be pressed twice on occasion is a better tradeoff. But I'm definitely not a UX expert, I just play one on Bugzilla.

(oh noe. more keyboard shortcuts.) Could we use cmd/ctrl-shift-L? That is somewhat consistent with Firebug's binding and is separate from the existing console key command. Unclear what we'd prefer to do if the Console isn't active if a user entered that key command. Maybe open the console? This is getting weird.

KISS! Ctrl-Shift-K: - If console is open, close it. - If console is closed, open+focus it (this behavior already exists) Ctrl-Shift-L: - If console is open, focus it. - If console is closed, open+focus it (just as if Ctrl-Shift-K were pressed) Short, simple, nothing you wouldn't expect (would this be a good first bug? I'd like to get involved with devtools)

(In reply to Corey Richardson (:Octayn) from comment #18) > KISS! > > Ctrl-Shift-K: > - If console is open, close it. > - If console is closed, open+focus it (this behavior already exists) > > Ctrl-Shift-L: > - If console is open, focus it. > - If console is closed, open+focus it (just as if Ctrl-Shift-K were pressed) that's what I'm thinking! We'll have to verify that Cmd/Ctrl-shift-L is available everywhere, but hopefully it is. > Short, simple, nothing you wouldn't expect (would this be a good first bug? > I'd like to get involved with devtools) I think this would be a great first bug as long as you don't mind having a couple of rounds of inevitable shortcut key churning. :) code on!

I also prefer the solution proposed in comment 18. I actually like being able to press Ctrl-Shift-K to close the Web Console quickly when I work with the web page - I use this often. Having to focus the Web Console to press Escape or to press Ctrl-Shift-K is cumbersome.

What is to be done about shortcuts that clash with the shortcuts that extensions (specifically, Firebug) have? Ignore them?

(In reply to Corey Richardson (:Octayn) from comment #21) > What is to be done about shortcuts that clash with the shortcuts that > extensions (specifically, Firebug) have? Ignore them? Yes. Extensions can do whatever they want.

This was also requested quite a few times in the devtools booth today. I asked for feedback on the solution in comment 14 (because this is what GCLI does and it was the only one I could remember) and people were generally in favor.

ok, I have a patch for this, and it's getting complicated. The issue I'm running into is by doing this (the suggestion from comment 14), we are letting the WebConsole control the closing of the toolbox. Once it's selected and focused, if you click away to another tool and then use the key command to return to the console, it's eager to close the toolbox. I could put in some extra checks, but I think I have a simpler solution: Only use cmd-alt-k to raise the console or focus the input line. We have cmd-alt-i to close the toolbox already and I *think* that's what people use. Posting my patch here for preservation.

I would appreciate extra checks over taking away my ability to close toolbox by pressing Ctrl Shift K :( Most of the times, I just have to quickly open the console, see if some error is there and close it back :)

Comment on attachment 8362694 [details] [diff] [review] focusKey Review of attachment 8362694 [details] [diff] [review]: ----------------------------------------------------------------- Thank you for the patch! I used Ctrl-Shift-K to open/close devtools, but I've recently started to use F12. Still, I believe most users have the webconsole shortcut as the way to toggle devtools in Firefox (muscle memory). I believe we should avoid breaking expected behavior. Nonetheless, I wont block on this idea, simply because I also believe we have too many shortcuts. We should have only one for devtools - F12 or whatever else, as long as it's just one, with predictable behavior. (...tempted to write a longer comment arguing for the above...) The patch is a WIP. Anything specific I should provide feedback on? ::: browser/devtools/webconsole/webconsole.js @@ -562,4 @@ > > this.jsterm = new JSTerm(this); > this.jsterm.init(); > - this.jsterm.inputNode.focus(); Which code focuses the input on open?

(In reply to Mihai Sucan [:msucan] from comment #27) > Comment on attachment 8362694 [details] [diff] [review] > focusKey > > Review of attachment 8362694 [details] [diff] [review]: > ----------------------------------------------------------------- > > Thank you for the patch! > > I used Ctrl-Shift-K to open/close devtools, but I've recently started to use > F12. Still, I believe most users have the webconsole shortcut as the way to > toggle devtools in Firefox (muscle memory). I believe we should avoid > breaking expected behavior. Nonetheless, I wont block on this idea, simply > because I also believe we have too many shortcuts. We should have only one > for devtools - F12 or whatever else, as long as it's just one, with > predictable behavior. > > (...tempted to write a longer comment arguing for the above...) > > The patch is a WIP. Anything specific I should provide feedback on? > > ::: browser/devtools/webconsole/webconsole.js > @@ -562,4 @@ > > > > this.jsterm = new JSTerm(this); > > this.jsterm.init(); > > - this.jsterm.inputNode.focus(); > > Which code focuses the input on open? This is now triggered by the focusOrClose() callback registered via the onkey handler in panel.js. A really round-about way of triggering this, but if we go this route we'll have to be careful about adding checks based on focus and set focus in very specific places. I've seen code get ridiculous in an attempt to fix bad focus routines and with async we run the risk of introducing races. Which brings me to... I use cmd-alt-k to open and close the console ALL The Time. I'm not recommending we break that lightly. But I'm not really confident that this approach will work to implement the tri-state (x 2 because of the split-console) logic we'll need to implement to check this without some kind of clever focus model. We've already got a good example of a shortcut that opens a tool and doesn't close it (cmd-alt-c for inspector). I'm suggesting we duplicate that. It's the simplest possible way to fix this and people can use cmd-alt-i (or F12) for closing the toolbox as normal.

WIP. 27 test failures in the webconsole test tree with this applied. Needs work.

Ok, let's bite the bullet, then. I agree that trying to get that tri-state shortcut to work nicely (ctrl-shift-k), will get us into a can of worms - and bugs. How about we use f6 which focuses the address bar? When you are in the devtools, f6 focuses the webconsole input if it's vislble (it doesn't switch to it), otherwise it focuses the address bar. Optional actions: A. when the addressbar has focus, pressing f6 again would switch to the console input. B. when the console input has focus, pressing f6 again could switch to the address bar. I think this could help us avoid breaking memory muscle, and we dont need to mess around with the tri-state shortcut. Thoughts? The more I think of it, the more I like the idea + action B (we dont really need to trouble ourselves with action A).

why do we need an extra key at all? We already have Ctrl/Cmd...K which should do the job nicely. Overriding and interacting with existing browser keys, especially function keys is going to come with its own set of problems.

(In reply to Rob Campbell [:rc] (:robcee) from comment #31) > why do we need an extra key at all? We already have Ctrl/Cmd...K which > should do the job nicely. Overriding and interacting with existing browser > keys, especially function keys is going to come with its own set of problems. Good point, but not entirely convinced. Let's see how the patch for ctrl-shift-k turns out.

needs a test or two...

Comment on attachment 8368105 [details] [diff] [review] focusKey Review of attachment 8368105 [details] [diff] [review]: ----------------------------------------------------------------- Patch looks better, and it's a nice UX - nicer than I expected. I looked into some tests and they seem to fail because the input is not focused when their "console-is-ready" event handlers execute. This patch either fails to focus the input on console open, or it focuses the input after the promises resolve / webconsole-ready|selected events fire. ::: browser/devtools/framework/toolbox.js @@ +821,5 @@ > /** > + * Focus split console's input line > + */ > + focusConsoleInput: function() { > + let hud = HUDService.getOpenWebConsole(); This is a no-no. :) getOpenWebConsole() is as dumb as it can be (I want it removed). Please use this.getPanel("webconsole").hud. (note that getPanel() can return null if the panel is not loaded yet) @@ +841,4 @@ > > if (this._splitConsole) { > this.loadTool("webconsole").then(() => { > + this.focusConsoleInput(); Just reading this makes me say: focusTool("webconsole") should continue to work in such cases. Why do you need this change? ::: browser/devtools/main.js @@ +88,5 @@ > > + preventClosingOnKey: true, > + onkey: function(panel, toolbox) { > + if (toolbox.splitConsole) > + return toolbox.focusConsoleInput(); This function will always warn about not always returning a value. ::: browser/devtools/webconsole/panel.js @@ +35,5 @@ > + focusInput: function WCP_focusInput() > + { > + let inputNode = this.hud.jsterm.inputNode; > + > + if (!inputNode.getAttribute("focused")) Is the check needed? ::: browser/devtools/webconsole/webconsole.js @@ -562,4 @@ > > this.jsterm = new JSTerm(this); > this.jsterm.init(); > - this.jsterm.inputNode.focus(); Does it break things if you keep this here? If yes, how? This should fix some test failures, if you add it back.

this should be ready to go. had to change some tests since the focus semantics have changed slightly.

Comment on attachment 8371757 [details] [diff] [review] focusKey Review of attachment 8371757 [details] [diff] [review]: ----------------------------------------------------------------- Patch looks good. All tests pass. Thanks! r+ with the two comments below addressed: ::: browser/devtools/framework/toolbox.js @@ +822,5 @@ > /** > + * Focus split console's input line > + */ > + focusConsoleInput: function() { > + let hud = HUDService.getOpenWebConsole(); getOpenWebConsole() uses HUDService.currentContext() and direct Firefox tabs. This code breaks with remote targets. getOpenWebConsole() should not be used - it should be removed. Please use this.getPanel("webconsole").hud ::: browser/devtools/webconsole/test/browser_webconsole_input_field_focus_on_panel_select.js @@ +39,5 @@ > > function consoleReopened(hud) > { > + is(hud.jsterm.inputNode.hasAttribute("focused"), false, > + "inputNode should not be focused"); We had a bug about specifically focusing the js input when people switch back from other tools to the console. This patch reverts that. Is there a good reason for this? IIAMN, the problem is caused by the changes in onPanelSelected from webconsole.js.

(In reply to Mihai Sucan [:msucan] from comment #36) > Comment on attachment 8371757 [details] [diff] [review] > focusKey > > Review of attachment 8371757 [details] [diff] [review]: > ----------------------------------------------------------------- > > Patch looks good. All tests pass. Thanks! > > r+ with the two comments below addressed: > > ::: browser/devtools/framework/toolbox.js > @@ +822,5 @@ > > /** > > + * Focus split console's input line > > + */ > > + focusConsoleInput: function() { > > + let hud = HUDService.getOpenWebConsole(); > > getOpenWebConsole() uses HUDService.currentContext() and direct Firefox > tabs. This code breaks with remote targets. getOpenWebConsole() should not > be used - it should be removed. > > Please use this.getPanel("webconsole").hud > > ::: > browser/devtools/webconsole/test/ > browser_webconsole_input_field_focus_on_panel_select.js > @@ +39,5 @@ > > > > function consoleReopened(hud) > > { > > + is(hud.jsterm.inputNode.hasAttribute("focused"), false, > > + "inputNode should not be focused"); > > We had a bug about specifically focusing the js input when people switch > back from other tools to the console. This patch reverts that. Is there a > good reason for this? IIAMN, the problem is caused by the changes in > onPanelSelected from webconsole.js. I interpreted that failure as what happens when we click on a link in the output area and open a panel. We lose focus there because we're specifically overriding the focus() call in that case. I was a little on the fence about that since there isn't much you can do from the keyboard in those net panels (and I still want to get rid of them altogether). The test itself is switching from the debugger back to the console and is kind of artificial. Since it's gotten harder to lose focus on the input area, I think in practice most of the time you'll regain focus on the input line. If you think it's a problem, I can readd a focus() call in onPanelSelected() to cover this case and revert the test change.

going through try while we wait for the trees to reopen... https://tbpl.mozilla.org/?tree=Try&rev=575fdc6d4c26

what was the final expected behavior here?

(In reply to [:tracy] Tracy Walker - QA Mentor from comment #46) > what was the final expected behavior here? The final expected behavior is ctrl-shift-k / cmd-shift-k should: 1. open the developer tools, with the webconsole tab, if the toolbox is not already open. 2. switch to the webconsole tab, if the toolbox is open. 3. focus the webconsole JS input, if the toolbox is open and if the webconsole tab is already selected. 4. if the split console is visible, just focus the JS input.

(In reply to Mihai Sucan [:msucan] from comment #47) > (In reply to [:tracy] Tracy Walker - QA Mentor from comment #46) > > what was the final expected behavior here? > > The final expected behavior is ctrl-shift-k / cmd-shift-k should: > > 1. open the developer tools, with the webconsole tab, if the toolbox is not > already open. > 2. switch to the webconsole tab, if the toolbox is open. > 3. focus the webconsole JS input, if the toolbox is open and if the > webconsole tab is already selected. > 4. if the split console is visible, just focus the JS input. On Mac 10.9 with latest builds it is option + command + k that does the above. (which is also the shortcut listed in the tools menu for Web Console)

(In reply to [:tracy] Tracy Walker - QA Mentor from comment #48) > On Mac 10.9 with latest builds it is option + command + k that does the > above. (which is also the shortcut listed in the tools menu for Web Console) Yes, I forgot it's option on macs. (Linux here...)

From 1005915: using F12 for closing dev tools is fine as it also opens the dev tools. Ctrl-Shift-K still opens the dev tools (similar to F12) which is a little confusing.