Bug 20930 - Web Inspector: Show node description in inspector highlight
Summary: Web Inspector: Show node description in inspector highlight
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: Alexander Pavlov (apavlov)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-09-18 21:02 PDT by Matt Lilek
Modified: 2010-09-17 06:37 PDT (History)
7 users (show)

See Also:


Attachments
[IMAGE] Screenshot of a node description displayed. (109.40 KB, image/png)
2010-07-30 10:48 PDT, Alexander Pavlov (apavlov)
no flags Details
[IMAGE] Screenshot of a fixed node description. (13.49 KB, image/png)
2010-09-16 04:51 PDT, Alexander Pavlov (apavlov)
no flags Details
[PATCH] Suggested solution (5.06 KB, patch)
2010-09-17 03:09 PDT, Alexander Pavlov (apavlov)
no flags Details | Formatted Diff | Diff
[PATCH] Chromium compilability fix (5.45 KB, patch)
2010-09-17 03:48 PDT, Alexander Pavlov (apavlov)
pfeldman: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Matt Lilek 2008-09-18 21:02:03 PDT
Especially once bug 17772 lands, it would be useful to display the "element#id.class" string from the breadcrumb bar on the node highlight itself.  We may not want it to be visible by default, but would definitely be useful with a modifier key.
Comment 1 Timothy Hatcher 2008-09-18 21:10:09 PDT
I like this idea! Always visible seems fine, or always when in Node Search mode.
Comment 2 Alexander Pavlov (apavlov) 2010-07-29 09:38:03 PDT
This simple request needs thorough UI design (cross-platform, title layout/font, etc.)
Comment 3 Alexander Pavlov (apavlov) 2010-07-30 10:48:15 PDT
Created attachment 63083 [details]
[IMAGE] Screenshot of a node description displayed.

This is a suggested description appearance: <nodeName#id.class1.class2...classN> [offsetWidth x offsetHeight].

Timothy, could you have a look at it and suggest something wrt the UI/data presentation?
Comment 4 Timothy Hatcher 2010-07-30 13:24:49 PDT
Some quick comments:

I think showing this note in the middle of the box would be better and be less likely to be clipped. If you show it in the middle I don' think it needs a box around it, maybe just bolder text or a text shadow.

Showing the node text in <> is odd, since it is an invalid tag when there is an id or class name. Just drop the <> or something smarter.

Don't use "x" for pixel size, use &times;. It is odd to give just one size, since there are multiple deminisions when padding and margin, what does this size include?
Comment 5 Alexander Pavlov (apavlov) 2010-07-30 13:37:05 PDT
(In reply to comment #4)

Thanks for taking a look!

> Some quick comments:
> 
> I think showing this note in the middle of the box would be better and be less likely to be clipped. If you show it in the middle I don' think it needs a box around it, maybe just bolder text or a text shadow.

Rendering the label in the middle risks obscuring border/margin rectangles for small elements (esp. narrow ones, should they have a lot of style classes) but I seem to have seen this approach elsewhere so this might work ok.
Without any background, it's going to be quite messy since it may overlay some existing text or image containing a similar pattern in the label area, even with a shadow (I've tried it out).

> Showing the node text in <> is odd, since it is an invalid tag when there is an id or class name. Just drop the <> or something smarter.

Yes, we were just sitting there experimenting, and this was one state snapshot. I don't know if there's something good we can use to enclose the text, guess should just drop the <>.

> Don't use "x" for pixel size, use &times;. It is odd to give just one size, since there are multiple deminisions when padding and margin, what does this size include?

This text is rendered in the native code (TextRun), so have to look up what &times; corresponds to.
This size includes [offsetWidth x offsetHeight]. Do you have any suggestions on how to layout multiple sizes/more data in this "tooltip" area?
Comment 6 Alexander Pavlov (apavlov) 2010-09-10 02:15:46 PDT
This bug is related to bug 17221
Comment 7 Alexander Pavlov (apavlov) 2010-09-16 04:51:29 PDT
Created attachment 67781 [details]
[IMAGE] Screenshot of a fixed node description.

The new screenshot addressed the "times" and element identifier format issues. Please speak up.
Comment 8 Alexander Pavlov (apavlov) 2010-09-17 02:14:10 PDT
PFELDMAN, PLEASE DO SPEAK UP :)
Comment 9 Pavel Feldman 2010-09-17 02:32:52 PDT
Looks good to me, send the patch for review!
Comment 10 Alexander Pavlov (apavlov) 2010-09-17 03:09:08 PDT
Created attachment 67896 [details]
[PATCH] Suggested solution
Comment 11 WebKit Review Bot 2010-09-17 03:27:34 PDT
Attachment 67896 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/4022041
Comment 12 Alexander Pavlov (apavlov) 2010-09-17 03:48:17 PDT
Created attachment 67898 [details]
[PATCH] Chromium compilability fix
Comment 13 Pavel Feldman 2010-09-17 06:17:44 PDT
Comment on attachment 67898 [details]
[PATCH] Chromium compilability fix

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

> WebCore/ChangeLog:5
> +        Show node description in inspector highlight

Web Inspector: prefix.

> WebCore/inspector/InspectorController.h:35
> +#include "FloatRect.h"

Use forward declarations instead.
Comment 14 Alexander Pavlov (apavlov) 2010-09-17 06:37:37 PDT
Committing to http://svn.webkit.org/repository/webkit/trunk ...
        M       WebCore/ChangeLog
        M       WebCore/inspector/InspectorController.cpp
        M       WebCore/inspector/InspectorController.h
Committed r67703