Bug 17772 - Inspector should support point-and-click to select a node to inspect
Summary: Inspector should support point-and-click to select a node to inspect
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Enhancement
Assignee: Matt Lilek
URL:
Keywords: InRadar
: 19322 (view as bug list)
Depends on:
Blocks:
 
Reported: 2008-03-11 09:36 PDT by Adam Roben (:aroben)
Modified: 2008-09-19 17:12 PDT (History)
4 users (show)

See Also:


Attachments
Initial cut (11.83 KB, patch)
2008-09-15 19:23 PDT, Matt Lilek
timothy: review+
Details | Formatted Diff | Diff
round 2 (12.83 KB, patch)
2008-09-18 20:41 PDT, Matt Lilek
timothy: review+
Details | Formatted Diff | Diff
button (1.72 KB, image/png)
2008-09-18 20:44 PDT, Matt Lilek
no flags Details
Better button (5.78 KB, image/png)
2008-09-18 21:34 PDT, Timothy Hatcher
no flags Details
Even better button (5.75 KB, image/png)
2008-09-18 21:37 PDT, Timothy Hatcher
no flags Details
with suggestions (12.34 KB, patch)
2008-09-18 21:42 PDT, Matt Lilek
dev+webkit: review-
Details | Formatted Diff | Diff
round 3 (12.34 KB, patch)
2008-09-19 16:30 PDT, Matt Lilek
no flags Details | Formatted Diff | Diff
round 3 (13.26 KB, patch)
2008-09-19 16:33 PDT, Matt Lilek
timothy: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adam Roben (:aroben) 2008-03-11 09:36:57 PDT
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.
Comment 1 Adam Roben (:aroben) 2008-03-11 09:38:12 PDT
<rdar://problem/5792395>
Comment 2 Matt Lilek 2008-05-30 08:59:26 PDT
*** Bug 19322 has been marked as a duplicate of this bug. ***
Comment 3 Matt Lilek 2008-09-15 19:23:32 PDT
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.
Comment 4 Matt Lilek 2008-09-15 19:24:37 PDT
Hrm....git diff didn't include the button png....
Comment 5 Matt Lilek 2008-09-18 20:41:00 PDT
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.
Comment 6 Matt Lilek 2008-09-18 20:44:01 PDT
Created attachment 23550 [details]
button
Comment 7 Timothy Hatcher 2008-09-18 20:52:44 PDT
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();
Comment 8 Timothy Hatcher 2008-09-18 21:34:03 PDT
Created attachment 23551 [details]
Better button
Comment 9 Timothy Hatcher 2008-09-18 21:37:09 PDT
Created attachment 23552 [details]
Even better button
Comment 10 Matt Lilek 2008-09-18 21:42:21 PDT
Created attachment 23553 [details]
with suggestions
Comment 11 Matt Lilek 2008-09-18 21:48:11 PDT
Comment on attachment 23553 [details]
with suggestions

broke something, hang on
Comment 12 Matt Lilek 2008-09-19 16:30:13 PDT
Created attachment 23583 [details]
round 3

Fixes the bugs with toggle behavior and includes Tim's version of the button
Comment 13 Matt Lilek 2008-09-19 16:33:20 PDT
Created attachment 23584 [details]
round 3

I know how to read and click on the right file, honest
Comment 14 Matt Lilek 2008-09-19 17:12:08 PDT
Committed revision 36685.