Bug 111193

Summary: Web Inspector: Keyboard shortcut for "Inspect Element" only works when Web Inspector is open.
Product: WebKit Reporter: Nick Sergeant <nick>
Component: Web InspectorAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: bburg, chrisjshull, joepeck, pmuellr, timothy, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
[PATCH] Proposed Fix
timothy: review+
[PATCH] Improved Version timothy: review+

Description Nick Sergeant 2013-03-01 10:38:07 PST
Chrome's "Inspect Element" keyboard shortcut will bring up the DOM view with Style sidebar open even if the web inspector is closed (it will open the inspector, first).

WebKit's keyboard shortcut for this only works if the Web Inspector is already open.
Comment 1 Radar WebKit Bug Importer 2013-03-01 10:57:56 PST
<rdar://problem/13325889>
Comment 2 Nick Sergeant 2015-12-01 13:40:59 PST
Why was this marked as invalid?
Comment 3 BJ Burg 2015-12-01 15:15:52 PST
It's marked INVALID because it's not a WebKit bug.

This is something the user agent (i.e., Safari) would have to implement. WebKit has no control over browser-level keybindings, as they are separate from the Inspector's window. Feel free to file a bug report at bugreport.apple.com.

I'm a bit puzzled, though. What's the point of using this shortcut (Cmd-Shift-C) when the web content is focused instead of the inspector? You can just right click "Inspect Element" anywhere, and then select a node using the tree or the existing shortcut. Is it just for the element highlighting?
Comment 4 Nick Sergeant 2015-12-01 21:12:24 PST
> This is something the user agent (i.e., Safari) would have to implement. WebKit has no control over browser-level keybindings, as they are separate from the Inspector's window. Feel free to file a bug report at bugreport.apple.com.

Ah, okay.

> I'm a bit puzzled, though. What's the point of using this shortcut (Cmd-Shift-C) when the web content is focused instead of the inspector? You can just right click "Inspect Element" anywhere, and then select a node using the tree or the existing shortcut. Is it just for the element highlighting?

As a developer I spend a bunch of time going from "looking at a page" to "I need to inspect this element". With Chrome, I can hit the keyboard shortcut, and it'll immediately invoke the "Inspect Element" function and bring up the inspector. Yes, I could use my mouse, but I'm a keyboard-oriented developer and I know many others are as well.
Comment 5 BJ Burg 2015-12-02 08:46:18 PST
(In reply to comment #4)
> 
> As a developer I spend a bunch of time going from "looking at a page" to "I
> need to inspect this element". With Chrome, I can hit the keyboard shortcut,
> and it'll immediately invoke the "Inspect Element" function and bring up the
> inspector. Yes, I could use my mouse, but I'm a keyboard-oriented developer
> and I know many others are as well.

Ah, I see. There are plenty of small bugs that make keyboard navigation difficult in the WebKit Web Inspector. The issues are often related to accessibility problems as well. If you are interested in helping to report or fix some of these issues, I'd be happy to point you in the right direction. Sorry that this one is out of our scope though.
Comment 6 Nick Sergeant 2015-12-02 11:13:29 PST
Thanks, Brian! Unfortunately I no longer use Safari for development so I probably wouldn't be able to help out much, but if I ever do switch back, I will definitely consider helping out.
Comment 7 BJ Burg 2015-12-02 11:42:06 PST
(In reply to comment #1)
> <rdar://problem/13325889>

This report has been forwarded to the appropriate team. You don't have to do anything.
Comment 8 Timothy Hatcher 2016-03-29 10:28:33 PDT
This will likely need WebKit changes too, so reopening this bug.
Comment 9 Joseph Pecoraro 2016-04-11 21:34:38 PDT
Created attachment 276215 [details]
[PATCH] Proposed Fix
Comment 10 Joseph Pecoraro 2016-04-11 21:35:43 PDT
I couldn't think of a better name then "PointToInspect". The UI can show something different. We also call this "SearchingForNode" in some cases.
Comment 11 Timothy Hatcher 2016-04-12 10:02:59 PDT
Comment on attachment 276215 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=276215&action=review

> Source/WebKit2/WebProcess/WebPage/WebInspector.cpp:226
> +    m_page->corePage()->inspectorController().show();

Why show in the disable case?
Comment 12 Joseph Pecoraro 2016-04-12 11:20:53 PDT
(In reply to comment #11)
> Comment on attachment 276215 [details]
> [PATCH] Proposed Fix
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=276215&action=review
> 
> > Source/WebKit2/WebProcess/WebPage/WebInspector.cpp:226
> > +    m_page->corePage()->inspectorController().show();
> 
> Why show in the disable case?

This is basically a bring to front, but that actually should happen automatically if the user selected something, so I should be able to drop it here!
Comment 13 BJ Burg 2016-04-12 11:47:21 PDT
Comment on attachment 276215 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=276215&action=review

> Source/WebKit2/UIProcess/API/C/WKInspector.cpp:130
> +    toImpl(inspectorRef)->togglePointToInspect();

I would prefer a simple isPointToInspectEnabled / setPointToInspectEnabled pair rather than having toggle. That way a client doesn't have to double-check every time if they want to unconditionally enable it.

> Source/WebKit2/WebProcess/WebPage/WebInspectorUI.messages.in:39
> +    EnablePointToInspect()

Can this just be one message, SetPointToInspectEnabled?
Comment 14 Joseph Pecoraro 2016-04-12 12:11:40 PDT
Created attachment 276259 [details]
[PATCH] Improved Version

This changes the workflow.

Now when Toggle happens in the UIProcess:
  - if starting, connect() so the inspector doesn't open until you select something
  - if closing, we can do nothing if nothing was selected. If something was selected, the inspector will bring to front automatically anyways
Comment 15 Joseph Pecoraro 2016-04-12 12:36:43 PDT
<http://trac.webkit.org/changeset/199380>