- From: Anant Narayanan <anant@mozilla.com>
- Date: Thu, 16 Aug 2012 11:22:57 -0700
- To: public-webrtc@w3.org
Cameron, Thanks for the comments! Most of them have been incorporated into the latest ED. Regards, -Anant On 06/20/2012 10:22 PM, Cameron McCormack wrote: > Hi, > > Below are some comments on the IDL currently in the spec after a brief > look. I haven't reviewed any prose. > > > 4.1.1 SdpType > 4.1.3 SessionDescriptionCallback > 4.1.4 PeerConnectionErrorCallback > 4.1.5 PeerState Enum > 4.1.6 IceState Enum > 4.1.7 IceCandidate Type > 4.1.8 IceCandidateCallback > > The definitions in these sections are all missing semicolons at the end. > > > 4.1.2 SessionDescription Class > > If you want, you can just write "stringifier;" rather than "stringifier > DOMString ();", since they are equivalent. > > Why do you want these to be stringifiable, by the way? > > > 4.1.9 IceServers Type - Option 1 > > The attribute needs to be written as: > > attribute DOMString[][] servers; > > Similarly for 4.1.10 (square brackets as part of the type). > > > 4.1.11 PeerConnection interface > > s/Boolean/boolean/g in the IDL block. > > If you specify default values for optional arguments, then all preceding > optional arguments must also have default values. So I would either > rewrite createAnswer to: > > void createAnswer > (SessionDescription offer, > SessionDescriptionCallback successCallback, > optional PeerConnectionErrorCallback? failureCallback = null, > optional MediaConstraints? constraints = null, > optional boolean createProvisionalAnswer = false); > > or > > void createAnswer > (SessionDescription offer, > SessionDescriptionCallback successCallback, > optional PeerConnectionErrorCallback failureCallback, > optional MediaConstraints constraints, > optional boolean createProvisionalAnswer); > > and just describe the behaviour when createProvisionalAnswer is not > specified in prose. Similarly for updateIce. > > You can't use [TreatNullAs=EmptyString] on a type that already accepts > null, like createDataChannel's label argument does. Either remove the > "?" from the type or drop the [TreatNullAs=EmptyString] and describe in > prose what happens when you pass null. > > Please put [TreatNonCallableAsNull] on all of the event listener > attributes, like you do for DataChannel. We want the behaviour of all > event listener attributes across the platform to be consistent. (There > should hopefully be a simpler definition to reference at some point in > the future that won't look as awkward as "[TreatNonCallableAsNull] > attribute Function?".) > > > 8.1 DataChannel > > I recommend using enums instead of integer constants. I see there is a > PeerState enum defined earlier in the document -- can you use this instead? > > It might be good to support all the same kinds of types being passed to > send() that XHR allows. > > > 10.3 MediaStreamTrackEvent > > The "readonly" should be removed from the dictionary member. > > It seems like the attribute on MediaStreamTrackEvent should be of type > "MediaStreamTrack?" rather than "MediaStreamTrack", if you can use null > in the event initialisation dictionary. (Similarly for MediaStreamEvent > in the following section.) > > >
Received on Thursday, 16 August 2012 18:23:26 UTC