Re: [webrtc-pc] Missing url in RTCIceCandidateInit (#2795)

> I claim there's no web compat problem here. The only concern might be people expecting relayProtocol and url to start showing up in 2, but nothing in the spec supports such an assumption.

The spec clearly supports this assumption as it states that all attributes are derived from `candidate` except `candidate`, `sdpMid`, `sdpMLineIndex` and `usernameFragment`. These are precisely the 4 members that the spec serialises in toJSON.
I think this explains why `url` was put in `RTCPeerConnectionIceEvent` in the first place and not the candidate.
Also, am I right in thinking that all current implementations are actually supporting full fidelity?

`relayProtocol` and `url` are new proposals and they both break this assumption.
It is obviously unfair to say that it is safe to do it for `url` given we have done it for `relayProtocol`, none of them have been implemented yet AFAIK.

Maybe we should have introduced a RTCIceLocalCandidate that could be partially exported to a RTCIceCandidateInit, representing a remote candidate. This model seems cleaner to me.
That said, this is too late, adding a subclass now is probably not feasible, see RTCPeerConnectionIceEvent constructor.
I do not see any clean solution, sigh...

The current spec state is not great given the merged PRs have been missing some areas where the spec would need to be updated.
To properly expose `relayProtocol` and `url` directly in RTCIceCandidate, we might want to:
- change RTCIceCandidate definition to not let people expect toJSON full fidelity. Maybe also add a comment in the WebIDL stating that the fields below are for local candidates only and are not expected to be exported.
- a way to construct a candidate with both `relayProtocol`and `url` values being set by JS. This might be a bit odd to add them to RTCIceCandidateInit but maybe this is good enough.
- change RTCPeerConnectionIceEventInit to not include url.
The alternative is to put `relayProtocol` and `url` back in the event, consistent with the spec but this is less ergonomic and less aligned with other web APIs.

-- 
GitHub Notification of comment by youennf
Please view or discuss this issue at https://github.com/w3c/webrtc-pc/issues/2795#issuecomment-1316773358 using your GitHub account


-- 
Sent via github-notify-ml as configured in https://github.com/w3c/github-notify-ml-config

Received on Wednesday, 16 November 2022 10:35:09 UTC