RTCIceCandidate constructor shim throwing error if ufrag given for Firefox beta (111)

[paisl3y]

Please read first!

Please use discuss-webrtc for general technical discussions and questions.

• [ x] I have provided steps to reproduce (e.g. a link to a jsfiddle)
• [ x] I have provided browser name, version and adapter.js version
• [ x] This issue only happens when adapter.js is used

Note: If the checkboxes above are not checked (which you do after the issue is posted), the issue will be closed.

Versions affected

Firefox (Beta) version 111.0b3 (64-bit)
[email protected]

Description

RTCIceCandidate() constructor throws the below error when ufrag supplied on the candidate line.

TypeError: setting getter-only property "usernameFragment"
RTCIceCandidate common_shim.js:35
debugger eval code:1

Error is not seen in current stable Firefox (v110). No error is thrown if ufrag is not given.

Steps to reproduce

Try creating RTCIceCandidate using the following candidateInfo:

new RTCIceCandidate({ candidate: 'candidate:4234997325 1 udp 2043278322 192.168.0.56 44323 typ host generation 0 ufrag iTuJ network-id 1', sdpMid: 0 })

Expected results

Newly created RTCIceCandidate object returned.

Actual results

The following error is thrown:
TypeError: setting getter-only property "usernameFragment"
RTCIceCandidate common_shim.js:35
debugger eval code:1

[fippo]

@jan-ivar Firefox breaking like this doesn't sound good? Past attempts by Chromium to enforce read-onlyness of attributes (in particular RTCSessionDescription's sdp) turned out to break too much for no gain.

[fippo]

Related Firefox bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1812346

[jan-ivar]

The bug is in adapter.js here:

  const nativeCandidate = new NativeRTCIceCandidate(args);
  const parsedCandidate = SDPUtils.parseCandidate(args.candidate);
→ const augmentedCandidate = Object.assign(nativeCandidate, parsedCandidate); // TypeError

TIL the difference between:

1. assignment to a read-only attribute, which is ignored and doesn't throw, vs.
2. Object.assign which does: "In case of an error, for example if a property is non-writable, a TypeError is raised".

This fiddle shows the latter throwing in all browsers including Firefox 111+, so unfortunately this shim, which was specific to Firefox, was relying on non-standard behavior of Firefox 110 and earlier.

We'll back out the fix that broke adapter in Firefox 111 beta for now, to give adapter time to fix this, and for all users of adapter to update (which may be a while).

[fippo]

The fix is going to say

const nativeCandidate = new NativeRTCIceCandidate(args);
const parsedCandidate = SDPUtils.parseCandidate(args.candidate);
const augmentedCandidate = Object.assign({}, nativeCandidate, parsedCandidate);

right?

Adoption is going to be a problem, there is still a lot of usage on version 7 and not even 8.
You might consider not doing this until you implement attribute parsing, in particular foundation which is what prevents this from happening in Chrome.

[jan-ivar]

That would return the wrong type I fear. [Deleted: pilot error since I forgot RTCIceCandidate has a custom toJSON]

Agreed about adoption.

[fippo]

Thank you for backing this out @jan-ivar! I agree with the plan to reland we lets still try to get #1123 out