Closed Bug 1391274 Opened 2 years ago Closed 2 years ago

The console should show the symbol description when previewing a symbol object

Categories

(DevTools :: Console, defect)

defect
Not set

Tracking

(firefox57 verified)

VERIFIED FIXED
Firefox 57
Tracking Status
firefox57 --- verified

People

(Reporter: Oriol, Assigned: Oriol)

Details

Attachments

(1 file, 1 obsolete file)

Enter this code to the console:

    Object(Symbol("abc"))

Result: Symbol {  }
Expected: Symbol { "abc" }
This is the grip we get : 

```
{
  "type": "object",
  "actor": "server2.conn2.child18/obj30",
  "class": "Symbol",
  "extensible": true,
  "frozen": false,
  "sealed": false,
  "ownPropertyLength": 0,
  "preview": {
    "kind": "Object",
    "ownProperties": {},
    "ownSymbols": [],
    "ownPropertiesLength": 0,
    "ownSymbolsLength": 0,
    "safeGetterValues": {}
  }
}
```

So, 1) it is rendered by the Grip rep, not the Symbol one, 2) we're missing the Symbol's description (see https://github.com/devtools-html/devtools-core/blob/16876e8a69a07f26a511fd021e7c26325a13f501/packages/devtools-reps/src/reps/stubs/symbol.js#L6-L9 to see what we usually get).
Probably object.js should use wrappedPrimitivePreviewer. This would provide a wrappedValue property with the symbol grip.
Attached patch console-symbol-object.patch (obsolete) — Splinter Review
With this patch,
    Object(Symbol("abc"))
is rendered as
    ▶ Symbol { Symbol(abc) }

The repetition of "Symbol" is redundant, but this can be handled in Reps.

I removed the condition that the class of the prototype of the constructor should be the name of the constructor. That's because Symbol.prototype is not a Symbol object. This also implies that
    Object.setPrototypeOf(new String('aa'), null)
becomes
    ▶ String { "aa" }
instead of the current
    ▶ String { … }
Assignee: nobody → oriol-bugzilla
Status: NEW → ASSIGNED
Attachment #8909818 - Flags: review?(nchevobbe)
Comment on attachment 8909818 [details] [diff] [review]
console-symbol-object.patch

Review of attachment 8909818 [details] [diff] [review]:
-----------------------------------------------------------------

This seems reasonable to me.
Could you add a test to make sure the grip that we send in this case is what we expect please ? 
I'd happily land this with a test, thanks Oriol !
Attachment #8909818 - Flags: review?(nchevobbe) → review+
Attachment #8909818 - Attachment is obsolete: true
Comment on attachment 8910209 [details]
Bug 1391274 - Add a Symbol object previewer to the console.

https://reviewboard.mozilla.org/r/181694/#review187398

Thanks Oriol, this is looking good.
I pushed to TRY and when it's green we'll push this patch.
Attachment #8910209 - Flags: review?(nchevobbe) → review+
I already pushed to try (https://treeherder.mozilla.org/#/jobs?repo=try&revision=105cf96aad6f), and it's green.
Keywords: checkin-needed
It was without the test :)
We should wait to see if it is not failing for some reason
Oh sorry, it was, I missed it.
Let's push it then
Pushed to autoland
Keywords: checkin-needed
Pushed by nchevobbe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1c88655364be
Add a Symbol object previewer to the console. r=nchevobbe
https://hg.mozilla.org/mozilla-central/rev/1c88655364be
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
I have reproduced this Bug with Nightly 57.0a1 (2017-08-17) on Windows 10, 64 Bit!

The fix is now verified on latest Beta 57.0b7

Build ID 	20171009192146
User Agent 	Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:57.0) Gecko/20100101 Firefox/57.0
QA Whiteboard: [bugday-20171011]
I have reproduced this Bug with Nightly 57.0a1 (2017-08-17) on Ubuntu 16.04 LTS!

This bug's fix is verified with latest Beta!

Build ID 	20171023180840
User Agent 	Mozilla/5.0 (X11; Linux x86_64; rv:57.0) Gecko/20100101 Firefox/57.0

[bugday-20171025]
Verified per comment 15.
Status: RESOLVED → VERIFIED
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.