Closed Bug 753332 Opened 12 years ago Closed 10 years ago

Have nicer variable/property value displays for specific types in the VariablesView

Categories

(DevTools :: Object Inspector, defect, P3)

defect

Tracking

(Not tracked)

RESOLVED DUPLICATE of bug 843004

People

(Reporter: vporof, Unassigned)

References

Details

Attachments

(1 file)

      No description provided.
Blocks: 752834
No longer blocks: 752834
Depends on: 752834
I'm a little concerned about https://bugzilla.mozilla.org/show_bug.cgi?id=752834#c2
Let's say we have Arguments, somewhere in there there's an Array, and inside it is an Object. We'll need to display [ bla, [ blu, ble, { blo: bli, ... } ... ] ... ]. This means recursively and silently expanding nodes to get their contents in order to create a proper value label.

Solutions: 1. directly access the value itself and pull a json (does eval work here?)
           2. getPrototypeAndProperties until we get tired

Panos?
Attached patch v-1Splinter Review
WIP. Doesn't do much.
(In reply to Victor Porof from comment #1)
> I'm a little concerned about
> https://bugzilla.mozilla.org/show_bug.cgi?id=752834#c2
> Let's say we have Arguments, somewhere in there there's an Array, and inside
> it is an Object. We'll need to display [ bla, [ blu, ble, { blo: bli, ... }
> ... ] ... ]. This means recursively and silently expanding nodes to get
> their contents in order to create a proper value label.
> 
> Solutions: 1. directly access the value itself and pull a json (does eval
> work here?)
>            2. getPrototypeAndProperties until we get tired
> 
> Panos?

I don't understand how 1 works. 2 is how I imagine this would work, and additionally maybe only show first-level values and use an ellipsis for the rest. Multiple protocol requests on pause may make the debugger appear slow (although we could add a 50ms timeout to stop recursion and be in line with Firefox latency principles).
No longer depends on: 752834
Depends on: 752834
Alright, so this is harder than I thought. We can just use the thing we have now.
Priority: P2 → P3
I think we should have some sort of protocol support for this.

Talked it over with dcamp about it a bit, and the main outcome of the discussion was that the algorithm for displaying these descriptions (getPrototypeAndProperties until we get tired) requires a lot of data, but results in very little (a short string). Doing this for each variable in the view would (a) not provide consistent results, if timing out after a short time to avoid making the debugger appear slow, (b) may result in overcomplicating the debugger frontend for something that's more elegantly and easily fixed via the protocol, and (c) sending too much data over the wire to get the result will be sad.

One very good idea is to have a moz-prefixed function, like mozGetShortDescription, which would act something like this:
JSON.stringify(myObject).slice(0, -MAX_LENGTH) +? ellipsis
(In reply to Victor Porof [:vp] from comment #5)
> I think we should have some sort of protocol support for this.
> 
> Talked it over with dcamp about it a bit, and the main outcome of the
> discussion was that the algorithm for displaying these descriptions
> (getPrototypeAndProperties until we get tired) requires a lot of data, but
> results in very little (a short string). Doing this for each variable in the
> view would (a) not provide consistent results, if timing out after a short
> time to avoid making the debugger appear slow, (b) may result in
> overcomplicating the debugger frontend for something that's more elegantly
> and easily fixed via the protocol, and (c) sending too much data over the
> wire to get the result will be sad.
> 
> One very good idea is to have a moz-prefixed function, like
> mozGetShortDescription, which would act something like this:
> JSON.stringify(myObject).slice(0, -MAX_LENGTH) +? ellipsis

You are suggesting that such a method be added to the JS engine, correct? If JS people agree, I'm all for that, but can't we have something simple working in the meantime? Just asking once for the object/array properties would be enough to display a useful summary IMO. No need for deep inspection to provide something that should fit in 20 characters or so.
(In reply to Panos Astithas [:past] from comment #6)
> (In reply to Victor Porof [:vp] from comment #5)
> > I think we should have some sort of protocol support for this.
> > 
> > Talked it over with dcamp about it a bit, and the main outcome of the
> > discussion was that the algorithm for displaying these descriptions
> > (getPrototypeAndProperties until we get tired) requires a lot of data, but
> > results in very little (a short string). Doing this for each variable in the
> > view would (a) not provide consistent results, if timing out after a short
> > time to avoid making the debugger appear slow, (b) may result in
> > overcomplicating the debugger frontend for something that's more elegantly
> > and easily fixed via the protocol, and (c) sending too much data over the
> > wire to get the result will be sad.
> > 
> > One very good idea is to have a moz-prefixed function, like
> > mozGetShortDescription, which would act something like this:
> > JSON.stringify(myObject).slice(0, -MAX_LENGTH) +? ellipsis
> 
> You are suggesting that such a method be added to the JS engine, correct? If
> JS people agree, I'm all for that, but can't we have something simple
> working in the meantime? Just asking once for the object/array properties
> would be enough to display a useful summary IMO. No need for deep inspection
> to provide something that should fit in 20 characters or so.

I actually think there is no need for an extra request, because we already make that to decide if we should display the arrow or not.
(In reply to Panos Astithas [:past] from comment #7)
> I actually think there is no need for an extra request, because we already
> make that to decide if we should display the arrow or not.

Scratch that, we don't actually make any redundant requests at all.
(In reply to Panos Astithas [:past] from comment #6)
> (In reply to Victor Porof [:vp] from comment #5)
> > I think we should have some sort of protocol support for this.
> > 
> > Talked it over with dcamp about it a bit, and the main outcome of the
> > discussion was that the algorithm for displaying these descriptions
> > (getPrototypeAndProperties until we get tired) requires a lot of data, but
> > results in very little (a short string). Doing this for each variable in the
> > view would (a) not provide consistent results, if timing out after a short
> > time to avoid making the debugger appear slow, (b) may result in
> > overcomplicating the debugger frontend for something that's more elegantly
> > and easily fixed via the protocol, and (c) sending too much data over the
> > wire to get the result will be sad.
> > 
> > One very good idea is to have a moz-prefixed function, like
> > mozGetShortDescription, which would act something like this:
> > JSON.stringify(myObject).slice(0, -MAX_LENGTH) +? ellipsis
> 
> You are suggesting that such a method be added to the JS engine, correct? If
> JS people agree, I'm all for that, but can't we have something simple
> working in the meantime? Just asking once for the object/array properties
> would be enough to display a useful summary IMO. No need for deep inspection
> to provide something that should fit in 20 characters or so.

Apparently I got it all wrong here. After talking to Victor, he explained that he just wants to move the object/array traversal code from the client to the server. One way to do it would be an extra "summary" property in an Object's grip, or alternatively, an extra getObjectSummary request.

As a point of comparison, Chrome displays arrays pretty much as plainly as we do, but Firebug shows a toString-lookalike that goes 2 levels deep. For that, with the current code, we'd need one extra request for the object/array and one for each of its properties that are either objects or arrays. With the "getObjectSummary" we'd need one extra request for the object/array. With the "summary" property we wouldn't need any extra requests at all.

Besides the complexity of the async code in the client, the current situation poses a risk that the debugger frontend will freeze on pauses while fetching variable summaries. We could refrain from adding such a costly feature to the frontend, but it definitely seems useful after you've played with Firebug for a while.
Taking this bug, as discussed on irc with Victor.
Assignee: vporof → nfitzgerald
Summary: Have nicer property name displays for specific types in the PropertyView → Have nicer variable/property value displays for specific types in the VariablesView
See Also: → 843004
Blocks: 843004
There's an aspect of the protocol that I think was specified but didn't make it into the implementation: you're supposed to be able to pipeline requests --- that is, send a whole bunch of requests to a request/reply actor, without waiting for each reply. Since the actor follows the request/reply pattern, it's always easy to pair up requests and replies.

This doesn't save any bandwidth, but I *suspect* (no data!) that it's round-trip time that hurts us more than the actual size of the requests, and as far as round-trip time is concerned, a series of pipelined requests is essentially one big fat request: it should have excellent latency.

So my hope was that we could do deep expansion of objects like this efficiently, by enumerating an object's own properties, then sending off requests for all the properties' values at once (if there aren't too many), and then gathering them up as they come back.

Unfortunately, I believe our client explicitly prevents pipelining, in DebuggerClient.prototype._sendRequests. :(

And maybe it wouldn't work anyway.

In the mean time, adding a "deepPrototypeAndProperties" packet that gives you all the data you want doesn't sound like a bad idea.

I don't think actually using JSON.stringify is a good idea:

js> JSON.stringify({ x: function() { } }) 
"{}"
js> 

I know this is just for a summary, but while that behavior is fine for a serialization format that simply can't express functions (JSON), that's actively deceptive in a debugger. "Oh, that can't it; the object I was expecting has tons of properties!"
For a one-deep summary, doesn't a single prototypeAndProperties request give you everything you want?
(In reply to Jim Blandy :jimb from comment #13)
> For a one-deep summary, doesn't a single prototypeAndProperties request give
> you everything you want?

Consider nested arrays like [1, [2, [3, 4]]] etc. 
A simple description of the top level properties ("foo: [], bar: {}" or "foo: Array, bar: Object") is sometimes less than optimal. Firebug is rather smart about it and peels off a few layers to give the most info possible about the inner structure of an object without adding too much clutter.
Ideally, you generate the string you want to display on the server. As pointed out by Victor, different kinds of objects have different requirements - on the server you can "peel off" the object with no restrictions. If we are to do it from the client, we would have to send off a bunch of requests to the server for every object we receive during logging, and we would have to halt display until replies are received.

Consider the case when you receive a ton of messages from the console API with many objects that you need to display. In the mean time, exceptions happen, somewhere else in the page scripts, or some network requests. We can't show all of the other messages, out-of-order, and wait for the console API messages to be ready for display. We can introduce sorting, but then the output would be like a puzzle: one message comes in here, one there, another at the top, and so on.

Another option would be to show messages as they arrive, and update the output later, when the server replies with all the details the client needs to prettify output for the given object. This might be odd, but workable.
(In reply to Victor Porof [:vp] from comment #14)
> > For a one-deep summary, doesn't a single prototypeAndProperties request give
> > you everything you want?
> 
> Consider nested arrays like [1, [2, [3, 4]]] etc.
> A simple description of the top level properties ("foo: [], bar: {}" or
> "foo: Array, bar: Object") is sometimes less than optimal. Firebug is rather
> smart about it and peels off a few layers to give the most info possible
> about the inner structure of an object without adding too much clutter.

That seems really nice. That kind of inspection would certainly entail a lot of round trips. Making the decisions on the server about what to show, when to use ellipsis, etc. makes sense.

Is it really just about generating strings? Don't we want these little object displays to be inspectable / expandable / editable / and so on? If that's the case, then we should be sending back something grip-like, with actor names in the appropriate places so the client can refer to specific objects in subsequent queries.
(In reply to Jim Blandy :jimb from comment #16)
> Is it really just about generating strings? Don't we want these little
> object displays to be inspectable / expandable / editable / and so on? If
> that's the case, then we should be sending back something grip-like, with
> actor names in the appropriate places so the client can refer to specific
> objects in subsequent queries.

These display strings will be used for the "summary" label on the right of the property name that is currently displaying a toString look-alike ([object Array]). Expanding the property will still be available to let you drill down its own properties, making further prototypeAndProperties requests as necessary.
(In reply to Panos Astithas [:past] from comment #17)
> (In reply to Jim Blandy :jimb from comment #16)

One thing to also take into consideration is that Firebug also colorizes numbers, strings nulls, undefineds, etc. Now that I think more about it, maybe it'd be useful if the summary sent by the server was also accompanied by a bit of metadata about what it actually contains. If not, then maybe a crude parser on the frontend could match primitives and be done with it, and while it feels a bit dirty, it's certainly a dumb but simple and pertinent solution.
(In reply to Victor Porof [:vp] from comment #18)
> One thing to also take into consideration is that Firebug also colorizes
> numbers, strings nulls, undefineds, etc. Now that I think more about it,
> maybe it'd be useful if the summary sent by the server was also accompanied
> by a bit of metadata about what it actually contains.

Right, this is what I was getting at: the server should certainly gather the data and make decisions about what to display, how deep to traverse, and so on, because that's what requires fine-grained interactions with the data (through Debugger.Object instances).

But the server should then report its results to the client in a machine-readable form, with appropriate details (somewhat bulky is okay). The actual production of the display string can be done on the client. It'd still be one round trip, but the client would get more benefit from the data the server gathered.

(My intuition is that converting things to strings always loses data, and should be done as late as possible.

"The string is a stark data structure and everywhere it is passed there is much duplication of process. It is a perfect vehicle for hiding information."
-- Alan Perlis, who also said stuff I don't really get, but I like this one
)
Un-assigning myself because it seems we haven't completely agreed what needs to be done and I'm not actually working on a patch.
Assignee: nfitzgerald → nobody
Status: ASSIGNED → NEW
How about something like, from the following input object:

> [1, { a: true, b: new Map() }, 'b', , null];

You would get (perhaps from a debugger protocol request rather than automatically for every object):

> {
>   type: "object",
>   class: "Array",
>   properties: [{
>     key: "0",
>     type: "number",
>     value: "1"
>   }, {
>     key: "1",
>     type: "object",
>     class: "Object",
>     properties: [{
>       key: "a",
>       type: "boolean",
>       value: "true"
>     }, {
>       key: "b",
>       type: "object",
>       class: "Map",
>       properties: []
>     }]
>   }, {
>     key: "2",
>     type: "string",
>     value: "b"
>   }, {
>     key: "4",
>     type: "null",
>     value: "null"
>   }]
> }

It would need some additional protections for circular references and whatnot, but I think the basic idea would work well.
Maybe something like this for circular references.
> var obj = { a: true, b: new Map() };
> obj.self = obj;
> [1, obj, obj, 'b', , null];

> {
>   type: "object",
>   id: 0,
>   class: "Array",
>   properties: [{
>     key: "0",
>     type: "number",
>     value: "1"
>   }, {
>     key: "1",
>     type: "object",
>     id: 1,
>     class: "Object",
>     properties: [{
>       key: "a",
>       type: "boolean",
>       value: "true"
>     }, {
>       key: "b",
>       type: "object",
>       class: "Map",
>       properties: []
>     }, {
>       key: "self",
>       ref: 1
>     }]
>   }, {
>     key: "2",
>     ref: 1
>   }, {
>     key: "3",
>     type: "string",
>     value: "b"
>   }, {
>     key: "5",
>     type: "null",
>     value: "null"
>   }]
> }
Moving stuff to the Object Inspector component.
Filter on COSMICMIGRAINE.
Component: Developer Tools: Debugger → Developer Tools: Object Inspector
Version: 12 Branch → unspecified
I don't know what the current plan for this is, but it's one of the biggest things keeping me away from the built-in dev tools right now. It's just not very useful without a quick preview when debugging. Also, I think coloring of the output string is a critical part of this. Otherwise quickly observing object summaries and finding exactly what you want is tougher. It's as important in debugging as it is in writing code. Just thought I'd throw that out there in case that wasn't actually the plan.
No longer blocks: 843004
Depends on: 843004
found this accidentally. Victor, were you planning on doing anything with this? Should we pass this to Benvie?
(In reply to Rob Campbell [:rc] (:robcee) from comment #26)
> found this accidentally. Victor, were you planning on doing anything with
> this? Should we pass this to Benvie?

I'm not working on this at the moment. See also comment 20, we haven't yet reached a consensus on what the best approach is.
This bug is closely related to bug 843004. I'm preparing a proposal for protocol changes for ObjectActors to allow us to have pretty output for objects / property values. In terms of UI changes that will be up to whatever we decide for the console and the vview.

I will sync with Brandon and Rob about the protocol changes.
(In reply to Brandon Benvie [:benvie] from comment #21)
> You would get (perhaps from a debugger protocol request rather than
> automatically for every object):
> 
> > {
> >   type: "object",
> >   class: "Array",
> >   properties: [{
> >     key: "0",
> >     type: "number",
> >     value: "1"

I haven't looked at this in detail, but I like the idea.

Shouldn't this include grips, in case we want to look at the objects?
Can we mark this bug as fixed by bug 843004? Or do we want more variablesview-specific changes? Some time in the future.
FIXED! Thank you Mihai :)
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → DUPLICATE
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: