Bug 30913 - Web Inspector: hover over JS "things" in source and see their values
Summary: Web Inspector: hover over JS "things" in source and see their values
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Timothy Hatcher
URL:
Keywords:
: 17239 (view as bug list)
Depends on:
Blocks:
 
Reported: 2009-10-29 09:57 PDT by Patrick Mueller
Modified: 2009-11-03 11:38 PST (History)
10 users (show)

See Also:


Attachments
demo (1.75 KB, patch)
2009-11-03 06:50 PST, Keishi Hattori
no flags Details | Formatted Diff | Diff
Initial implementation (2.62 KB, patch)
2009-11-03 09:42 PST, Keishi Hattori
timothy: review-
Details | Formatted Diff | Diff
Initial implementation 2 (2.68 KB, patch)
2009-11-03 10:08 PST, Keishi Hattori
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Patrick Mueller 2009-10-29 09:57:02 PDT
Spawned from: https://twitter.com/siracusa/status/5259300238

   The WebKit Inspector is getting a lot better, 
   but I still really miss the ability to mouse-over 
   JavaScript variables and see their values.

Some discussion on IRC re: what the "things" are.  Expressions?  Or just variables?  Expressions which are property accesses of the form "x.y.z"?  Do we need to worry about getters?

From IRC, xenon says: "we woud likely need to limit it to properties, no functions and not getters", which I think means things that match "x.y.z", but am unsure how we detect getters.  Am thinking supporting getters, by not doing anything special with them, is probably ok - most getters are likely side-effect free, or relatively so.

Also mentioned on IRC was that the new API caretRangeFromPoint() will make this easier to implement.
Comment 1 John Siracusa 2009-10-29 10:25:24 PDT
I'd start with "do what FireBug does," which seems to be variables and simple properties (e.g., x and x.y.z, where you can mouse-over x, y, or z).

The main reason I want this feature is that I find it difficult to see the values of variables when they don't fit within the "Scope Variables" sidebar area.  The FireBug-style mouse-over tooltips have plenty of room to show even very long values.  Also, even for short values, mousing over a variable is direct and doesn't require scanning a list of variables in the sidebar.
Comment 2 Mark Rowe (bdash) 2009-10-29 13:38:22 PDT
Xcode has a quite reasonable UI for doing this, including an affordance for evaluating expressions that can potentially run application code.
Comment 3 Keishi Hattori 2009-11-03 06:42:53 PST
*** Bug 17239 has been marked as a duplicate of this bug. ***
Comment 4 Keishi Hattori 2009-11-03 06:50:55 PST
Created attachment 42377 [details]
demo

I don't think we need to use caretRangeFromPoint.
Comment 5 Timothy Hatcher 2009-11-03 08:57:55 PST
(In reply to comment #4)
> Created an attachment (id=42377) [details]
> demo
> 
> I don't think we need to use caretRangeFromPoint.

Awesome! You are right. Now that our syntax highlighter is better, this is good!

We should figure out when to clear the title attribute. Maybe right in mouseover, so you don't see stale data?

Also this should be while paused only i think.

Later we might want to add UI like Xcode to let you dig in and expand objects, etc.
Comment 6 Keishi Hattori 2009-11-03 09:42:22 PST
Created attachment 42390 [details]
Initial implementation

> We should figure out when to clear the title attribute. Maybe right in
> mouseover, so you don't see stale data?
> 
> Also this should be while paused only i think.
Fixed.

> Later we might want to add UI like Xcode to let you dig in and expand objects,
> etc.
I think the red dotted line around the evaluated expression in Xcode is also a nice touch.
Comment 7 Timothy Hatcher 2009-11-03 09:55:01 PST
Comment on attachment 42390 [details]
Initial implementation

> +            identElement.addEventListener("mouseover", function(event) {

Can you split out the inline functions and give them named and put them somewhere and reference them by name? They are too long to be good inline.


> +                }
> +                var expression = parts.join(".");

Put a new line between these lines.


> +                WebInspector.panels.scripts.evaluateInSelectedCallFrame(expression, false, "console", function(result, exception) {

Again break out the inline function.


> +                    function onmouseout(event) {

Brace on the next line.
Comment 8 Keishi Hattori 2009-11-03 10:08:03 PST
Created attachment 42393 [details]
Initial implementation 2
Comment 9 Adam Roben (:aroben) 2009-11-03 10:11:06 PST
Comment on attachment 42393 [details]
Initial implementation 2

It seems a little unfortunate to reevaluate the expression each time the mouse moves over it if no code has executed since last time. We could cache the result of the evaluation somewhere and then invalidate the cache the next time JS is allowed to execute.
Comment 10 Timothy Hatcher 2009-11-03 11:18:36 PST
(In reply to comment #9)
> (From update of attachment 42393 [details])
> It seems a little unfortunate to reevaluate the expression each time the mouse
> moves over it if no code has executed since last time. We could cache the
> result of the evaluation somewhere and then invalidate the cache the next time
> JS is allowed to execute.

Good point. We should do this.
Comment 11 WebKit Commit Bot 2009-11-03 11:38:08 PST
Comment on attachment 42393 [details]
Initial implementation 2

Clearing flags on attachment: 42393

Committed r50472: <http://trac.webkit.org/changeset/50472>
Comment 12 WebKit Commit Bot 2009-11-03 11:38:13 PST
All reviewed patches have been landed.  Closing bug.