w3c / webrtc-extensions

A repository for experimental additions to the WebRTC API
https://w3c.github.io/webrtc-extensions/
Other
58 stars 19 forks source link

"return undefined" when populating a dictionary member is unclear #34

Open foolip opened 4 years ago

foolip commented 4 years ago

"Otherwise, return undefined" is used in two places: https://w3c.github.io/webrtc-extensions/#dom-rtcrtpcontributingsource-capturetimestamp https://w3c.github.io/webrtc-extensions/#dom-rtcrtpcontributingsource-sendercapturetimeoffset

If one takes this literally, one would create an own property with the value undefined. However, the intended behavior is almost certainly to not add the member at all.

Aside: undefined is a JS type/value, where void is the IDL type. The IDL type "can only be used as the return type of an operation or the parameter of a promise type" (https://heycam.github.io/webidl/#idl-void) so there actually isn't a way to do express the IDL dictionary equivalent of { something: undefined }.

foolip commented 4 years ago

@henbos

henbos commented 4 years ago

Ah you're right, this should say "return null;". We need to make it nullable.

henbos commented 4 years ago

Intent-wise this is an editorial issue, but getting the spec to reflect the intent is not editorial. But I expect being able to update spec to not get any pushback.

henbos commented 4 years ago

Hang on, why isn't undefined allowed? Dictionary attributes are by default not required, so passing in something with undefined is OK... but if this object was to be returned, are you saying that it is not allowed to return undefined, but one has to return null if the browser has the attributes in the WebIDL?

Is this just one of those rules I should accept as a WebIDL quirk?

alvestrand commented 4 years ago

I think the point is that "this dictionary doesn't have a value for this attribute, so looking it up will return undefined" is different spec-wise than "this dictionary has a member, and its value is "undefined"" (and the latter language is forbidden by the rule quoted above).

I think the non-return of the non-value for the non-present attribute has to be pushed up to the function that constructs the dictionary.

henbos commented 4 years ago

Ooooh...

alvestrand commented 4 years ago

@foolip, would "Otherwise, this member will not be populated" work?

henbos commented 4 years ago

So if it is clarified that the dictionary is constructed with or without this value, this is OK? (And JavaScript-wise you would see undefined.) It doesn't need to be made nullable?

foolip commented 4 years ago

Having things be both nullable and optional is a bit redundant, so I'd stick with just optional.

"Otherwise, this member will not be populated" makes it clear what should happen, but I'm not sure if there's a standard incantation for this that other specs use.

https://drafts.fxtf.org/geometry/#dommatrixinit-dictionary has some example language for how to set dictionary members. If this was all steps of an algorithm, then those steps would simply be inside conditionals and there would be "otherwise don't set it" step.

henbos commented 4 years ago

OK, I'll think of some sensible way to express it. But then this is a purely editorial issue. It will remain optional, as originally intended.

foolip commented 4 years ago

Yep, thanks for fixing!