Open Bug 1526688 Opened 6 years ago Updated 8 days ago

Show instance constructor name in console instead of "Object"

Categories

(DevTools :: Console, defect, P2)

65 Branch
defect

Tracking

(Not tracked)

People

(Reporter: jelle, Unassigned, NeedInfo)

References

(Blocks 1 open bug)

Details

(Whiteboard: [dt-q])

Attachments

(1 file)

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.

Setting the component, in order to involve the development team.

Component: Untriaged → Console
Product: Firefox → DevTools
Priority: -- → P3
Status: UNCONFIRMED → NEW
Ever confirmed: true

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/

Priority: P3 → P2
See Also: → 1562463

Brian, is this something we need better server-side/JS engine support for, or is it frontend (or sth else)?

Flags: needinfo?(bhackett1024)

(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.

Flags: needinfo?(bhackett1024)

Couldn't ES6's instance.constructor.name get is halfway there?

function MyClass() {};
console.log((new MyClass()).constructor.name);
Flags: needinfo?(bhackett1024)

(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.

Flags: needinfo?(bhackett1024)

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?

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.

(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 see Object and not Array.

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.

(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)?

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.

Whiteboard: [dt-q]

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}

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.

Flags: needinfo?(loganfsmyth)

(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.

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?

Flags: needinfo?(oriol-bugzilla)

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.

Flags: needinfo?(oriol-bugzilla)

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).

Flags: needinfo?(loganfsmyth)

@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?).

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.

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?

Flags: needinfo?(loganfsmyth)

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.

Flags: needinfo?(loganfsmyth)

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.

Severity: normal → S3

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.

Flags: needinfo?(nchevobbe)
Flags: needinfo?(nchevobbe)

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.

Duplicate of this bug: 1898871

Bryan, would you have an idea on how we could handle this?

Flags: needinfo?(bthrall)
See Also: → 1908650

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 (any displayName 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 JSClasses are used for all classes or just native ones. If user-defined classes are indeed JSClasses, 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.

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 and displayName 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.

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 a value. This is used in the Function 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 and unhandledrejection event listeners to the window, and have those silently preventDefault() to prevent errors from being logged.

  • ...Attach error and unhandledrejection event listeners to the window, have those silently preventDefault() 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-throwing it from your error 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 what ObjectRep is used for).

  • The class property on a DebuggedObject returns "Object" for user-defined classes (provided they don't extend some other built-in instead), which means that SpiderMonkey's JSClass 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 native DebuggerObject, 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 for GripRep.

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 on this.userClassName. I figure if DebuggerObject.class is expensive enough to cache here, then fetching the constructor name would be even more so. (Note that proto is a getter on DebuggerObject.)
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 the title prop. Obviously, it'd be cleaner to actually edit the code and have it check the new object.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 classes 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.

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.

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.

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: