Focus Input line when clicking anywhere in the console

RESOLVED FIXED in Firefox 29

Status

defect
RESOLVED FIXED
6 years ago
Last year

People

(Reporter: rcampbell, Assigned: rcampbell)

Tracking

unspecified
Firefox 29
x86
macOS
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Assignee

Description

6 years ago
Posted 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+
Assignee

Comment 2

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

Comment 3

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

Comment 4

6 years ago
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+
Assignee

Comment 6

6 years ago
Posted patch focusSplinter Review
fixed up. Thanks for the review!
Attachment #8362543 - Attachment is obsolete: true
Attachment #8363180 - Flags: review+
Attachment #8363180 - Flags: checkin?
Assignee

Updated

6 years ago
Whiteboard: [land-in-fx-team]
Assignee

Comment 7

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

Updated

6 years ago
Blocks: 962531
Assignee

Updated

6 years ago
Blocks: 612253

Updated

6 years ago
Depends on: 965042

Updated

3 years ago
Depends on: 1239992

Updated

Last year
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.