Firefox: do not attempt to modify read-only property in ice candidate shim #1123
Conversation
|
Sadly the patch doesn't apply clean to v7 or v6 (that one is still ES5) but it is easy enough to backport. |
|
|
||
| // Add a serializer that does not serialize the extra attributes. | ||
| augmentedCandidate.toJSON = function toJSON() { | ||
| // Override serializer to not serialize the extra attributes. |
There was a problem hiding this comment.
Hmm, this should be unnecessary, since enumerable defaults to false. Did it not work in ES5? If the default changed, does the following help?
Object.defineProperty(nativeCandidate, key, {
value: parsedCandidate[key],
enumerable: false
});There was a problem hiding this comment.
If anything, my pilot error was caused by the custom toJSON on the native object persisting, causing me to mistakenly think the properties hadn't been copied, because I was using JSON.stringify to dump things to console in my first fiddle attempts, and even new enumerable properties weren't showing up for me.
There was a problem hiding this comment.
To clarify, I believe even with enumerable: true I would not expect new properties to appear in JSON, because of the native toJSON for RTCIceCandidate is already custom and limited.
There was a problem hiding this comment.
True but this code still has to deal with the theoretical possibility of Chromes native toJSON predating the right thing
There was a problem hiding this comment.
Oh I missed that this was fixing up existing code. LGTM!
|
|
||
| // Add a serializer that does not serialize the extra attributes. | ||
| augmentedCandidate.toJSON = function toJSON() { | ||
| // Override serializer to not serialize the extra attributes. |
There was a problem hiding this comment.
Oh I missed that this was fixing up existing code. LGTM!
fixes #1122