Closed Bug 834463 Opened 7 years ago Closed 7 years ago

Incorrect RTCConfiguration format

Categories

(Core :: WebRTC: Networking, defect, P1, critical)

defect

Tracking

()

VERIFIED FIXED
mozilla21

People

(Reporter: jib, Assigned: jib)

Details

(Whiteboard: [WebRTC], [blocking-webrtc+])

Attachments

(1 file)

Bug 825703 expects the following arg in RTCPeerConnection(arg):

  [ { url:"stun:23.21.150.121" },
    { url:"turn:user@turn.example.org", credential:"mypass"} ]

when it should expect:

  { "iceServers": [ { url:"stun:23.21.150.121" },
                    { url:"turn:user@turn.example.org", credential:"mypass"} ] }

according to http://dev.w3.org/2011/webrtc/editor/webrtc.html#rtcconfiguration-type
Status: NEW → ASSIGNED
Attachment #706103 - Flags: review?(ekr)
Attachment #706103 - Flags: review?(bzbarsky)
Comment on attachment 706103 [details] [diff] [review]
Corrected RTCConfiguration format.

Review of attachment 706103 [details] [diff] [review]:
-----------------------------------------------------------------

lgtm and works in my test harness
Attachment #706103 - Flags: review?(ekr) → review+
Comment on attachment 706103 [details] [diff] [review]
Corrected RTCConfiguration format.

r=jst
Attachment #706103 - Flags: review?(bzbarsky) → review+
Attachment #706103 - Flags: checkin?(rjesup)
https://hg.mozilla.org/mozilla-central/rev/290d4a86ea19
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
For what it's worth, the new setup looks an awful lot like this:

  dictionary RTCConfiguration {
    sequence<RTCIceServer> iceServers;
  };

(I know the spec has RTCIceServer[]; I think that's buggy and whoever cares about the spec should raise it as a spec issue).

In any case, it seems like you could just declare the above in WebIDL, then use this dictionary in the C++ like you already do with RTCIceServer, and drop more JSAPI goop here.  Perhaps a followup?
Keywords: verifyme
Comment on attachment 706103 [details] [diff] [review]
Corrected RTCConfiguration format.

Forgot to clear checkin? flag
Attachment #706103 - Flags: checkin?(rjesup) → checkin+
Status: RESOLVED → VERIFIED
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.