Open Bug 1129201 Opened 9 years ago Updated 2 years ago

PerformanceResourceTiming objects' toJSON is enumerable in FF35+

Categories

(Core :: DOM: Core & HTML, defect, P5)

35 Branch
x86
macOS
defect

Tracking

()

UNCONFIRMED

People

(Reporter: e.a.nakashima, Unassigned)

Details

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_2) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/40.0.2214.94 Safari/537.36

Steps to reproduce:

Working with resource timings in Firefox 35-37, I noticed that they have `toJSON` as an enumerable property.

This can make it tricky to collect resource timings in a way that's also compatible with Firefox < 34, because before PerformanceResourceTiming objects supported `JSON.stringify`(#1047848), a possible workaround was to copy all of their properties over to a plain Object and stringify that. In Firefox 35, that workaround creates objects with a toJSON property that throws `TypeError: 'toJSON' called on an object that does not implement interface PerformanceResourceTiming` when you attempt to stringify them.

It doesn't exactly seem like a bug, but it's a bit surprising since `toJSON` isn't part of the interface per the spec. http://www.w3.org/TR/resource-timing/#resources-included It's also inconsistent with Chrome 40 & IE 11, where PerformanceResourceTiming objects either don't have .toJSON or it's not enumerable. 

Test code:

// Logs "toJSON" in Firefox 35
var resources = window.performance.getEntriesByType('resource');

for (prop in resources[0]) {
  if (prop === 'toJSON') console.log(prop);
}


Actual results:

Code example logs "toJSON" in Firefox 35, 36, 37


Expected results:

Ideally it'd be great to have PerformanceResourceTiming objects support .toJSON but not have it as an enumerable property. (i.e. code example shouldn't log anything)
Component: Untriaged → DOM
Product: Firefox → Core
The spec has just been updated with a serializer.  See https://w3c.github.io/resource-timing/#performanceresourcetiming

Per http://heycam.github.io/webidl/#es-serializer the toJSON that a serializer produces is enumerable.  So what we have right now matches the current spec.

But maybe the toJSON in the spec should not be enumerable?  Cameron?  The only toJSON in ES6 is non-enumerable, but that's because _everything_ on the standard prototypes in ES6 is non-enumerable...  Note that this is similar to https://www.w3.org/Bugs/Public/show_bug.cgi?id=26179

On the third hand, changing behavior yet again just means people now need code to cope with _three_ behaviors.  If they're worried about Firefox 34 compat, that is; if they aren't they can just JSON.stringify the PerformanceResourceTiming directly and be done with it...

The real question is whether serializers should produce an enumerable toJSON in general.
Flags: needinfo?(cam)
I don't know that we have many uses of serializer yet -- is this the first?

It's not a common pattern to rely on an object enumerating its properties in a particular way as part of its API design, which is what it sounds like PerformanceResourceTiming is doing here.  For this uncommon pattern, perhaps we should have a way of inhibiting all properties from being enumerated on it apart from certain ones (by default, IDL attributes, or perhaps only those explicitly indicated in the IDL)?

You could make the argument that toJSON and toString should be non-enumerable as they are sort of exposing services to other parts of ES, unlike random other IDL operations on an interface.  But I can't see a reason why in the future someone may want to put a method on PerformanceResourceTiming objects, in which case just special casing toJSON/toString for non-enumerability doesn't make sense.
Flags: needinfo?(cam)
So I don't think in general toJSON should be non-enumerable, unless we decide to do that for toString as well.  I also don't think their non-enumerability on built-in objects in ES6 affects anything, since we already have a split of "things defined in IDL are enumerable, things defined in ES6 are not".
> I don't know that we have many uses of serializer yet

We totally do.  We call it "jsonifier" in our IDL, because we only implement a small subset of the "serializer" bits in the spec and have no plans to do more.  See http://mxr.mozilla.org/mozilla-central/search?string=jsonifier&find=webidl&tree=mozilla-central

> which is what it sounds like PerformanceResourceTiming is doing here

No, that's just consumers of this interface who are trying to hack in a serializer in browsers that don't support one.  The interface itself doesn't rely on enumeration in any way.

Note, also, that ES6 classes have decided to have everything be non-enumerable by default, so we may want to start doing that for IDL objects maybe...
https://bugzilla.mozilla.org/show_bug.cgi?id=1472046

Move all DOM bugs that haven’t been updated in more than 3 years and has no one currently assigned to P5.

If you have questions, please contact :mdaly.
Priority: -- → P5
Component: DOM → DOM: Core & HTML
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.