Closed
Bug 1050439
Opened 9 years ago
Closed 9 years ago
Focus should be restored to previously active element after split console is closed
Categories
(DevTools :: Framework, defect)
DevTools
Framework
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 34
People
(Reporter: bgrins, Assigned: bgrins)
References
Details
Attachments
(1 file, 2 obsolete files)
10.20 KB,
patch
|
jwalker
:
review+
|
Details | Diff | Splinter Review |
If you have a text box focused, then ESC to open split console, then ESC to close split console then the text box should be refocused for convenience. Same with the debugger/style editor source editor elements once Bug 1043154 lands.
Assignee | ||
Comment 1•9 years ago
|
||
Joe, what do you think about this approach? The idea is that we want to restore the active element once the split console is closed (either via ESC or via clicking the split console button). The way I'm doing it here is tracking the focus event when the split console is opened and storing a reference to a node only if it is not a child of the console panel itself. I'm using document.activeElement to store the original element then using the target for the focus event for subsequent focused elements. There may be some better way that I'm not thinking of to handle this. There are probably cases where something funny could happen, like if I focus on a node in the markup view, then enter $0.remove() in the console and close split console. In this case the focused element is not a child of the document anymore, but it doesn't throw an exception or anything, I think the call to .focus() just doesn't do anything.
Comment 2•9 years ago
|
||
Comment on attachment 8469591 [details] [diff] [review] restore-split-console-focus-WIP.patch Review of attachment 8469591 [details] [diff] [review]: ----------------------------------------------------------------- What happens to the focus if we start with the focus in the style editor, press escape to open the split console, then select the debugger then press escape again to close the split console? ::: browser/devtools/framework/toolbox.js @@ +1010,5 @@ > + // If the console is split and we are focusing an element outside > + // of the console, then store the newly focused element, so that it > + // can be restored once the split console closes. > + // For instance -> open split console, select a text box, focus console > + // again, then close the console. The text box should be refocused. Maybe this should be a doc comment? /** ... */ @@ +1019,5 @@ > + originalTarget.ownerDocument.defaultView; > + > + // Only track elements if it doesn't belong to the web console > + if (webconsolePanel._frameWindow != activeElementWindow) { > + this._savedActiveElement = originalTarget; Do we need to start this function with: this._savedActiveElement = null; So we don't end up going back to where we were some time ago if we don't end up storing an _savedActiveElement this time? @@ +1050,5 @@ > Services.prefs.setBoolPref(SPLITCONSOLE_ENABLED_PREF, false); > this._refreshConsoleDisplay(); > this.emit("split-console"); > + > + console.log("Console has been closed... restoring focus", this._savedActiveElement); Can probably remove this
Attachment #8469591 -
Flags: feedback?(jwalker) → feedback+
Assignee | ||
Comment 3•9 years ago
|
||
(In reply to Joe Walker [:jwalker] from comment #2) > Comment on attachment 8469591 [details] [diff] [review] > restore-split-console-focus-WIP.patch > > Review of attachment 8469591 [details] [diff] [review]: > ----------------------------------------------------------------- > > What happens to the focus if we start with the focus in the style editor, > press escape to open the split console, then select the debugger then press > escape again to close the split console? This is tricky to do without focusing something in the process, but I've been able to do it by using cmd+] to switch panels. Nothing really happens.. it calls focus on the hidden element, which doesn't seem to do anything. > > @@ +1019,5 @@ > > + originalTarget.ownerDocument.defaultView; > > + > > + // Only track elements if it doesn't belong to the web console > > + if (webconsolePanel._frameWindow != activeElementWindow) { > > + this._savedActiveElement = originalTarget; > > Do we need to start this function with: > > this._savedActiveElement = null; > > So we don't end up going back to where we were some time ago if we don't end > up storing an _savedActiveElement this time? Yes, good idea.
Assignee | ||
Comment 4•9 years ago
|
||
Attachment #8469591 -
Attachment is obsolete: true
Attachment #8473428 -
Flags: review?(jwalker)
Assignee | ||
Comment 5•9 years ago
|
||
Attachment #8473428 -
Attachment is obsolete: true
Attachment #8473428 -
Flags: review?(jwalker)
Attachment #8473430 -
Flags: review?(jwalker)
Updated•9 years ago
|
Attachment #8473430 -
Flags: review?(jwalker) → review+
Assignee | ||
Comment 6•9 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=d477341bce2a
Keywords: checkin-needed
Assignee | ||
Comment 7•9 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/b4ea3496213f
Keywords: checkin-needed
Whiteboard: [fixed-in-fx-team]
Comment 8•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/b4ea3496213f
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 34
Updated•5 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•