(In reply to Lin Clark [:linclark] from comment #5) > Right, sorry if my wording of that question confused. > > As I said above, Nicholas's work would make it output: > > > Object { objProp: Object {}, strProp: "test string" } > > But this is not what we want... we want it to just be object, correct? Yes, correct. Honza

OK, sounds like with the planned updates doing console.log({objProp: {id: 1}, strProp: "test string"}) will continue to output: Object { objProp: Object, strProp: "test string" } That works for me.

I am updating the patch so, `Object` is rendered. Not that this will also affect JSON rendering (JSON view and JSON HTTP responses in the Console panel). Honza

Comment on attachment 8756389 [details] [diff] [review] bug1275556.patch Review of attachment 8756389 [details] [diff] [review]: ----------------------------------------------------------------- I didn't get a chance to manually test this, but it looks good to me.

Forgot to mention, we should make sure this passes tests before committing. I'm not sure if any of the JSON viewer or DOM Panel tests depends on the current output.

The Obj rep should support 'tiny' mode just like the 'Grip' rep does. Honza

Lin, here is a patch adding support for 'tiny' mode to the Obj rep. We might want to yet discuss/change how JS objects are rendered: Two options: `{}` - used by Reps atm (and Firebug), it's closer to JS syntax and requires less space (I recall the FB users required that). `Object` - this is used by the Console panel atm and Chrome devtools use it too. Honza

Brian, what do you think about JS object representation? See the comment above. Honza

Brian, for some more context, let's say we have this object... {objProp: {id: 1}, strProp: "test string"} --- here's what we currently show in console: > Object { objProp: Object, strProp: "test string" } Here's what we would show with Reps as they are: > { objProp: {}, strProp: "test string" } And here's what we would show with the changes Nicholas is putting in to place in https://github.com/bgrins/gecko-dev/pull/85: > Object { objProp: Object {}, strProp: "test string" } --- In the meeting this morning, we were leaning towards handling it as we currently do (which matches Chrome). Honza, given the work that Nicholas is doing, this would mean that in tiny mode the grip would only output its title (which is linked), right?

(In reply to Lin Clark [:linclark] from comment #3) > Honza, given the work that Nicholas is > doing, this would mean that in tiny mode the grip would only output its > title (which is linked), right? Nicholas's patch is changing getTitle() method that is used in 'tiny' and other modes. So, it would output `Object {}` - where `Object` is a link. See the following code that renders Grip rep. The 'tiny' mode is using getTitle() and `{}` string. https://github.com/bgrins/gecko-dev/blob/console-frontend/devtools/client/shared/components/reps/grip.js#L142 Honza

Right, sorry if my wording of that question confused. As I said above, Nicholas's work would make it output: > Object { objProp: Object {}, strProp: "test string" } But this is not what we want... we want it to just be object, correct?

(In reply to Lin Clark [:linclark] from comment #10) > Forgot to mention, we should make sure this passes tests before committing. > I'm not sure if any of the JSON viewer or DOM Panel tests depends on the > current output. All JSON and DOM tests pass for me, and here is a link to try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1b6e5510862f I changed one thing in the patch. Obj.getTitle() should not use `object.class` since the `object` is not a grip and so, "Object" title should always be used. Honza

New try push (the previous has wrong commit message) https://treeherder.mozilla.org/#/jobs?repo=try&revision=603a3056b97e Honza

I downloaded the previous patch (the one that I reviewed yesterday) to use with tests. I noticed that nested objects are rendering like this. > Object{objProp: Object, strProp: "test string"} I think we need a space in there after the first title.

(In reply to Lin Clark [:linclark] from comment #13) > I downloaded the previous patch (the one that I reviewed yesterday) to use > with tests. I noticed that nested objects are rendering like this. > > > Object{objProp: Object, strProp: "test string"} > > I think we need a space in there after the first title. Agreed, done. Honza

There are clipboard failures, but otherwise it looks good. Honza