Closed Bug 1324788 Opened 3 years ago Closed 1 year ago

Update RTCIceCandidateStats to spec

Categories

(Core :: WebRTC, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla65
Tracking Status
firefox53 --- affected
firefox65 --- fixed

People

(Reporter: jib, Assigned: ng)

References

(Blocks 2 open bugs)

Details

(Keywords: dev-doc-complete, site-compat)

Attachments

(1 file)

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 [1][2].

[1] https://w3c.github.io/webrtc-stats/#dom-rtcicecandidatestats
[2] https://dxr.mozilla.org/mozilla-central/rev/567894f026558e6dada617a3998f29aed06ac7d8/dom/webidl/RTCStatsReport.webidl#131
Rank: 15
Priority: -- → P1
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.
Assignee: nobody → na-g
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.
Keywords: stale-bug
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.
Blocks: 1380555
Keywords: stale-bug
(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 [1][2].

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 [1]:  "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?

[1] https://searchfox.org/mozilla-central/rev/c3fef66a5b211ea8038c1c132706d02db408093a/dom/webidl/RTCStatsReport.webidl#147-150
Flags: needinfo?(drno)
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"
Blocks: 1488512
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.
Flags: needinfo?(drno)
Blocks: 1508543
Bug 1489040 is a sub set of this, renaming ipAddress to ip.
No longer blocks: 1508543
Depends on: 1508543
Blocks: 1508543
Depends on: 1489040
No longer depends on: 1508543
Duplicate of this bug: 1498652
Spec Notes: there are 3 RTCIceCandidateStats fields that are in the spec that are not implemented after this:
* deleted[0]: this field has open issue about removing this field,
* url[1]: this field will be implemented in bug 1508543,
* networkType[2]: the privacy properties of this field are still being debated.

[0] https://w3c.github.io/webrtc-stats/#dom-rtcicecandidatestats-deleted
[1] https://w3c.github.io/webrtc-stats/#dom-rtcicecandidatestats-url
[2] 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 na-g@nostrum.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
https://hg.mozilla.org/mozilla-central/rev/e5c59d7e5a55
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
Duplicate of this bug: 1435774
Duplicate of this bug: 1498652
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.

The main page should be up-to-date, as should all of the existing subpages. A number of subpages for properties of the RTCIceCandidateStats object need to be written still.

Progress going forward will be covered on this issue on GitHub: https://github.com/mdn/sprints/issues/49

You need to log in before you can comment on or make changes to this bug.