Closed Bug 865871 Opened 7 years ago Closed 7 years ago

Object inspector can't be closed

Categories

(DevTools :: Console, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 23

People

(Reporter: ianbicking, Assigned: msucan)

References

Details

Attachments

(2 files, 1 obsolete file)

Attached image Screenshot of inspector
As far as I can see, there's no way to close the object inspector in the console.  Once you've opened the inspector, it just sits there until you close and reopen the web console.  It can hide functionality as shown in the screenshot.
Summary: Object inspect can't be closed → Object inspector can't be closed
I would dupe this to bug 865788, but I'm not sure if we want one bug per tool or not. Leaving it as it is until someone who knows whether we can define the keybinding in VariablesView or not can comment.
See Also: → 865788
(In reply to Panos Astithas [:past] from comment #1)
> I would dupe this to bug 865788, but I'm not sure if we want one bug per
> tool or not. Leaving it as it is until someone who knows whether we can
> define the keybinding in VariablesView or not can comment.

Quick note: the VariablesView has no idea about it's owner, nor how it's contained, so I don't think it should know how to close itself, since this responsibility is different for each tool.

When it comes to keybindings, there's no way of defining custom actions for a VariablesView at this point. It'd make for a nifty API, so I suggest opening a bug about it.
Thanks for the bug report. This issue is known, see bug 843280. However, that one is about a generic way to collapse sidebars.

The ESC idea is fine and we can do this in the web console.
Assignee: nobody → mihai.sucan
Status: NEW → ASSIGNED
Attached patch proposed patch (obsolete) — Splinter Review
Added an event handler for keypress that listens for the Escape key to close the sidebar, and a test that checks this works as expected. I also cleaned-up small bits of code.

Please let me know if you have any concerns. Thanks!

Try push: https://tbpl.mozilla.org/?tree=Try&rev=0653265b236b
Attachment #744220 - Flags: review?(past)
Comment on attachment 744220 [details] [diff] [review]
proposed patch

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

It's an improvement, to be sure, but I think we could do better than this. I'm not going to block you if you disagree though.

::: browser/devtools/webconsole/webconsole.js
@@ +3120,5 @@
> +  {
> +    let tag = aEvent.target.nodeName;
> +    if (aEvent.keyCode != Ci.nsIDOMKeyEvent.DOM_VK_ESCAPE || aEvent.shiftKey ||
> +        aEvent.altKey || aEvent.ctrlKey || aEvent.metaKey ||
> +        ["input", "textarea", "select", "textbox"].indexOf(tag) > -1) {

This seems to be too strict in practice. The 2 most common cases for me are:
1. inspect(foo), glance at the sidebar, press ESC
2. console.log(foo), click on [object Foo], glance at the sidebar, press ESC
In neither case the sidebar is hidden, because the focus is on one of these ignored elements.

If the reason for this limit is the case of pressing ESC to dismiss the autocomplete menu, then maybe handling that case explicitly would be better?
Attachment #744220 - Flags: review?(past) → review+
(In reply to Panos Astithas [:past] from comment #5)
> Comment on attachment 744220 [details] [diff] [review]
> proposed patch
> 
> Review of attachment 744220 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> It's an improvement, to be sure, but I think we could do better than this.
> I'm not going to block you if you disagree though.
> 
> ::: browser/devtools/webconsole/webconsole.js
> @@ +3120,5 @@
> > +  {
> > +    let tag = aEvent.target.nodeName;
> > +    if (aEvent.keyCode != Ci.nsIDOMKeyEvent.DOM_VK_ESCAPE || aEvent.shiftKey ||
> > +        aEvent.altKey || aEvent.ctrlKey || aEvent.metaKey ||
> > +        ["input", "textarea", "select", "textbox"].indexOf(tag) > -1) {
> 
> This seems to be too strict in practice. The 2 most common cases for me are:
> 1. inspect(foo), glance at the sidebar, press ESC
> 2. console.log(foo), click on [object Foo], glance at the sidebar, press ESC
> In neither case the sidebar is hidden, because the focus is on one of these
> ignored elements.
> 
> If the reason for this limit is the case of pressing ESC to dismiss the
> autocomplete menu, then maybe handling that case explicitly would be better?

Good idea. I didn't know if it yesterday. I have updated the patch accordingly.

After I submitted the patch, yesterday, I had the idea of autofocusing vview when you click objects (only then!), also to put focus back in jsterm inputNode after you press Escape in vview. Together with your idea, I think the updated patch nicely improves usability.

Thanks!

Landed:

https://hg.mozilla.org/integration/fx-team/rev/4d4f26313ab2
Whiteboard: [fixed-in-fx-team]
Attached patch updated patchSplinter Review
Attachment #744220 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/4d4f26313ab2
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 23
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.