Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Webidl fixes #143

Closed
wants to merge 2 commits into from
Closed

Webidl fixes #143

wants to merge 2 commits into from

Conversation

dontcallmedom
Copy link
Member

No description provided.

dontcallmedom added a commit to dontcallmedom/mediacapture-main that referenced this pull request Mar 10, 2015
known to the User Agent for the kind given as argument. A
supported constrainable property MUST be represented by a member whose name is
the constraint name and whose value is <code>true</code>. Any
the constraint name and whose value is truthy. Any
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we nail this down to {} (with a note that it's truthy, and note that future specs may choose to return other values)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can't use {} if the type is MediaTrackSettings, because the members of that dictionary are simple values, not objects.

If we adopt Jan-Ivar's suggestion of using MediaTrackConstrainttSet instead, then we can and should use the more specific {}.

@burnburn
Copy link
Contributor

As you can imagine, I do like the idea of returning an object but do NOT like the idea of re-using MediaTrackConstraintSet. As I've said before, if we are going to go to the trouble of defining dictionaries using WebID in order to provide some sort of contract for the objects, then we should define the proper one. Without knowing what, if anything we might want to return in the future here, I'd rather define a new type that only allows boolean values to be returned for each supported constrainable property. Or, alternatively, define a new type that allows any simple or object value to be returned, without saying anything about the object's structure but defining that truthy values (including {}) mean that the constrainable property is supported.

dontcallmedom added a commit to dontcallmedom/mediacapture-main that referenced this pull request Mar 17, 2015
@dontcallmedom
Copy link
Member Author

I think having a 3rd interface to manage constraints is a path leading to maintenance nightmares, but since it doesn't affect interoperability (at least in the short term), let's go with this for now (as proposed in #146).

@jan-ivar
Copy link
Member

I'm happy we're getting explicit WebIDL types, but I agree with @dontcallmedom . For implementors, having four places to maintain and update (Supported, Settings, Capabilities and Constraints) for each new constraint added, is a cost. For readers, the syntactic symmetry and relationship between four seemingly discrete types is buried in prose, a missed opportunity for clarity through types IMHO. I can't think of an API where I've seen member-sets transcend types like this, so it may in fact be unprecedented as well. We may have cut the cake wrong somewhere when similar member-sets are not even rooted in a shared base type.

The spec is full of assumptions that constraints and settings are related in a member-by-member enumeration: " If the constraint is required ('min', 'max', or 'exact'), and the settings dictionary's value for the constraint does not satisfy the constraint, use positive infinity." - but oddly, I can't find a definition of "satisfied" anywhere.

The discrete typing seems especially egregious for getSupportedConstraints(), an API that literally tells you what members the dictionary MediaTrackConstraintSet supports! Seeing a member in the returned dictionary would inherently prove that MediaTrackConstraintSet supports it, provided it actually returned a MediaTrackConstraintSet dictionary. This seems so intuitive that I'm having a hard time explaining why a different type makes no sense.

@adam-be
Copy link
Member

adam-be commented Mar 18, 2015

The current situation with 3 different objects with the same dictionary member names (but with different types) is far from ideal. But, I don't think it gets much worse by adding a fourth, and it would be more in line in the direction we have taken so far.

So if we want to start reusing structures, we should reuse structures all over and not only for this fourth case.

@jan-ivar
Copy link
Member

@adam-be are you saying the direction we've taken so far is taking us farther from ideal?

Being away from ideal is not an argument for moving away from ideal, rather an argument for moving toward it.

@adam-be
Copy link
Member

adam-be commented Mar 18, 2015

My comment was a bit exaggerated and sounded more negative that I indended. What I meant to say was that we currently have three objects that needs to be updated when a new constraint is added. That's not ideal, but the cost when we use "tailored" objects for each type of usage.

@jan-ivar
Copy link
Member

+1 on reusing dictionaries all over.

@fluffy
Copy link
Contributor

fluffy commented Mar 19, 2015

took other option

@fluffy fluffy closed this Mar 19, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants