Closed
Bug 1211038
Opened 9 years ago
Closed 9 years ago
Cmd-F does not focus the debugger search input
Categories
(DevTools :: Debugger, defect)
DevTools
Debugger
Tracking
(firefox43 unaffected, firefox44 verified)
VERIFIED
FIXED
Firefox 44
Tracking | Status | |
---|---|---|
firefox43 | --- | unaffected |
firefox44 | --- | verified |
People
(Reporter: jryans, Assigned: bgrins)
References
Details
(Keywords: regression, Whiteboard: [polish-backlog][bugday-20160111])
Attachments
(1 file)
STR:
1. Open the debugger
2. Click in the source editor (where script source is shown) so it is focused
3. Press Cmd-F (on Mac, or the appropriate shortcut for your OS) to search the current file
ER:
A "#" character is added to the search input and the input is focused so you can type there immediately.
AR:
A "#" character is still added to the input, but it is not focused, so you have to click into the input manually.
I've seen some CodeMirror bugs fly by lately, possibly related. I think it's a recent regression.
Assignee | ||
Updated•9 years ago
|
Flags: needinfo?(bgrinstead)
Reporter | ||
Comment 2•9 years ago
|
||
Additional data from bug 1212178:
"With the source focussed, pressing Ctrl+F doesn't focus the search box. It does add # though.
Ctrl+Alt+F, Ctrl+D also don't, but Ctrl+L, Ctrl+Alt+V and Ctrl+P do."
Comment 3•9 years ago
|
||
Regression window:
https://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=21015383ac41061eab25d01b546cd2713ff17b36&tochange=2bfe5435c1d1
Regressed by: Bug 1132557
Blocks: 1132557
Updated•9 years ago
|
Keywords: regressionwindow-wanted
Reporter | ||
Updated•9 years ago
|
Whiteboard: [polish-backlog]
Reporter | ||
Updated•9 years ago
|
status-firefox43:
--- → unaffected
status-firefox44:
--- → affected
Comment 4•9 years ago
|
||
I can confirm: no focus, does add the #.
Reporter | ||
Comment 5•9 years ago
|
||
Here is the sequence of events that happens in CM 5.7 when you press a key:
1. CM's `onKeyDown` is called
2. This method does the following early on:
cm.curOp.focus = activeElt();
3. It calls into `handleKeyBinding`
4. For Cmd-F, `handleKeyBinding` eventually triggers the debugger to focus its custom search field via a function the debugger set on this shortcut in CM's `extraKeys`
5. After `onKeyDown` returns, `endOperation` is called via the `operation` function that wraps this event handler
6. Eventually we arrive at `endOperation_W2`, which contains:
if (op.focus && op.focus == activeElt()) ensureFocus(op.cm);
7. Since this is true because of step 2, `ensureFocus` pulls the focus back into the editor.
Looking back at the (quite old) version of CM we used to be on (4.2), it does not seem like there was a code path that would grab focus **after** a key handler runs.
Marijn, is there an option or different approach we can take to avoid CM focusing back to itself like this?
Flags: needinfo?(marijnh)
Comment 6•9 years ago
|
||
If I follow the logic then the expression `op.focus && op.focus == activeElt()` will be false if this happens, and the editor _won't_ refocus itself. If I set up a text case where I bind a key using CodeMirror's keymap mechanism to focus an external textarea, things work as hoped -- when I press they key, the textarea gains focus, and the editor loses it.
Could it be that your focused element isn't part of the same document? That might complicate things, though my understanding is that in that case, focusing CodeMirror in the no-longer-focused document won't cause the element in the focused document to lose focus.
Flags: needinfo?(marijnh)
Assignee | ||
Comment 7•9 years ago
|
||
(In reply to Marijn Haverbeke from comment #6)
> If I follow the logic then the expression `op.focus && op.focus ==
> activeElt()` will be false if this happens, and the editor _won't_ refocus
> itself. If I set up a text case where I bind a key using CodeMirror's keymap
> mechanism to focus an external textarea, things work as hoped -- when I
> press they key, the textarea gains focus, and the editor loses it.
>
> Could it be that your focused element isn't part of the same document? That
> might complicate things, though my understanding is that in that case,
> focusing CodeMirror in the no-longer-focused document won't cause the
> element in the focused document to lose focus.
Yes, they are in different documents. And focusing CM in the separate document does take focus back away from the currently selected document.
Comment 8•9 years ago
|
||
Does this patch work for you?
```
diff --git a/lib/codemirror.js b/lib/codemirror.js
index ae16a87..d914807 100644
--- a/lib/codemirror.js
+++ b/lib/codemirror.js
@@ -3093,7 +3093,8 @@
if (cm.state.focused && op.updateInput)
cm.display.input.reset(op.typing);
- if (op.focus && op.focus == activeElt()) ensureFocus(op.cm);
+ if (op.focus && op.focus == activeElt() && (!document.hasFocus || document.hasFocus()))
+ ensureFocus(op.cm);
}
function endOperation_finish(op) {
```
Assignee | ||
Comment 9•9 years ago
|
||
(In reply to Marijn Haverbeke from comment #8)
> Does this patch work for you?
>
> ```
> diff --git a/lib/codemirror.js b/lib/codemirror.js
> index ae16a87..d914807 100644
> --- a/lib/codemirror.js
> +++ b/lib/codemirror.js
> @@ -3093,7 +3093,8 @@
>
> if (cm.state.focused && op.updateInput)
> cm.display.input.reset(op.typing);
> - if (op.focus && op.focus == activeElt()) ensureFocus(op.cm);
> + if (op.focus && op.focus == activeElt() && (!document.hasFocus ||
> document.hasFocus()))
> + ensureFocus(op.cm);
> }
>
> function endOperation_finish(op) {
> ```
Yes, that seems to do the trick!
Comment 10•9 years ago
|
||
Great -- I've merged it into CodeMirror's master branch
Assignee | ||
Comment 11•9 years ago
|
||
Bug 1211038 - Fix focus issue with debugger searchbox using CodeMirror upstream patch;r=jryans
Attachment #8679521 -
Flags: review?(jryans)
Reporter | ||
Comment 12•9 years ago
|
||
Comment on attachment 8679521 [details]
MozReview Request: Bug 1211038 - Fix focus issue with debugger searchbox using CodeMirror upstream patch;r=jryans
https://reviewboard.mozilla.org/r/23457/#review20911
Hooray, it works great!
Attachment #8679521 -
Flags: review?(jryans) → review+
Reporter | ||
Updated•9 years ago
|
Assignee: nobody → bgrinstead
Assignee | ||
Comment 13•9 years ago
|
||
Looks like the infra for osx blew up on this try push, but linux is looking good so far: https://treeherder.mozilla.org/#/jobs?repo=try&revision=57334000da1c
Assignee | ||
Updated•9 years ago
|
Flags: needinfo?(bgrinstead)
Comment 15•9 years ago
|
||
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 44
Comment 16•9 years ago
|
||
Reproduced this bug by following comment 0 with Firefox Nightly 44.0a1 (2015-10-02);
(Build ID: 20151002030204) on Linux, 64 Bit
This Bug is now verified as fixed on Latest Firefox Dev Edition 44.0a2 (2015-12-03)
Build ID: 20151203004003
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:44.0) Gecko/20100101 Firefox/44.0
QA Whiteboard: [bugday-20151202]
Comment 17•9 years ago
|
||
I have reproduced this bug by following comment 0 with Firefox Nightly 44.0a1
(Build ID: 20151002030204; User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:44.0) Gecko/20100101 Firefox/44.0)
The bug is now verified as fixed on latest Beta 44.0b7
Build ID: 20160107144911;
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:44.0) Gecko/20100101 Firefox/44.0
[testday-20160108]
Comment 18•9 years ago
|
||
As this bug is verified on both Linux (comment 16) and Windows (Comment 17), I am marking this as verified!
Status: RESOLVED → VERIFIED
Whiteboard: [polish-backlog] → [polish-backlog][bugday-20160111]
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•