Re: some comments on the IDL in the WebRTC spec

thank you kindly for these - we need to get these fixes into the draft

On Jun 21, 2012, at 1:22 , 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 Friday, 22 June 2012 01:05:15 UTC