1277799 - Define @@toStringTag on DOM objects

Status: RESOLVED FIXED

(Core :: DOM: Bindings (WebIDL), defect, P2)

Description

[Tom S [:evilpie]]

In ES6 Object.prototype.toString doesn't use a [[Class]] meta property anymore, so instead we have to add a Symbol.toStringTag property to all DOM prototype objects.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Note that this will require actually solving the problem of value properties over Xrays in bindings...  We sort of have it for constants right now, but we don't support string-valued constants because rooting and which zone things are in become issues.  So we will need to think about this a bit.  Maybe extend IDL constants to allow a Value or a const char* or something.

Or just special-case this ID in the Xray code.  That might be best, actually.

Comment 2

[Andrew Overholt [:overholt]]

How urgently do we need to do this?

Comment 3

[Tom S [:evilpie]]

I will try to implement @@toStringTag on the JS side in Firefox 50 (probably next week). After we implement this it would be nice to the DOM side as soon as possible, but as long as we keep the fallback code that I wrote everything should just keep working. (Object.prototype.toString on DOM object keeps working, but if some super insane new code expects DOM objects to have @@toStringTag that would fail)
It would be nice to finish the DOM side in the same release, especially because of Promises as they are still implemented there.

Comment 4

[Boris Zbarsky [:bzbarsky]]

> It would be nice to finish the DOM side in the same release

The problem is that we still haven't decided what the DOM side behavior should be.  :(

Comment 5

[Tom S [:evilpie]]

This needs to be resolved at some point. Is this being discussed at whatwg or somewhere? If we want to self-host Object.prototype.toString (bug 1346546) we don't want to call into C++ to get the class name for all DOM objects.

Comment 6

[Boris Zbarsky [:bzbarsky]]

In fairness, we could have a builtin, even inlinable, for getting the class name.  I agree it would be annoying.

This is not being actively discussed.  The current state is that Chrome ships one behavior, everyone else ships a different one, all the non-Chrome behaviors are not using @@toStringTag afaict, and Apple/Microsoft are not talking about this at all.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Bug 1424160 is going to add some infrastructure for doing this.

Comment 8

[Tom S [:evilpie]]

Does that mean this is going to be fixed?

Comment 9

[Boris Zbarsky [:bzbarsky]]

Well, so far I've made it somewhat easier to fix by doing various webidl parser and codegen legwork needed and using it for the generated webidl iterator prototypes.

Remaining work here:

1)  Set a toStringTag on all interfaces, not just the iterator interfaces.  This is basically one
    line of code in the webidl parser when creating the IDLInterface object.
2)  Probably need to fix Xrays to expose the @@toStringTag.  I didn't do that in bug 1424160
    because it didn't matter too much.

The big question, still, is whether we want the resulting behavior....

Comment 11

[Jason Orendorff [:jorendorff]]

The WebIDL spec seems to require toStringTag properties on everything. The wording has been there since 2014.

Comment 12

[Boris Zbarsky [:bzbarsky]]

Yes, that's not really viable and no one implements that or plans to. See https://www.w3.org/Bugs/Public/show_bug.cgi?id=28244 and https://github.com/heycam/webidl/pull/357 for discussion.

Comment 13

[Tom S [:evilpie]]

It seems like the currently plan is to put Symbol.toStringTag only on the prototype object. Seems like we can go ahead and implement this. I am not sure if this enough to actually remove the current non-standard fallback (Using the old and busted [[Class]]). I suspect we still might need it for XPIDL or something like that ...

Comment 14

[Tom S [:evilpie]]

(In reply to Boris Zbarsky [:bzbarsky] from comment #9)

Well, so far I've made it somewhat easier to fix by doing various webidl
parser and codegen legwork needed and using it for the generated webidl
iterator prototypes.

Remaining work here:

1. Set a toStringTag on all interfaces, not just the iterator interfaces.
   This is basically one line of code in the webidl parser when creating the IDLInterface object.

Who can point out this one line code change? It's not obvious to me.

2. Probably need to fix Xrays to expose the @@toStringTag. I didn't do
   that in bug 1424160 because it didn't matter too much.

I though we already supported that, but I guess I can probably fix this. I understand Xrays well enough probably.

Comment 15

[Timothy Gu [:TimothyGu]]

(In reply to Tom Schuster [:evilpie] from comment #14)

(In reply to Boris Zbarsky [:bzbarsky] from comment #9)

Remaining work here:

1. Set a toStringTag on all interfaces, not just the iterator interfaces.
   This is basically one line of code in the webidl parser when creating the IDLInterface object.

Who can point out this one line code change? It's not obvious to me.

Looks like it's this: https://searchfox.org/mozilla-central/rev/41c3ea3ee8eab9ce7b82932257cb80b703cbba67/dom/bindings/parser/WebIDL.py#1614

IDLInterface objects generally get constructed in handleNonPartialObject. Changing the default toStringTag should suffice. However, note that iterator interfaces are created with an overridden toStringTag here.

Comment 16

[Tom S [:evilpie]]

Thanks, I think I've got it. I suspect this is probably not totally what you had in mind, but it seems like we don't really need toStringTag: https://treeherder.mozilla.org/#/jobs?repo=try&revision=701c1e627eb11e0016c0eb84602c6d6adc59a29d

Comment 17

[Timothy Gu [:TimothyGu]]

Note that Object.getPrototypeOf(Window.prototype) also need a @@toStringTag with the value "WindowProperties", since otherwise now it inherits the "EventTarget" tag.

Comment 18

[Tom S [:evilpie]]

(In reply to Timothy Gu [:TimothyGu] from comment #17)

Note that Object.getPrototypeOf(Window.prototype) also need a @@toStringTag with the value "WindowProperties", since otherwise now it inherits the "EventTarget" tag.

Thank you! I had wondered about some failure in the try run related to this. It seems like we use a custom proxy for WindowProperties, so we need to make some additional changes there.

Comment 19

[Tom S [:evilpie]]

Attachment: Bug 1277799 - Define @@toStringTag on all DOM interface prototype objects. r?peterv

Comment 21

[Tom S [:evilpie]]

I am going to wait for https://github.com/web-platform-tests/wpt/pull/23140 to merge so I don't have to adjust tests twice.

Comment 22

[Tom S [:evilpie]]

Attachment: Bug 1277799 - Implent Xray support for @@toStringTag on DOM interface prototype objects.

Depends on D72179

Comment 23

[Tom S [:evilpie]]

Attachment: Bug 1277799 - Define @@toStringTag on all DOM interface prototype objects. (2nd Try)

Instead of manually defining toStringTag we now add the toStringTag symbol to the list of properties.
This is also how we usually define toStringTag in the JS engine.
Even though this changes more code I like this approach better. Everything is centralized in the generated bindings file.

I haven't added the required WindowProperties changes to this patch yet.

Comment 24

[Tom S [:evilpie]]

Looking good: https://treeherder.mozilla.org/#/jobs?repo=try&revision=eedea8f6480ffe9311a1d8a4baf8398328cc4dd2

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8e5e6fd199d3
Define @@toStringTag on all DOM interface prototype objects. r=peterv

Comment 26

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/8e5e6fd199d3

Comment 27

[Chris Mills [:cmills]]

:evilpie — I'm currently looking into documenting this on MDN, but I'm not sure I really understand what needs to be done. What web developer documentation would you like to see written about this?

Comment 28

[Tom S [:evilpie]]

(In reply to Chris Mills (Mozilla, MDN editor) [:cmills] from comment #27)

:evilpie — I'm currently looking into documenting this on MDN, but I'm not sure I really understand what needs to be done. What web developer documentation would you like to see written about this?

Good question! If we wanted to closely follow the standard we would have to add a Symbol.toStringTag property to almost every DOM interface on MDN. (This mostly means those with a prototype). The only real exception are the very very few namespace objects like CSS or console. I guess it's on the MDN team to decide if they want to add this mostly trivial documentation. HTMLBodyElement.prototype has a Symbol.toStringTag withe a value of "HTMLBodyElement", Blob.prototype[Symbol.toStringTag] is "Blob" etc. Maybe there is some better place for this sort of very "general" documentation. New in Firefox 78 is a given of course.

Comment 29

[Chris Mills [:cmills]]

Thanks :evilpie !

I've written up some general thoughts on this at https://github.com/mdn/sprints/issues/3292#issuecomment-634816769. Short version is, I think we are just gonna follow what you are suggesting about documenting it in a single place, and then we'll also add a rel note to the rel notes page.

Comment 30

[Chris Mills [:cmills]]

OK, I think I've documented this sufficiently; see https://github.com/mdn/sprints/issues/3292#issuecomment-638988623 for the full details.

Let me know if you need anything else. Thanks!