Bug 17772 – Inspector should support point-and-click to select a node to inspect

• Component: Web Inspector (Deprecated)
• Status: RESOLVED FIXED

[Adam Roben (:aroben)]

The Inspector should support point-and-click to select a node to inspect. This has a few advantages over the current "Inspect Element" context menu item:

1. It's clearer which node you're choosing, because the hovered node will be outlined before you choose it.
2. It's potentially more discoverable than "Inspect Element".
3. It doesn't require you to click on the node you're going to inspect, which could potentially change its state.

Bug 16532 covers a complimentary (and inverse) behavior.

[Adam Roben (:aroben)]

<rdar://problem/5792395>

[Matt Lilek]

*** Bug 19322 has been marked as a duplicate of this bug. ***

[Matt Lilek]

Created attachment 23456 [details]
Initial cut

As I wrote in the ChangeLog:

Despite putting this up for review, I'm not intending to land it as is. Among the known issues:
        - Need better art for the magnifying glass button
        - Is EventHandler really the best place for intercepting the node click - feels dirty. Would
          making -[WebView setHoverFeedbackSuspended:] cross-platform be a better route?
        - I think I can get away with not having to track searchingForNode in ElementsPanel
        - Some method names could be tweaked.

[Matt Lilek]

Hrm....git diff didn't include the button png....

[Matt Lilek]

Created attachment 23549 [details]
round 2

This isn't much different from the previous version, but is more landable - ie: uses localizedStrings and also fixes a couple of bugs like stopping the search when you switch panels, but still does have a few bugs.

[Matt Lilek]

Created attachment 23550 [details]
button

[Timothy Hatcher]

Comment on attachment 23549 [details]
round 2

+    InspectorController* inspector = m_page->inspectorController();
+    if (!inspector)
+        return;
+    inspector->mouseDidMoveOverElement(result, modifierFlags);


Better written as:

if (InspectorController* inspector = m_page->inspectorController())
       inspector->mouseDidMoveOverElement(result, modifierFlags);

+    InspectorController* controller = reinterpret_cast<InspectorController*>(JSObjectGetPrivate(thisObject));
+    if (controller)
+        controller->toggleSearchForNodeInPage();

Better written as:

if (InspectorController* controller = reinterpret_cast<InspectorController*>(JSObjectGetPrivate(thisObject)))
      controller->toggleSearchForNodeInPage();

[Timothy Hatcher]

Created attachment 23551 [details]
Better button

[Timothy Hatcher]

Created attachment 23552 [details]
Even better button

[Matt Lilek]

Created attachment 23553 [details]
with suggestions

[Matt Lilek]

Comment on attachment 23553 [details]
with suggestions

broke something, hang on

[Matt Lilek]

Created attachment 23583 [details]
round 3

Fixes the bugs with toggle behavior and includes Tim's version of the button

[Matt Lilek]

Created attachment 23584 [details]
round 3

I know how to read and click on the right file, honest

[Matt Lilek]

Committed revision 36685.