Closed Bug 1277799 Opened 8 years ago Closed 5 years ago

Define @@toStringTag on DOM objects

Categories

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

Product:
Core
Component:
DOM: Bindings (WebIDL)
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla78
Tracking Flags:
Tracking Status
firefox78 --- fixed

People

(Reporter: evilpie, Assigned: evilpie)

References

(
URL
)

Details

(Keywords: dev-doc-complete, parity-chrome, parity-safari, Whiteboard: btpp-followup-2016-07-05)

Attachments

(1 file, 2 obsolete files)

Bug 1277799 - Define @@toStringTag on all DOM interface prototype objects. r?peterv
(deleted), text/x-phabricator-request
Details
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.
Blocks: 1277801
Depends on: 1277880
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.
How urgently do we need to do this?
Flags: needinfo?(evilpies)
Whiteboard: btpp-followup-2016-07-05
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.
Flags: needinfo?(evilpies)
> 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. :(
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.
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.
Priority: -- → P2
Bug 1424160 is going to add some infrastructure for doing this.
Does that mean this is going to be fixed?
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....
Component: DOM → DOM: Core & HTML
Blocks: 1567699

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

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.

Component: DOM: Core & HTML → DOM: Bindings (WebIDL)

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

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

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

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

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

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

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

Assignee: nobody → evilpies

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.

Depends on D72179

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.

Attachment #9142771 - Attachment description: Bug 1277799 - Define @@toStringTag on all DOM interface prototype objects. → Bug 1277799 - Define @@toStringTag on all DOM interface prototype objects. (2nd Try)
Attachment #9142888 - Attachment is obsolete: true
Attachment #9143040 - Attachment is obsolete: true
Attachment #9142771 - Attachment description: Bug 1277799 - Define @@toStringTag on all DOM interface prototype objects. (2nd Try) → Bug 1277799 - Define @@toStringTag on all DOM interface prototype objects. r?peterv
Pushed by evilpies@gmail.com: https://hg.mozilla.org/integration/autoland/rev/8e5e6fd199d3 Define @@toStringTag on all DOM interface prototype objects. r=peterv

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

Status: NEW → RESOLVED
Closed: 5 years ago
status-firefox78: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla78
Blocks: 1635582
Regressions: 1636382

: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?

Flags: needinfo?(evilpies)

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

Flags: needinfo?(evilpies)

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.

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!

Keywords: dev-doc-needed → dev-doc-complete
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: