Need a shortcut key to focus the input line in web console

VERIFIED FIXED in Firefox 30

Status

()

Firefox
Developer Tools: Console
P2
normal
VERIFIED FIXED
7 years ago
2 months ago

People

(Reporter: rc, Assigned: rc)

Tracking

unspecified
Firefox 30
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [strings] [console-1])

Attachments

(1 attachment, 4 obsolete attachments)

(Assignee)

Description

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

Updated

7 years ago
Blocks: 593956
(Assignee)

Comment 1

7 years ago
ctrl/cmd-shift-L ?
Input from UX team wanted here.
Keywords: uiwanted

Updated

7 years ago
Assignee: nobody → pwalton
Per a discussion with Kevin I'm going to propose ⌘⇧I (for "Insert"?)
I believe this breaks string freeze, so marking as [strings].
Whiteboard: [strings]

Comment 5

7 years ago
I proposed cmd/ctrl-shift-I for "input" :)

how does this break string freeze? it needs to appear on a menu somewhere?

Comment 6

7 years ago
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.
(Assignee)

Comment 8

7 years ago
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.

Comment 11

7 years ago
mass change: filter on PRIORITYSETTING
Priority: -- → P2

Updated

7 years ago
Whiteboard: [strings] → [strings] [console-1]

Updated

7 years ago
Assignee: pwalton → nobody
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?
Keywords: uiwanted

Comment 13

6 years ago
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?

Comment 15

6 years ago
(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.
(Assignee)

Comment 17

6 years ago
(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.
No longer blocks: 593956
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)
(Assignee)

Comment 19

6 years ago
(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?
(Assignee)

Comment 22

6 years ago
(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.

Updated

6 years ago
Component: Developer Tools → Developer Tools: Console
QA Contact: developer.tools → developer.tools.console
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.
(Assignee)

Comment 24

4 years ago
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.
Assignee: nobody → rcampbell
Status: NEW → ASSIGNED
(Assignee)

Comment 25

4 years ago
Created attachment 8362694 [details] [diff] [review]
focusKey
Attachment #8362694 - Flags: feedback?(mihai.sucan)
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?
(Assignee)

Comment 28

4 years ago
(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.
(Assignee)

Comment 29

4 years ago
Created attachment 8363253 [details] [diff] [review]
focusKey

WIP. 27 test failures in the webconsole test tree with this applied. Needs work.
Attachment #8362694 - Attachment is obsolete: true
Attachment #8362694 - Flags: feedback?(mihai.sucan)
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).
Flags: needinfo?(rcampbell)
(Assignee)

Comment 31

4 years ago
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.
Flags: needinfo?(rcampbell)
(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.
(Assignee)

Updated

4 years ago
Depends on: 960695
(Assignee)

Updated

4 years ago
Depends on: 964268
(Assignee)

Comment 33

4 years ago
Created attachment 8368105 [details] [diff] [review]
focusKey

needs a test or two...
Attachment #8363253 - Attachment is obsolete: true
Attachment #8368105 - Flags: review?(mihai.sucan)
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.
Attachment #8368105 - Flags: review?(mihai.sucan)
(Assignee)

Comment 35

4 years ago
Created attachment 8371757 [details] [diff] [review]
focusKey

this should be ready to go.

had to change some tests since the focus semantics have changed slightly.
Attachment #8368105 - Attachment is obsolete: true
Attachment #8371757 - Flags: review?(mihai.sucan)
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.
Attachment #8371757 - Flags: review?(mihai.sucan) → review+
(Assignee)

Comment 37

4 years ago
(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.
(Assignee)

Comment 38

4 years ago
Created attachment 8372360 [details] [diff] [review]
focusKey
Attachment #8371757 - Attachment is obsolete: true
Attachment #8372360 - Flags: review+
(Assignee)

Comment 39

4 years ago
going through try while we wait for the trees to reopen...

https://tbpl.mozilla.org/?tree=Try&rev=575fdc6d4c26
(Assignee)

Comment 40

4 years ago
https://hg.mozilla.org/integration/fx-team/rev/eae9d7db19ca
(Assignee)

Updated

4 years ago
Whiteboard: [strings] [console-1] → [strings] [console-1][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/eae9d7db19ca
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Whiteboard: [strings] [console-1][fixed-in-fx-team] → [strings] [console-1]
Target Milestone: --- → Firefox 30
Duplicate of this bug: 970269

Updated

4 years ago
Duplicate of this bug: 970394

Updated

4 years ago
Duplicate of this bug: 973056

Updated

4 years ago
Duplicate of this bug: 978968
what was the final expected behavior here?
Keywords: verifyme
(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...)

Updated

3 years ago
Status: RESOLVED → VERIFIED
Keywords: verifyme
Depends on: 988506
Duplicate of this bug: 1005915

Comment 51

3 years ago
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.

Updated

3 years ago
Depends on: 1041920

Updated

2 months ago
Duplicate of this bug: 1372700
You need to log in before you can comment on or make changes to this bug.