Closed Bug 960695 Opened 7 years ago Closed 7 years ago

Focus Input line when clicking anywhere in the console

Categories

(DevTools :: Console, defect)

x86
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 29

People

(Reporter: rcampbell, Assigned: rcampbell)

References

Details

Attachments

(1 file, 2 obsolete files)

Attached patch focus (obsolete) — Splinter Review
Shouldn't have to click inside the textbox to focus the console
Attachment #8361260 - Flags: feedback?(mihai.sucan)
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!
Attachment #8361260 - Flags: feedback?(mihai.sucan) → feedback+
(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!
Assignee: nobody → rcampbell
Status: NEW → ASSIGNED
Attached patch focus (obsolete) — Splinter Review
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.
Attachment #8361260 - Attachment is obsolete: true
Attachment #8362543 - Flags: review?(mihai.sucan)
what did my editor do to the indenting in that test file? I don't even...
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).
Attachment #8362543 - Flags: review?(mihai.sucan) → review+
Attached patch focusSplinter Review
fixed up. Thanks for the review!
Attachment #8362543 - Attachment is obsolete: true
Attachment #8363180 - Flags: review+
Attachment #8363180 - Flags: checkin?
Whiteboard: [land-in-fx-team]
https://hg.mozilla.org/integration/fx-team/rev/f5fb918f295f
Flags: in-testsuite+
Whiteboard: [land-in-fx-team] → [fixed-in-fx-team]
Comment on attachment 8363180 [details] [diff] [review]
focus

Please just use checkin-needed :)
Attachment #8363180 - Flags: checkin?
https://hg.mozilla.org/mozilla-central/rev/f5fb918f295f
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 29
Filed bug 964268 because single line text selection was not working anymore.
Blocks: 962531
Blocks: 612253
Depends on: 965042
Depends on: 1239992
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.