Don't fire icecandidatepairadd if closed + avoid competing normative prose #177
Conversation
|
LGTM, makes sense to guard the event. Thanks! |
| @@ -717,6 +721,41 @@ <h3> | |||
| Changing | |||
| the selected candidate pair after a successful nomination requires an ICE restart. | |||
| </p> | |||
| <p> | |||
| When the [= ICE agent =] has [= formed =] a candidate pair, the [= user agent =] MUST [= queue a task =] to <dfn | |||
There was a problem hiding this comment.
I think it's a little vague when a candidate pair is "formed". So I think it might be better to phrases this in terms of the ICE agent is going to add a candidate pair, and just avoid the use of the term "form".
There was a problem hiding this comment.
I take it you are referring to Section 6.1.2.2, where candidate pairs are "formed", but some of these can be discarded in sections 6.1.2.3, 6.1.2.4 and 6.1.2.5 before any connectivity checks are ever sent.
Only at the end of these steps does the ICE agent have a list of "added" candidate pairs on which connectivity checks could be sent, and only these bear surfacing up to the application. RFC 8445 does not have any terminology for candidate pairs that get to this stage.
So perhaps the definition in terminology could be rephrased as:
"The process of adding candidate pairs is defined as forming candidate pairs and pruning redundant pairs in [RFC 8445] Section 6.1.2."
There was a problem hiding this comment.
Should we file a follow-up issue about this? Since "form" is old language, and we're not sure what is needed here.
There was a problem hiding this comment.
"formed" is existing language. This PR merely adds a link. I think a rename would be a separate issues.
There was a problem hiding this comment.
Merging this one, feel free to file an issue @pthatcher if you think this can be improved
|
Editors can integrate if @jan-ivar files an issue |
Fixes #176. cc @sam-vi — these are some items I missed in review.
Preview | Diff