Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

RTCPeerConnectionIceErrorEvent.errorText seems to correspond to RTCPeerConnectionIceErrorEventInit.statusText #2603

Closed
youennf opened this issue Nov 19, 2020 · 9 comments · Fixed by #2611
Closed

RTCPeerConnectionIceErrorEvent.errorText seems to correspond to RTCPeerConnectionIceErrorEventInit.statusText #2603

youennf opened this issue Nov 19, 2020 · 9 comments · Fixed by #2611
Assignees
@alvestrand

Comments

@youennf
Copy link
Contributor

youennf commented Nov 19, 2020

Having two names for the same thing is misleading.
How about migrating RTCPeerConnectionIceErrorEventInit.statusText to RTCPeerConnectionIceErrorEventInit.errorText?
@youennf
Copy link
Contributor Author

youennf commented Nov 19, 2020

Looking at WPT, I do not see any test written that uses statusText.

@alvestrand
Copy link
Contributor

I think we should rename it - it's a mistake.

@alvestrand
Copy link
Contributor

@dontcallmedom how do we deal with this given the state of the spec? (Chrome is as far as we know the only implementation of the mistake)

@alvestrand alvestrand self-assigned this Nov 19, 2020
@jan-ivar
Copy link
Member

To clarify: we should rename the init dictionary (.statusText) since they're rarely used in practice, to what's on the actual event (.errorText).

@dontcallmedom
Copy link
Member

having looked into this, given that it's a substantive (i.e. implementations have to adapt) but really minor change (esp from an IPR perspective), the suggestion is to make the change in the spec and get implementors confirmations they're OK with the change should allow us to move to PR without another CR cycle.

@youennf
Copy link
Contributor Author

youennf commented Nov 23, 2020

the suggestion is to make the change in the spec and get implementors confirmations they're OK with the change should allow us to move to PR without another CR cycle.

That works for me. onicecandidaterror was just added to WebKit and is using errorText everywhere.

@alvestrand
Copy link
Contributor

I can say that Chrome will adapt.

@jan-ivar
Copy link
Member

Works for me as well.

@alvestrand
Copy link
Contributor

Change to Chrome is done in https://chromium-review.googlesource.com/c/chromium/src/+/2555078

pull bot pushed a commit to Alan-love/chromium that referenced this issue Nov 24, 2020
@alvestrand
Rename RTCPeerConnectionIceErrorEventInit.statusText to errorText
b4468a4 
This aligns it with the name of the field in IceErrorEvent.
Background: w3c/webrtc-pc#2603

Bug: chromium:1151946
Change-Id: I81ef78b43905b05d5a494ac21a4b86545d3fbd8b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2555078
Reviewed-by: Henrik Boström <[email protected]>
Commit-Queue: Harald Alvestrand <[email protected]>
Cr-Commit-Position: refs/heads/master@{#830298}
dontcallmedom added a commit that referenced this issue Nov 24, 2020
@dontcallmedom
Fix RTCPeerConnectionIceErrorEventInit.statusText to match RTCPeerCon…
6fd8775 
…nectionIceErrorEvent.errorText

close #2603
@dontcallmedom dontcallmedom mentioned this issue Nov 24, 2020
Closed
alvestrand added a commit that referenced this issue Nov 24, 2020
@alvestrand
Change RTCPeerConnectionIceErrorEventInit.statusText to errorText
535322e 
Fixes #2603
Also points out that a test exists - in the process of being merged
upstream, see Chromium PR for status:

https://chromium-review.googlesource.com/c/chromium/src/+/2555011
@alvestrand alvestrand mentioned this issue Nov 24, 2020
Merged
@dontcallmedom dontcallmedom mentioned this issue Nov 30, 2020
Closed
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
@alvestrand @copybara-github
Rename RTCPeerConnectionIceErrorEventInit.statusText to errorText
2fbc196 
This aligns it with the name of the field in IceErrorEvent.
Background: w3c/webrtc-pc#2603

Bug: chromium:1151946
Change-Id: I81ef78b43905b05d5a494ac21a4b86545d3fbd8b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2555078
Reviewed-by: Henrik Boström <[email protected]>
Commit-Queue: Harald Alvestrand <[email protected]>
Cr-Commit-Position: refs/heads/master@{#830298}
GitOrigin-RevId: b4468a4052ab4538d5361ce04ac5730880b7b248
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@alvestrand alvestrand

Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@dontcallmedom @alvestrand @jan-ivar @youennf