Closed Bug 1324788 Opened 3 years ago Closed 1 year ago
Candidate Stats to spec
47 bytes, text/x-phabricator-request
|Details | Review|
From henbos in https://github.com/webrtc/adapter/pull/402#issuecomment-268203654 : "The Chrome and Firefox localcandidate/remotecandidate are similarly incorrect, e.g. have "ipAddress" and "portNumber" instead of spec's "ip" and "port", so given that Firefox is updated with hyphens it makes sense to update this too." Bug 1322503 presents an opportunity to fix this dictionary to be spec compliant. At present `candidateType` appears to be the lone member which name has not changed in the spec .  https://w3c.github.io/webrtc-stats/#dom-rtcicecandidatestats  https://dxr.mozilla.org/mozilla-central/rev/567894f026558e6dada617a3998f29aed06ac7d8/dom/webidl/RTCStatsReport.webidl#131
FWIW for time reasons, I'm less worried about missing members like `url` than I am about keeping legacy names around forever. If we can demote legacy names to the non-hyphenated version only, that would be a win IMHO.
This is an assigned P1 bug without activity in two weeks. If you intend to continue working on this bug for the current release/iteration/sprint, remove the 'stale-bug' keyword. Otherwise we'll reset the priority of the bug back to '--' on Monday, August 28th.
Mass change P1->P2 to align with new Mozilla triage process
Priority: P1 → P2
Looks like this fell off our radar. We missed the opportunity that was bug 1322503 here. In hindsight, I should have marked it as a blocking that bug, not dependent on it. Let's see if we can piggyback on bug 1380555's timeframe for this.
(In reply to Jan-Ivar Bruaroey [:jib] (needinfo? me) from comment #0) > At present `candidateType` appears to be the lone member which > name has not changed in the spec . Not so. See https://github.com/w3c/webrtc-stats/issues/360#issuecomment-418365325 candidateType's enum values are now incompatible, and conflict with the spec (and Chrome's). The spec has: "host", "srflx", "prflx", "relay" We have : "host", "serverreflexive", "peerreflexive", "relayed" We need to up-prioritize fixing our code to spec names. This will break existing users of ice stats in Firefox. :-/ Nils, thoughts?  https://searchfox.org/mozilla-central/rev/c3fef66a5b211ea8038c1c132706d02db408093a/dom/webidl/RTCStatsReport.webidl#147-150
there is a chance that existing users *might* check for the old names because chrome legacy stats used this. I just shaved of some code for chrome legacy stats on appearin but still had to keep type checks for both "relay" and "relayed"
If not someone has a great idea how to temporarily maintain backwards compatibility I would be in favor of just switching to the spec and announce what we do widely.
Bug 1489040 is a sub set of this, renaming ipAddress to ip.
Spec Notes: there are 3 RTCIceCandidateStats fields that are in the spec that are not implemented after this: * deleted: this field has open issue about removing this field, * url: this field will be implemented in bug 1508543, * networkType: the privacy properties of this field are still being debated.  https://w3c.github.io/webrtc-stats/#dom-rtcicecandidatestats-deleted  https://w3c.github.io/webrtc-stats/#dom-rtcicecandidatestats-url  https://w3c.github.io/webrtc-stats/#dom-rtcicecandidatestats-networktype
Bug 1324788 - P1 - rename RTCIceCandidate stat "portNumber" to spec "port" Bug 1324788 - P2 - update RTCIceCandidateStats candidateType enum to spec Bug 1324788 - P3 - add RTCIceCandidatePair.priority stat Bug 1324788 - P4 - update WebRTC ICE candidate stats field componentId to spec name transportId Bug 1324788 - P5 - remove deprecated RTCIceCandidateStats.mozLocalTransport field Bug 1324788 - P6 - update WebRTC ICE candidate stats field transport to spec name, protocol Bug 1324788 - P7 - remove deprecated RTCIceCandidateStats.candidateId Bug 1324788 - P8 - reorder RTCIceCandidateStats dictionary members to match the spec
WebIDL review notes: Spec: https://w3c.github.io/webrtc-stats/#icecandidate-dict* See also: Comment 17
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/e5c59d7e5a55 Bug 1324688 - Bring RTCIceCandidateStats up to spec r=mjf,jib,smaug
Attachment #9027618 - Attachment description: Bug 1324688 - Bring RTCIceCandidateStats up to spec → Bug 1324788 - Bring RTCIceCandidateStats up to spec
Posted site compatibility note: https://www.fxsitecompat.com/en-CA/docs/2018/rtcicecandidatestats-has-been-updated-to-the-latest-spec/
Note to MDN writers: I've added a note to the Fx65 rel notes to cover this: https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/65#APIs In terms of the docs, the RTCIceCandidateStats main interface page looks to be in good shape; I think sheppy updated the information on there (and the compat data) recently. It would be nice to more completely flesh out the subpages though.
You need to log in before you can comment on or make changes to this bug.