Show instance constructor name in console instead of "Object"
Categories
(DevTools :: Console, defect, P2)
Tracking
(Not tracked)
People
(Reporter: jelle, Unassigned, NeedInfo)
References
(Blocks 1 open bug)
Details
(Whiteboard: [dt-q])
Attachments
(1 file)
81.92 KB,
image/png
|
Details |
User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:65.0) Gecko/20100101 Firefox/65.0
Steps to reproduce:
I logged an instance of a custom class to the console, for example:
function MyClass() {};
console.log(new MyClass());
Actual results:
Firefox prints the object I can inspect, but it looks like Object { }
Expected results:
I expect to see the constructor name instead of Object
, so: MyClass { }
Chrome does it like this, and it makes the console output much cleaner and easier to debug.
The built-in classes (like Map, Date, Promise, DocumentFragment, ...) DO show the constructor name
I noticed some of them contained the Symbol.toStringTag
property in their prototype, but even adding that did nothing.
Comment 1•6 years ago
|
||
Setting the component, in order to involve the development team.
Updated•6 years ago
|
Updated•6 years ago
|
Comment 2•5 years ago
|
||
Impacts React, Angular, and other framework debugging and aligns with optimizing for modern build chains (which depend on es6 classes).
Also reported here: https://www.reddit.com/r/javascript/comments/ecyhmr/mozilla_survey_how_can_devtools_support_you/fbhbnng/
Comment 3•5 years ago
|
||
Brian, is this something we need better server-side/JS engine support for, or is it frontend (or sth else)?
Comment 4•5 years ago
|
||
(In reply to :Harald Kirschner :digitarald from comment #3)
Brian, is this something we need better server-side/JS engine support for, or is it frontend (or sth else)?
This would need spidermonkey changes in order to fix. When JS code calls 'new MyClass()' we create a vanilla object (a PlainObject in spidermonkey terms) which doesn't have any notion of what function was used to construct it or other information about where it came from. We can look at its prototype chain, which in this case will have been constructed lazily the first time MyClass.prototype was needed --- MyClass will point to the prototype, but not the other way around, however. Using the prototype gets more complicated with the typical pattern here of having something like MyClass.prototype = { ... }
or MyClass.prototype = someLibraryFunction(...)
and the prototype could be any random object. Determining the constructor function from an instance object would require some pretty significant new complexity and performance/memory overhead.
Comment 5•5 years ago
|
||
Couldn't ES6's instance.constructor.name
get is halfway there?
function MyClass() {};
console.log((new MyClass()).constructor.name);
Comment 6•5 years ago
|
||
(In reply to :Harald Kirschner :digitarald from comment #5)
Couldn't ES6's
instance.constructor.name
get is halfway there?function MyClass() {}; console.log((new MyClass()).constructor.name);
Hmm, that does fix the issue of getting to the constructor from the default function prototype, but if MyClass.prototype is anything other than its default value then we lose the link between the constructor function and the prototype.
Comment 7•5 years ago
|
||
Hmm, that does fix the issue of getting to the constructor from the default function prototype, but if MyClass.prototype is anything other than its default value then we lose the link between the constructor function and the prototype.
Not sure what would change the prototype, do you have an example of what use case would miss?
Would it make sense to split this into supporting the basic constructor.name
for labeling and a follow up bug for advanced JS instrumentation?
Comment 8•5 years ago
|
||
I oppose this change. If I want to see the constructor name, I can just use instance.constructor.name
.
What I want to see is the internal JSClass, so please don't remove this.
For example, Object.create(Array.prototype)
is a plain object instead of an exotic array object, so I do want to see Object
and not Array
.
Reporter | ||
Comment 9•5 years ago
|
||
(In reply to Oriol Brufau [:Oriol] from comment #8)
I oppose this change. If I want to see the constructor name, I can just use
instance.constructor.name
.
What I want to see is the internal JSClass, so please don't remove this.
For example,Object.create(Array.prototype)
is a plain object instead of an exotic array object, so I do want to seeObject
and notArray
.
The idea is that you can easily see what the constructor is in multiple places, not just by doing a manual console.log
on a single variable. It should also be visible at a glance when inspecting an object (or many, multiple objects)
Also: Object.create(Array.prototype)
does in fact make an Array instance, albeit a broken one with weird, unexpected behaviours like square-bracket style property getting, JSON stringifying and such. As you said yourself: it's quite the exotic odd one out :)
Yes, the way it currently is programmed in Firefox means you can easily see the difference between a "real" array and a "fake" array, but this one very special, very weird use-case should not prevent this from being implemented as other browsers have done.
Comment 10•5 years ago
|
||
(In reply to Jelle De Loecker from comment #9)
Also:
Object.create(Array.prototype)
does in fact make an Array instance, albeit a broken one with weird, unexpected behaviours like square-bracket style property getting, JSON stringifying and such. As you said yourself: it's quite the exotic odd one out :)
Don't get tricked by the instanceof
operator, it's not really an instance since it hasn't been created by the Array constructor.
It's just a plain ordinary object. There is no unexpected behavior, it's not an exotic object. Real arrays are exotic objects (spec).
Showing Array
in devtools will just confuse people since they will assume it behaves as a real array exotic object but it doesn't.
but this one very special, very weird use-case should not prevent this from being implemented as other browsers have done.
It's precisely in weird cases when the devtools should be especially useful. In trivial cases they are less needed.
Also, I don't think the proposal is well defined. What if the [[Prototype]] changes? What if the [[Prototype]] doesn't have a constructor
property? What if its value is not a constructor? What if its value changes? What if it's a getter (the devtools are not supposed to call getters with side-effects)?
Comment 11•5 years ago
|
||
It's precisely in weird cases when the devtools should be especially useful. In trivial cases they are less needed.
The general inspection should prioritize that logging most common cases just works (common patterns in frameworks, best practices, etc). We should start documenting edge cases so they can be treated with care. I am not sure if these should block any of the work here though.
Updated•5 years ago
|
Comment 12•5 years ago
•
|
||
Angular classes run into the same issue, reported via https://discourse.mozilla.org/t/firefox-web-console-property-getter/59165
Firefox:
Object { powers: (4) […], model: {…}, submitted: false }
Object { id: 42, name: "", power: "", alterEgo: undefined }
Chrome:
HeroFormComponent {powers: Array(4), model: Hero, submitted: false}
Hero {id: 42, name: "", power: "", alterEgo: undefined}
Comment 13•5 years ago
|
||
Logan, I'd love to hear your thoughts on the scope of fixing this, please. Given the comment 4 and comment 7 – do we need JS team help or is there some intermediate step that makes this work for more cases.
Comment 14•5 years ago
|
||
(In reply to :Harald Kirschner :digitarald from comment #7)
Not sure what would change the prototype, do you have an example of what use case would miss?
Here you have an example:
function Foo(){}
var f = new Foo();
f.constructor.name; // "Foo"
f; // Foo {} in Chromium
Object.defineProperty(Foo, "name", {value: "Bar"});
f.constructor.name; // "Bar"
f; // Foo {} in Chromium
Object.setPrototypeOf(f, null);
f.constructor.name; // TypeError
f; // Foo {} in Chromium
Chromium keeps showing Foo {}
even after changing Foo.name
or f.[[Prototype]]
. So the constructor name is preserved as an invariant, that's good (though I still prefer to see the internal JSClass).
But the proposed instance.constructor.name
utterly fails in these simple cases, without even starting to consider getters with side-effects, proxies, and nastier things.
Comment 15•5 years ago
|
||
Thanks for the additional examples.
Just to assess the trade offs of our options: What's the use case of the ladder 2 examples? Are you aware of frameworks/libraries depending on these patterns?
Comment 16•5 years ago
|
||
I don't know, I don't like frameworks/libraries :)
But if this change is done, I think it should behave consistently as in Chromium.
Implementing something half-broken will end up causing confusion and frustration IMO.
So I would ask the JS team, yes.
Comment 17•5 years ago
|
||
The logic I'd propose would be something along the lines of
const obj = /* ... */;
if (isProxy(obj)) {
return "Proxy";
}
// Default to the object's string tag.
let name = obj[Symbol.toStringTag];
if (typeof name !== "string") {
// Otherwise try to use the constructor name
let constructorFn = Reflect.get(Object.getPrototypeOf(obj), "constructor", obj);
if (constructorFn && constructorFn.name) {
name = constructorFn.name;
}
}
// If it's a builtin, it would probably match the above checks, but you never know if someone
// might have nulled out the object's prototype or something and we can at least try to show
// a useful name in those cases.
if (typeof name !== "string" && isBuiltinType(obj)) {
name = builtinTypeName(obj);
}
Given that 'toStringTag' would then already be a trivial way for user code to change the tag, I don't see a problem with also using obj.constructor.name
directly and using whatever value is there at runtime, but if we want to explore using the original name for the function that's probably also fine, but we'd still just run right into the same issue again because user code can also reassign constructor
itself (changing the constructor also affects the name in Chrome).
Comment 18•5 years ago
|
||
@loganfsmyth Since that proposed logic involves several MOP ops which are “unsafe” even when the obj is not a proxy, I don’t understand why proxies would be special-cased. If the solution must be side-effect free, special-casing proxies makes sense, but in that case, it appears one cannot perform the Get for "constructor" or "name" properties for the same reason (unless it’s implied that there’s some kind of engine level introspection available here to bail in certain cases?).
Comment 19•5 years ago
|
||
I special-cased Proxy
primarily under the assumption that we'd want our preview to make it clear that the object is a proxy and not the actual proxy target.
You're right about side effects though, and that's something we'll have to take into account, both by excluding proxies and by having every property access only work if the object is not a proxy and if the property descriptor is a not an accessor descriptor.
Longer term it's possible that we could use our eager-evaluation backend to support executing sideeffect-free getters, but that would be a larger project.
Comment 20•5 years ago
|
||
Logan, thanks a lot for drafting out how this could be solved. How confident would you be that this approach works for most instances (React, Angular and other common framework patterns)? Is there any other risky assumption to test out or input we need to gather – or would the next step to prioritize this to prototype?
Comment 21•5 years ago
|
||
Talked about this offline. I think what I have would be relatively safe. I think the main thing I'd want to block on is https://bugzilla.mozilla.org/show_bug.cgi?id=1625069 because I'd probably want to perform all of the property accesses I described using instant-eval so that they'd bail if they had sideeffects.
Comment 22•5 years ago
|
||
I still think that showing Array
for a non-array like Object.create(Array.prototype)
or new function Array(){}
will just add confusion.
Maybe, for builtin constructors, their name should only appear if the object is an actual instance. Otherwise show the internal class, or quote the name or something.
Updated•2 years ago
|
Comment 26•2 years ago
|
||
The severity field for this bug is relatively low, S3. However, the bug has 3 duplicates.
:nchevobbe, could you consider increasing the bug severity?
For more information, please visit auto_nag documentation.
Updated•2 years ago
|
Comment 27•2 years ago
|
||
I'm wondering if we could rely on Xraywrappers to retrieve the original constructor name?
When opening these two documents in two tabs:
data:text/html,<script>function Foo(){}; var f = new Foo(); Object.defineProperty(Foo, "name", {value: "Bar"});</script>
data:text/html,<script>function Foo(){}; var f = new Foo(); Object.setPrototypeOf(f, null);</script>
Then, using the Multiprocess Browser Toolbox, pick the content process for these two data: URI tabs and evaluating:
tabs[0].content.wrappedJSObject.f.constructor.name
returns Foo
tabs[1].content.wrappedJSObject.f.constructor.name
returns constructor is undefined
Overriding looks safe thanks to Xrays... but nullifying the prototype isn't.
Note that Cu.unwaiveXrays(tabs[1].content.wrappedJSObject.f).constructor.name
returns Object
which doesn't help.
Same with ChromeUtils.getClassName(tabs[0].content.wrappedJSObject.f)
.
This is based on JS::GetClass(obj)->name
which is most likely equivalent to DebuggerObject.getClassName:
https://searchfox.org/mozilla-central/rev/3b707c8fd7e978eebf24279ee51ccf07895cfbcb/js/src/debugger/Object.cpp#1663-1671
https://searchfox.org/mozilla-central/rev/3b707c8fd7e978eebf24279ee51ccf07895cfbcb/js/src/vm/JSObject-inl.h#533-540
Now I'm wondering if this JS_GetConstructor
method could help:
https://searchfox.org/mozilla-central/rev/3b707c8fd7e978eebf24279ee51ccf07895cfbcb/js/src/jsapi.cpp#1645-1660
But It sounds unlikely as the proto will most likely be undefined or lost.
Comment 29•9 months ago
|
||
Bryan, would you have an idea on how we could handle this?
Comment 30•10 days ago
|
||
BLUF: Using object.constructor?.name
would be a perfectly acceptable fix, and doesn't make users vulnerable to any confusion that a page can't already cause by just setting some_function.displayName = "something confusing"
, because yes, the devtools let you do that. The comments opposing the use of constructor.name
are spectacular bikeshedding, and I'm appalled that for six years, that bikeshedding kept anyone from fixing this problem when it has such a negative impact on UX.
That said, it may be possible to implement a more tamper-proof solution that addresses some of those comments anyway: if the constructor itself is still accessible, then it may be possible to retrieve its original name
, purely from the JS-based devtools and without needing any changes to native code. Alternatively, there may be an even easier solution that doesn't require the constructor at all.
Unfortunately, I'm not familiar enough with the devtools to know precisely how to implement the former solution, nor whether the latter solution exposes the information we need in the situations we need it. I'll go over what I've found, but speaking as an end user, if just displaying obj.constructor.name
is easiest, then please just do that first, file a new issue for implementing a more thorough fix, and then use whatever parts of this comment are helpful to that end. This issue makes Firefox's devtools really awful to use for inspecting complex, deeply nested data structures.
Using the constructor's name
property is fine, actually
To start with: it's already possible to fool the devtools with respect to a function's name, on purpose. Pages can set a displayName
property on certain entities, including functions, to override their usual naming, as documented here:
{
function Foo() {};
Foo.displayName = "Bar";
Foo; // ► function Bar()
}
So frankly, the correct response to comments #8, #14, and #22 is to ignore those concerns entirely and use obj.constructor?.name
anyway, because the devtools are already intentionally designed to let page authors do obfuscatory stuff if they so desire. (Other correct responses would be "Don't let the perfect be the enemy of the good," and "Play stupid games, win stupid prizes. Anyone who writes code like what you've shown us deserves what they get.")
As for comment #10, I'm not even convinced that Chrome's behavior is correct, given that it contradicts nearly everything observable in JS code (e.g. instanceof
checks). But it also doesn't matter. As a primarily C++ dev, I'd argue that changing an object's prototype is a nonsense operation anyway, creating a hybrid that has the per-instance state and properties of the old class, and the inherited properties and other characteristics of the new class; there's no 100% "correct" answer to the question "Which class's name should we display?" and so it's not a question worth bikeshedding or caring about in any way. The devtools can display randomly-generated insults directed at the user's pet cat, for all I care, and I doubt many would feel differently.
If we have the constructor, we can get its true name
It's possible for the devtools to recover a function's true name:
{
function Foo() {};
Object.defineProperty(Foo, "name", {value: "Bar"});
// Foo.name == "Bar" now, and yet:
Foo; // ► function Foo()
}
This works when the function is nested inside of objects as well (e.g. ({ func: Foo, name: Foo.name })
shows the function as Foo()
and the name as "Bar"
). So although the concerns expressed in previous comments are very much worth ignoring, the concerns about the constructor's name can be addressed should the constructor itself still be accessible; and they can be addressed without any edits to native code: the devtools JS should already have what it needs.
How do we get it?
We know that the function name must be accessible to FunctionRep
, the class for "reps" (UI representations) of functions, via at least one of the following values:
props.functionName
grip.userDisplayName
(anydisplayName
property set on the function object)grip.displayName
grip.name
The questions we need to answer are: which field is it, where does the value come from, and how do we get that value in this situation?
I straight-up could not find any setters for functionName
that pertain to the object inspector. I found several relating to stack traces, and the devtools docs mention them in that context as well, so that's probably the only time functionName
is used; we can safely ignore it.
We can see that userDisplayName
is filled in here by the previewer for functions. Remember that fact; it'll come up again later. (This also gives us a guide on how to access page-side properties of an object (objectActor
) that is being debugged.)
The other properties are taken from an ObjectActor. Those are created by ObjectActorPool.createObjectGrip
, which in turn is called by createValueGrip
, which...
...Okay, so I don't remember how I got to what I found next, because it's been hours, but somehow I wandered over to makeDebuggeeValue
, which ends up calling into native code. We eventually call into Debugger::wrapDebuggeeValue
to spawn a JavaScript object that wraps a native DebuggerObject
.
Now as it happens, that class's getter for displayName
requests a function's "display atom." There's also a getter for name
which special-cases bound functions, I assume for stack traces or something. Seems to me like we've found how the devtools are able to ignore a debugged page's shenanigans and show a function's true name.
So now we have our "which" and "where:" if we have an object and we know its constructor isn't Object
, then we can wrap that constructor in a new ObjectActor
; we can then access constructorObjectActor.obj.displayName
to get the constructor's true name. We could also try going the extra mile and getting the user-specified display name, following the example set by the function previewer.
How do we display it?
That leaves the "how." How do we access the constructor in the first place? And presumably we wouldn't want to repeatedly create and discard actors for it, so where do we store the actor we create, so we can reuse it? I assume we can grab the constructor more-or-less the same way the function previewer grabs the user-specified display name; but once we wrap it in an actor, I don't know where to store that. My skill set skews toward C++ and vanilla JS; I don't have hands-on experience with React, and have no clue where the component for displaying an object would want to store the ObjectActor
of that object's constructor.
There's also the matter of what component would even do the grabbing.
It looks like ObjectRep
and GripRep
can both be used to display objects in the devtools. Both of them have a getTitle
function that gets used for displaying the object's class name, falling back to Object
if nothing is retrievable. Both of them support a title
property, but I couldn't find any places where that's actually set. (The way reps are set up means that AFAIK it's basically impossible to, say, search for references to a title
field on some known type. I had to search for all instances of title
in the devtools folder, and I didn't spot anything relevant.) GripRep
on the other hand displays props.title || object.class || "Object"
, where object.class
(as I'll cover in the next section) gives us the name of a SpiderMonkey JSClass
.
If I had to make a blind guess (and I do), I'd say that GripRep
is used for any instances of native non-Object
classes (e.g. Map
, URL
) or any subclasses (native or user-defined) thereof; whereas ObjectRep
is used for any instances of Object
or its non-native subclasses. If that's correct, then ObjectRep
is the component one should modify to resolve this issue.
A possible alternative to relying on the constructor
Object constructors can be cleared. If I do myFoo.constructor = null
, then you'd have to access Object.getPrototypeOf(myFoo).constructor
; and if I null that, then you're screwed. Can we avoid relying on the constructor?
Given an ObjectActor
, I believe you can access myObjectActor.obj.class
to get the class name. This is a JS getter which wraps DebuggerObject::getClassName
to retrieve the name of a JSClass
instance.
I'm not familiar enough with SpiderMonkey to know whether JSClass
es are used for all classes or just native ones. If user-defined classes are indeed JSClass
es, then fixing this issue perfectly could be as easy as just having ObjectRep
return props.title || object.class || DEFAULT_TITLE
in its getTitle
function, the same way GripRep
does. Far, far, far simpler than all the stuff above involving the constructor.
That's all the time and energy I have tonight.
Comment 31•10 days ago
|
||
Using the constructor's
name
property is fine, actually
Well your reasoning would be in support of displayName
rather than name
? But:
displayName
is an own property, so it's more reasonable than depending on the property value of a property value of the prototype object.name
anddisplayName
need to be accessed on the constructor, and AFAIK there is no reliable way to get the constructor.
Play stupid games, win stupid prizes. Anyone who writes code like what you've shown us deserves what they get
It's likely that the person using devtools is not the person who wrote the code. It's not acceptable if the console breaks because of the code, so the console shouldn't blindly invoke getters or proxy traps if that's what you mean.
changing an object's prototype is a nonsense operation anyway
It doesn't matter if you think it's nonsense, it's still part of standard ECMAScript and people use it, so devtools need to handle it properly instead of insulting cats.
If we have the constructor, we can get its true name
AFAIK there is no reliable way to get the constructor.
A possible alternative to relying on the constructor
That's exactly the current behavior: https://searchfox.org/mozilla-central/rev/14f68f084d6a3bc438a3f973ed81d3a4dbab9629/devtools/server/actors/object.js#124,206
This getter seems to expose what ECMAScript 5 called "[[Class]] internal property" (https://262.ecma-international.org/5.1/#sec-8.6.2), hence the name I guess.
Comment 32•9 days ago
|
||
Responding
On changing prototypes
It doesn't matter if you think it's nonsense, it's still part of standard ECMAScript and people use it, so devtools need to handle it properly instead of insulting cats.
You didn't understand the point I was making there, so I'll put it another way.
If you have an instance of A
and you change its prototype to that of B
, then it's not clear whether it'd be more correct to display its class name as A
or B
, because the object is a strange hybrid of both. The object was originally created as an A
and has per-instance state and properties typical of an A
, and on that basis, Chrome displays A
; but the object's current prototype, inherited properties and behaviors, instanceof
behaviors, and nearly everything else observable within JS code all match B
. It would be reasonable to display either A
or B
, and given that this hypothetical scenario is all of the following --
-
A rare operation.
-
An operation that browser implementors have failed to optimize, and that popular reference material (MDN) actively warns against using on that basis.
-
A highly unusual (i.e. "nonsensical") operation in the context of classes and types.[1]
-- it's not worth insisting on the display of either A
or B
specifically, so long as one of them ends up getting displayed absent any other confounding factors. Either one is a reasonable result. And indeed, if we can even semi-reliably access the constructor (B
), then we will display one of those two reasonable results: B
. The proposed fix for this issue would produce a perfectly appropriate behavior here.
On proxies, getters, and failure modes
It's likely that the person using devtools is not the person who wrote the code. It's not acceptable if the console breaks because of the code, so the console shouldn't blindly invoke getters or proxy traps if that's what you mean.
It's possible for devtools to avoid invoking proxy traps and getters. The JS-side object that wraps a native DebuggerObject
has everything we need:
-
There's an
isProxy
getter that we can use to check whether the object is a proxy. It's strange that you didn't notice this, because it's used in the function you literally just linked to. -
You can access a property without invoking getters by querying the property descriptor (i.e.
object.getOwnPropertyDescriptor("constructor")
and checking whether it has avalue
. This is used in theFunction
previewer that I linked above.
This means that we can safely access Object.getPrototypeOf(subject)?.constructor?.name
without invoking proxy handlers or getters, and we can check whether the value is a string. If neither constructor
on the prototype nor name
on the constructor have been overridden, this will get us an accurate constructor name.
There are only two failure modes: we're unable to retrieve a string-type name due to these properties being tampered with (in which case we'll fall back to Object
), or the name
property was set to a different string. In that second rare case, we'd end up displaying the wrong name, but -- and we return, now, to the point I was actually making -- that does not matter. Given a choice between using a solution that works 99.99% of the time and then looking for a better solution for the other 0.01% of cases, or letting a significant UX issue rot with nothing done about it for six years, it should be clear which is the better approach.
Aside: It doesn't matter that a page could make this fix display the wrong text, because pages already have other ways to make the devtools display things that are wrong, and no one can ever fix that
It's already possible to make the devtools display confusing things, both as deliberate features of the devtools themselves, and as a result of web specs. For example, a page could:
-
...Attach
error
andunhandledrejection
event listeners to the window, and have those silentlypreventDefault()
to prevent errors from being logged. -
...Attach
error
andunhandledrejection
event listeners to the window, have those silentlypreventDefault()
to prevent errors from being logged, and then have them generate and log fake errors with text that sounds plausible. You can fake a call stack by just catching an error thrown on some other code path, holding onto it for later, and then re-throw
ing it from yourerror
event listener; both Firefox and Chrome will display the error's original call stack with no indication that it was re-thrown, much less where it was re-thrown from. If you have the devtools set up to pause on uncaught exceptions and you have them open at the time the re-throw happens, then you'll be taken to the re-throw instruction; but if you just see the error in the console after the fact, there's no evidence that you're being messed with. -
...Quietly use the
devtoolsFormatters
API to register a formatter that exactly mimics Firefox's devtools styling, while swapping out some of the displayed values for ones that are slightly wrong.
A page can also just detect when the devtools open, and fire off a setInterval
that spawns and leaks 10MB ArrayBuffers while appending thousands of nodes to the DOM at a time, until the entire tab hangs.
The browser cannot prevent pages from confounding the devtools, including by displaying incorrect data. It cannot prevent shenanigans. It's unreasonable to insist, as you seemed to in the comments I was replying to, that the bar to clear for implementing this feature is "It must be 100% immune to shenanigans, right off rip." The existence and capabilities of the error
event mean that the entire JS console fails to meet that bar.
That's exactly the current behavior:
Thank you for finding that. That's helped clarify a few things:
-
GripRep
is used for objects and is the thing that would need to be modified (so I have no clue whatObjectRep
is used for). -
The
class
property on aDebuggedObject
returns"Object"
for user-defined classes (provided they don't extend some other built-in instead), which means that SpiderMonkey'sJSClass
is only used for native classes and not ones existing in user code. -
When you said in comment 8 that you prefer seeing the
JSClass
name, you were using SpiderMonkey jargon to express that you'd rather people not fix this issue. Would you be amenable to adding a pref to re-enable the behavior you want?
Testing how a fix would work
I don't have a Firefox dev environment set up at this time, so I have to test this the goofy way.
If you open Firefox's hamburger menu, go to More Tools, and open the Browser Toolbox, you can use it to run the devtools on the devtools. Given a test tab that has devtools open, you can use the Browser Toolbox's "Pick an element from the page" button, just to the left of the Inspector tab, to target the devtools in that tab: this will allow you to use the Browser Toolbox's Debugger tab to breakpoint any code within the devtools client scripts.
Breakpointing code in the server scripts (i.e. ObjectActor
) is somewhat trickier: you'll need to make sure the Browser Toolbox is set to Multiprocess mode, and then search the Debugger's sources list for the right file, as the server runs on a separate process. Some files in /devtools/server
do get loaded in the client, but not object.js
which defines ObjectActor
. (Sometimes, your breakpoints in the server script files may also just stop working: they never trip the debugger. It seems like toggling the Browser Toolbox between Parent Process Only and Multiprocess modes, and then refreshing the test tab, can get those breakpoints to work again. Not sure what's up with that.)
You can use breakpoints and watches to tamper with the values of local variables within the debugged devtools instance, because assignment expressions work in watches. This allows us to test whether certain kinds of fixes can work. This is not a substitute for testing a real implementation, but as a quick preliminary test of the implementation I have in mind, it does the job.
I tested by setting breakpoints at the following locations:
-
Here, in
ObjectActor
's constructor, so I can grab and cache the object's constructor name. -
Here, in
ObjectActor.form
, so I can intercept the creation of "grips" that represent objects being debugged. This is the last moment at which it's easy (possible?) to access the nativeDebuggerObject
, with which we can query properties on the object being debugged. The grip is what gets exposed to a rep, so whatever state we want the rep to be aware of has to be set on the grip. -
Here, in the
getTitle
function forGripRep
.
When a breakpoint tripped, I used watch expressions to tamper with these functions' state:
- Within
ObjectActor
's constructor, I ran the following expression (condensed to one line), to grab and store the constructor name onthis.userClassName
. I figure ifDebuggerObject.class
is expensive enough to cache here, then fetching the constructor name would be even more so. (Note thatproto
is a getter onDebuggerObject
.)
this.userClassName = (function(o) {
let proto = o.proto;
if (!proto)
return null;
let cons = proto.getOwnPropertyDescriptor("constructor")?.value;
if (!cons || cons.isProxy)
return null;
let name = cons.getOwnPropertyDescriptor("name")?.value;
if (name && typeof name == "string")
return name;
return null;
})(this.obj);
-
Within
ObjectActor.form
, I ran:g.userClass = this.userClassName
. -
Within the
getTitle
function, I ran a simpler expression:props.title = object.userClass || props.title;
. This had the effect of forcing the rep to prefer the user class name we preserved earlier, by overwriting thetitle
prop. Obviously, it'd be cleaner to actually edit the code and have it check the newobject.userClass
property on the grip, but I don't have a Firefox dev environment set up, so I have to test it like this.
The result: Writing the following code in the console --
{
class Bar {};
new Bar()
}
-- displayed the following preview, when I forced those watches to run at the appropriate breakpoints:
Bar { }
Similarly, executing the following code in the console --
{
class ABC {};
let xyz = new ABC();
let proxy_proto = new Proxy(ABC.prototype, {
get(target, prop, receiver) {
if (prop == "constructor") {
return {
name: "EvilABC",
};
}
return target[prop];
},
});
Object.setPrototypeOf(xyz, proxy_proto);
xyz;
}
-- displayed the following result:
► ABC { }
I'm actually not sure how it's still able to react the prototype despite us putting a proxy in the way. If I have it evaluate xyz.constructor.name
, then we get "EvilABC"
, so we are switching the object's prototype to a proxy. Somehow, we're bypassing the proxy, so let's run a different test.
Instead of the prototype being the proxy, let's turn its constructor
property into a proxy. Evaluating the following code in the console --
{
class ABC {};
let xyz = new ABC();
let original = ABC.prototype.constructor;
ABC.prototype.constructor = new Proxy(original, {
get(target, prop, receiver) {
if (prop == "name") {
return "EvilABC";
}
return target[prop];
},
});
xyz;
}
-- produces the following result:
► Object { }
This explicitly confirms that if we encounter a proxy, we bail out and fall back to the default "Object"
string. (And indeed, as I was running this test, I saw in the debugger that the userClassName
on the ObjectActor
was null
.)
Evaluating the following code in the console --
{
class ABC {};
class DEF {};
let xyz = new ABC();
Object.setPrototypeOf(xyz, DEF.prototype);
xyz;
}
-- displays:
► DEF { }
This is one of the two outcomes for prototype changes that I argued was acceptable, above.
Finally, the following
{
class ABC {};
ABC.prototype.constructor = null;
let xyz = new ABC();
xyz;
}
produces ► Object { }
, so we should be safe from the property just straight-up being nuked, too.
Proposed fix
Modify ObjectActor
's constructor to set this.userClassName
to the object's constructor name, as done in the test described above.
Modify ObjectActor.form
to set g.userClass
as above.
Modify the getTitle
function for GripRep
to return props.title || object.userClass || object.class || DEFAULT_TITLE
.
Endnotes
[1] Swapping out an object's prototype is a reasonable operation within prototype-based programming (in a vacuum), but generally has no analogue in class-based languages (other than abusing UB in C++ to blow away a polymorphic class instance's v-table pointer, I guess). JavaScript is a prototype-based programming language, but it and Lua are the only mainstream prototype languages, and neither became mainstream for being prototype-based. In both of those languages, prototypes are overwhelmingly viewed by programmers as a means to implement class functionality, rather than as being valuable in and of themselves; and JavaScript itself bolstered that view by building class
es off the back of its prototype system, rather than basing them on some separate mechanism. Implementers, meanwhile, have taken basically the same view: changing an object's prototype is not optimized (and arguably not possible to optimize), and this is such a problem that MDN gave it a massive red warning box.
Swapping out the prototype is, therefore, weird as hell, not just generally but even within JavaScript specifically: it is a niche use case and it should be treated like one. The fact that it's part of the standard doesn't prevent it from being niche. The fact of it being niche and weird as hell is what makes it unclear whether it'd be more correct to show the old class name or the new class name; and the fact that that's so unclear is why it's not worth caring which of those two the browser shows.
Comment 33•9 days ago
|
||
Minor addendum: when retrieving the constructor as per this fix, we should verify that it (cons
) is an object rather than a primitive, before trying to access property descriptors on it. I think something like
this.userClassName = (function(o) {
let proto = o.proto;
if (!proto)
return null;
let cons = proto.getOwnPropertyDescriptor("constructor")?.value;
if (!cons || typeof cons != "object" || cons.isProxy) // <---- UPDATED CHECK
return null;
let name = cons.getOwnPropertyDescriptor("name")?.value;
if (name && typeof name == "string")
return name;
return null;
})(this.obj);
would do.
Comment 34•8 days ago
|
||
One last improvement to the proposed fix: instead of accessing the property descriptor on cons
, check if cons.callable
is truthy (to see if it's callable), and if so, grab cons.displayName
. These are the properties on the DebuggerObject
, i.e. a way to see if we're dealing with a function and, if we are, the function's true name. This means that instead of a 99% reliable solution, it'll be a 99.5% reliable solution.
Description
•