960695 - Focus Input line when clicking anywhere in the console

Status: RESOLVED FIXED

(DevTools :: Console, defect)

Description

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

Attachment: focus

Shouldn't have to click inside the textbox to focus the console

Comment 1

[Mihai Sucan [:msucan]]

Comment on attachment 8361260 [details] [diff] [review]
focus

LGTM. Please check the event target, so it doesn't change the focus when you click on anchors. Thanks!

Comment 2

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

(In reply to Mihai Sucan [:msucan] from comment #1)
> Comment on attachment 8361260 [details] [diff] [review]
> focus
> 
> LGTM. Please check the event target, so it doesn't change the focus when you
> click on anchors. Thanks!

good idea. Thank you!

Comment 3

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

Attachment: focus

Added the check for anchors. Not 100% happy with it but probably good enough. The first check for links appears to only really matter for network requests which open the network popup. I'd consider removing that check altogether and focusing the input line there too, but ...

Also added a test.

Comment 4

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

what did my editor do to the indenting in that test file? I don't even...

Comment 5

[Mihai Sucan [:msucan]]

Comment on attachment 8362543 [details] [diff] [review]
focus

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

Patch looks good. r+ with the following fixes:

Got an exception playing around:

System JS : ERROR resource://gre/modules/commonjs/toolkit/loader.js -> file:///home/mihai/src/mozilla/fx-team/browser/devtools//webconsole/webconsole.js:578 - TypeError: e.target.tagName is undefined

tagName is not available for all node types.


Also please fix indentation. :) Thank you!

::: browser/devtools/webconsole/test/browser_console_click_focus.js
@@ +14,5 @@
> +
> +function testInputFocus() {
> +  browser.removeEventListener("DOMContentLoaded", testInputFocus, false);
> +
> +  openConsole(null, function(hud) {

nit: openConsole().then(...)

(callback is "deprecated")

::: browser/devtools/webconsole/webconsole.js
@@ +574,5 @@
> +     * Only focus when the target node (or parent, as in source links) is
> +     * not an anchor.
> +     */
> +    this.outputNode.addEventListener("click", (e) => {
> +      if ((e.target.tagName.toLowerCase() != "a") &&

Does it focus the input on any button click? Maybe you should check that e.button == 0 (left mouse button).

Comment 6

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

Attachment: focus

fixed up. Thanks for the review!

Comment 7

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

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

Comment 8

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8363180 [details] [diff] [review]
focus

Please just use checkin-needed :)

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/f5fb918f295f

Comment 10

[Girish Sharma [:Optimizer]]

Filed bug 964268 because single line text selection was not working anymore.