Jan-Ivar Bruaroey [:jib] (needinfo? me) Reporter
	Description • 7 years ago

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

	Jan-Ivar Bruaroey [:jib] (needinfo? me) Reporter
	Comment 1 • 7 years ago

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.

	Emma Humphries ☕️🎸🧞‍♀️✨ (she/they) [:emceeaich] (Pacific Time) use needinfo
	Comment 2 • 7 years ago

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.

	Bulk Bug Changes for mreavy's org
	Comment 3 • 7 years ago

Mass change P1->P2 to align with new Mozilla triage process

	Jan-Ivar Bruaroey [:jib] (needinfo? me) Reporter
	Comment 4 • 6 years ago

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.

	Jan-Ivar Bruaroey [:jib] (needinfo? me) Reporter
	Comment 5 • 6 years ago

(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

	Philipp Hancke [:fippo]
	Comment 6 • 6 years ago

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"

	Nils Ohlmeier [:drno]
	Comment 7 • 6 years ago

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.

	Nico Grunbaum [:ng, @chew:mozilla.org] Assignee
	Comment 8 • 6 years ago

https://treeherder.mozilla.org/#/jobs?repo=try&revision=89b761109338b3215eaefccdc95a8a890bad63cd

	Nico Grunbaum [:ng, @chew:mozilla.org] Assignee
	Comment 9 • 6 years ago

Bug 1489040 is a sub set of this, renaming ipAddress to ip.

	Nico Grunbaum [:ng, @chew:mozilla.org] Assignee
	Comment 10 • 6 years ago

https://treeherder.mozilla.org/#/jobs?repo=try&revision=83ff42455d04ce8d2575348fe8ac76ed3669874b

	Nico Grunbaum [:ng, @chew:mozilla.org] Assignee
	Comment 11 • 6 years ago

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f70f0bb4636c477a05eb0c8448f5aed855afd00d

	Nico Grunbaum [:ng, @chew:mozilla.org] Assignee
	Comment 12 • 6 years ago

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b860d546a1b1ccf947dfac940ae091b39502c986

	Nico Grunbaum [:ng, @chew:mozilla.org] Assignee
	Comment 13 • 6 years ago

https://treeherder.mozilla.org/#/jobs?repo=try&revision=df9d92c73dd839d6450066ab20ec08b98b9564ff

	Nico Grunbaum [:ng, @chew:mozilla.org] Assignee
	Comment 15 • 6 years ago

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8004eea2ad21f84385634899f9492d3dd80abaac

	Nico Grunbaum [:ng, @chew:mozilla.org] Assignee
	Comment 16 • 6 years ago

https://treeherder.mozilla.org/#/jobs?repo=try&revision=497a7bbdb5af4add423f111b221f516037af82e2

	Nico Grunbaum [:ng, @chew:mozilla.org] Assignee
	Comment 17 • 6 years ago

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

	Nico Grunbaum [:ng, @chew:mozilla.org] Assignee
	Comment 18 • 6 years ago

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d61726aade9385f3de6d5afa75447aa085548f5e

	Nico Grunbaum [:ng, @chew:mozilla.org] Assignee
	Comment 19 • 6 years ago

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

	Nico Grunbaum [:ng, @chew:mozilla.org] Assignee
	Comment 20 • 6 years ago

WebIDL review notes: 
Spec: https://w3c.github.io/webrtc-stats/#icecandidate-dict*
See also: Comment 17

	Nico Grunbaum [:ng, @chew:mozilla.org] Assignee
	Comment 21 • 6 years ago

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f4f1f45096cd425cbe8414e7f349270fb195c0ca

	Nico Grunbaum [:ng, @chew:mozilla.org] Assignee
	Comment 22 • 6 years ago

https://treeherder.mozilla.org/#/jobs?repo=try&revision=22146f4f867223d8bce86b6225592c54368492d0

	Pulsebot
	Comment 23 • 6 years ago

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

	Cristina Coroiu [:ccoroiu]
	Comment 24 • 6 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/e5c59d7e5a55

	Kohei Yoshino [:kohei]
	Comment 26 • 5 years ago

Posted site compatibility note: https://www.fxsitecompat.com/en-CA/docs/2018/rtcicecandidatestats-has-been-updated-to-the-latest-spec/

	Chris Mills [:cmills]
	Comment 28 • 5 years ago

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.