892289 - Ctrl+L should clear the console output

Status: RESOLVED FIXED

(DevTools :: Console, defect, P2)

Description

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

In the webconsole, you should be able to quickly clear the console output by pressing Ctrl+L.

Comment 1

[Paul Rouget [:paul]]

ctrl+l on Linux and Windows focuses the url bar.

Comment 2

[Mihai Sucan [:msucan]]

We added ctrl/cmd-k for clear output. Do we really need to take over Ctrl-L?

Comment 3

[Paul Rouget [:paul]]

It's what all terminals do. It's what Chrome does. It's a standard shortcut. Even though I mentioned that it overrides the focus-url-bar shortcut, I think it would be great to support ctrl-l.

Comment 4

[Mihai Sucan [:msucan]]

From: https://etherpad.mozilla.org/hot-hot-hotkeys

Cmd-K clears the console output in Chrome on Macs. On Linux they use Ctrl-L for clear.

When I added Ctrl/Cmd-K to the console to clear output I had to decide about overriding the "focus search" or "focus location bar" shortcut. I decided to override the less common shortcut: the one used for focusing the search field (ctrl-k).

Also Victor, IIANM, pointed out that cmd-k is used for clear in iTerm. Also, for me, as a user, ctrl-k seems pretty common for clear.

Should we change the shortcut from ctrl/cmd-K to ctrl/cmd-L? or should we have cmd-k for mac, ctrl-L for windows and linux? (exactly matching chrome)

Comment 5

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

+1 for Ctrl+L

* It is used by all the terminals I have used (terminal on osx and ubuntu).

* It is the same thing Chrome does. (It does work on osx, I tested yesterday)

Comment 6

[Anton Kovalyov (:anton)]

Attachment: Make ctrl-l clear console

Made ctrl-l to clear console on all platforms. Had to change "log request/response bodies" to ctrl-r because of a conflict.

Once try tree is open, will push to it.

Comment 7

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

Comment on attachment 783901 [details] [diff] [review]
Make ctrl-l clear console

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

::: browser/locales/en-US/chrome/browser/devtools/webConsole.dtd
@@ +68,4 @@
>    -  console.error(). -->
>  <!ENTITY btnPageLogging.label   "Logging">
>  <!ENTITY btnPageLogging.tooltip "Log messages sent to the window.console object">
> +<!ENTITY btnPageLogging.accesskey "R">

need to change the entity name if you're changing values.

change it to btnPageLogging.accesskey2 or something.

@@ +77,4 @@
>  <!ENTITY filterOutput.placeholder "Filter output">
>  <!ENTITY btnClear.label        "Clear">
>  <!ENTITY btnClear.tooltip      "Clear the Web Console output">
> +<!ENTITY btnClear.accesskey    "L">

same here.

Comment 8

[Anton Kovalyov (:anton)]

Attachment: ctrl-l.patch

Oops, keep forgetting to change names.

Comment 9

[Anton Kovalyov (:anton)]

Try: https://tbpl.mozilla.org/?tree=Try&rev=306e24de255f

Comment 10

[Dave Camp (:dcamp)]

Isn't cmd-r Reload?  Is that a good key to steal?

Comment 11

[Dave Camp (:dcamp)]

err, ctrl-r on windows/linux I mean

Comment 12

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

Comment on attachment 783972 [details] [diff] [review]
ctrl-l.patch

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

ok, after reading this bug I became more confused about the state of the controls on this button. I'm concerned about stealing focus from the Location Bar. I'm also confused about whether or not we even want to do this on all platforms. Should we use Ctrl-L on Mac to actually clear the console like Chrome does?

personally, I'm beginning to think cmd/ctrl-K is acceptable.

::: browser/locales/en-US/chrome/browser/devtools/webConsole.dtd
@@ +77,4 @@
>  <!ENTITY filterOutput.placeholder "Filter output">
>  <!ENTITY btnClear.label        "Clear">
>  <!ENTITY btnClear.tooltip      "Clear the Web Console output">
> +<!ENTITY btnClear.accesskey2   "L">

access key on mac will use Ctrl. On Windows, this'll be Alt. Not exactly what we're looking for.

Comment 13

[Dave Camp (:dcamp)]

I think the Ctrl+L thing is fine and mostly agreed on.  My only concern is adding Cmd/Ctrl+R for Log Request/Response Bodies

Comment 14

[Dave Camp (:dcamp)]

And if Ctrl+L is currently being eaten by Log Request Bodies without complaint, I'm really happy to do the expected thing with it instead.

Comment 15

[Anton Kovalyov (:anton)]

Attachment: ctrl-l.patch

IFDEF version.

Comment 16

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

Comment on attachment 784084 [details] [diff] [review]
ctrl-l.patch

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

r+ with a successful test run and extra whitespace removed.

::: browser/devtools/webconsole/webconsole.xul
@@ +42,4 @@
>      <command id="cmd_fullZoomReset" oncommand="goDoCommand('cmd_fontSizeReset');"/>
>      <command id="cmd_close" oncommand="goDoCommand('cmd_close');" disabled="true"/>
>    </commandset>
> +

extra space?

@@ +53,4 @@
>      <key key="&fullZoomResetCmd.commandkey2;" command="cmd_fullZoomReset" modifiers="accel"/>
>      <key key="&findCmd.key;" command="cmd_find" modifiers="accel"/>
>      <key key="&clearOutputCmd.key;" command="consoleCmd_clearOutput" modifiers="accel"/>
> +    <key key="&closeCmd.key;" command="cmd_close" modifiers="accel"/>    

trailing whitespace.

Comment 17

[Anton Kovalyov (:anton)]

Attachment: ctrl-l.patch

Removed whitespace

Comment 18

[Mihai Sucan [:msucan]]

Just looked into the patch. I am fine with the changes. Anton, thanks for the patch!

Comment 19

[Mihai Sucan [:msucan]]

Comment on attachment 784097 [details] [diff] [review]
ctrl-l.patch

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

::: browser/devtools/webconsole/webconsole.xul
@@ +53,4 @@
>      <key key="&findCmd.key;" command="cmd_find" modifiers="accel"/>
>      <key key="&clearOutputCmd.key;" command="consoleCmd_clearOutput" modifiers="accel"/>
>      <key key="&closeCmd.key;" command="cmd_close" modifiers="accel"/>
> +#ifdef XP_MACOSX

Why is ifdef needed here? In bug 877262 we are switching to the jetpack loader and we are turning off preprocessing. Please avoid adding an ifdef here. Thanks!

Comment 20

[Anton Kovalyov (:anton)]

Is there another way to change modifiers (control vs accel) based on the OS?

Comment 21

[Mihai Sucan [:msucan]]

Yes. In webconsole.js when the UI is initialized, change the modifier for that key element. Do this for Services.appinfo.OS == "Darwin".

Comment 22

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

can this land? We have a patch and an R+.

Comment 23

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

ping for landing!

Comment 24

[Anton Kovalyov (:anton)]

I need to move away from #ifdef before landing (see comment 19).

Comment 25

[Anton Kovalyov (:anton)]

Attachment: ctrl-l.patch

A version without #IFDEF.

Comment 26

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

Comment on attachment 789244 [details] [diff] [review]
ctrl-l.patch

OK.

Comment 27

[Anton Kovalyov (:anton)]

https://hg.mozilla.org/integration/fx-team/rev/412de0630f87

Comment 28

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/412de0630f87

Comment 29

[Mihai Sucan [:msucan]]

Comment on attachment 789244 [details] [diff] [review]
ctrl-l.patch

Thanks for the updated patch.

1. why do you remove the accesskey for the Clear button?
2. how was the "R" accesskey picked for the Logging filter button? why not one of the existing letters in the Logging word?
3. why do you keep accel+k for clear output? See <key key="&clearOutputCmd.key;"> I expected we switch from accel+k to accel+L in this bug. It seems now we've added accel+L without removing accel+k.

Comment 30

[Anton Kovalyov (:anton)]

(In reply to Mihai Sucan [:msucan] from comment #29)
> Comment on attachment 789244 [details] [diff] [review]
> ctrl-l.patch
> 
> Thanks for the updated patch.
> 
> 1. why do you remove the accesskey for the Clear button?
> 2. how was the "R" accesskey picked for the Logging filter button? why not
> one of the existing letters in the Logging word?

Everything else was taken so I took R from clear button and re-used it for logging. We have two shortcuts for Clear now.

> 3. why do you keep accel+k for clear output? See <key
> key="&clearOutputCmd.key;"> I expected we switch from accel+k to accel+L in
> this bug. It seems now we've added accel+L without removing accel+k.

To be as close as possible to Chrome shortcuts. It supports both accel+L and accel+K because Mac apps usually support both of them.

Comment 31

[Girish Sharma [:Optimizer]]

(In reply to Anton Kovalyov (:anton) from comment #30)
> (In reply to Mihai Sucan [:msucan] from comment #29)
> > Comment on attachment 789244 [details] [diff] [review]
> > ctrl-l.patch
> > 
> > Thanks for the updated patch.
> > 
> > 1. why do you remove the accesskey for the Clear button?
> > 2. how was the "R" accesskey picked for the Logging filter button? why not
> > one of the existing letters in the Logging word?
> 
> Everything else was taken so I took R from clear button and re-used it for
> logging. We have two shortcuts for Clear now.

On Windows here, all the letters of the word "Logging" are free. Moreover, we can even keep it as is as the shortcuts use Ctrl as modifier while accesskeys use Alt. (Same for Linux I suppose) (Mac might have things differently)

Comment 32

[Mihai Sucan [:msucan]]

(In reply to Anton Kovalyov (:anton) from comment #30)
> Everything else was taken so I took R from clear button and re-used it for
> logging. We have two shortcuts for Clear now.

I see, but command keys are different from accesskeys, and IIANM users need accesskeys as well.


> > 3. why do you keep accel+k for clear output? See <key
> > key="&clearOutputCmd.key;"> I expected we switch from accel+k to accel+L in
> > this bug. It seems now we've added accel+L without removing accel+k.
> 
> To be as close as possible to Chrome shortcuts. It supports both accel+L and
> accel+K because Mac apps usually support both of them.

I'm not entirely happy with having both accel-K and accel+L for console clear output. This might show we are undecided. I am not sure having both shortcuts for the same command is useful/needed, given that they both take over important browser UI shortcuts for fields (accel+L for location bar, accel+K for search field), especially in the situation of having very few available shortcuts.

Chrome on Linux only allows Ctrl+L for clear output, Ctrl+K focuses the location bar and puts you in search mode. Does Chrome on Mac/Windows clear output for Cmd+K *and* Cmd+L?

Comment 33

[Girish Sharma [:Optimizer]]

(In reply to Mihai Sucan [:msucan] from comment #32)
> Chrome on Linux only allows Ctrl+L for clear output, Ctrl+K focuses the
> location bar and puts you in search mode. Does Chrome on Mac/Windows clear
> output for Cmd+K *and* Cmd+L?

Same for Windows.

Comment 34

[Girish Sharma [:Optimizer]]

I just checked on Mac, Ctrl + L is free and still can be used as the accesskey for Logging. Cmd + L will still work to clear Console.

Apart from having the same key, I do not see any issues with letting remain the "L" character for the accesskey.

Comment 35

[Dão Gottwald [:dao]]

(In reply to Mihai Sucan [:msucan] from comment #32)
> (In reply to Anton Kovalyov (:anton) from comment #30)
> > Everything else was taken so I took R from clear button and re-used it for
> > logging. We have two shortcuts for Clear now.
> 
> I see, but command keys are different from accesskeys, and IIANM users need
> accesskeys as well.

Can this please be addressed? Access keys and command keys are separate things, adding one doesn't mean you can remove the other.

Comment 36

[Anton Kovalyov (:anton)]

Filed bug 912240. Will do this week.