Closed Bug 878945 Opened 6 years ago Closed 6 years ago
Channel Web IDL to match spec changes (renaming dictionary values)
6.30 KB, patch
|Details | Diff | Splinter Review|
3.14 KB, patch
|Details | Diff | Splinter Review|
The spec for the RTCDataChannelInit dictionary changed once Microsoft and Google actually paid attention and made their preferences known (and people actually talked about the values). The result was renaming of 4 entries, and one of them swapped meaning in the rename (outOfOrderAllowed -> ordered=true). We need to change our usage to match. It would be good to support the existing names for a short time (fx22 basically) to avoid immediately breaking everyone. We'd throw warnings on use of deprecated names.
If we *need* to, we can release without naming changes, but that will force developers to sniff UA versions since dictionaries aren't easily sniffed directly.
Whiteboard: [WebRTC] [blocking-webrtc-]
Sounds like we should push to get this implemented asap. Tracking for 23 and 24 at least. Might be too late for 22, but we'll see.
Whiteboard: [WebRTC] [blocking-webrtc-] → [WebRTC] [blocking-webrtc-][spec-issue]
tested this with webrtc-landing/data_test.html; warnings appear on the web console
It might be useful to mention the replacements in the warnings.
Comment on attachment 757553 [details] [diff] [review] rename RTCDataChannelInit dictionary items to match updated spec Looking for a fast review to try to get into Beta 4 which cuts tomorrow EOD in california
Attachment #757553 - Flags: review?(bugs)
Comment on attachment 757553 [details] [diff] [review] rename RTCDataChannelInit dictionary items to match updated spec But I don't think we have this .webidl in beta
Attachment #757553 - Flags: review?(bugs) → review+
beta version of the patch (no webidl, warning function isn't available
Noticed we expose 'stream' as an attribute; now should be named 'id'. Tested (on m-c patch and beta patch)
Attachment #757582 - Attachment is obsolete: true
Attachment #757553 - Attachment is obsolete: true
Attachment #757601 - Flags: review?(bugs)
Attachment #757603 - Flags: review?(bugs)
Comment on attachment 757601 [details] [diff] [review] rename DataChannel dict items (beta patch, no warnings) We can't change interfaces in beta. You could add a new interface which has .id and implement it and expose in classinfo.
Attachment #757601 - Flags: review?(bugs) → review-
Comment on attachment 757603 [details] [diff] [review] rename RTCDataChannelInit dictionary items to match updated spec update uuid
Attachment #757603 - Flags: review?(bugs) → review+
Attachment #757603 - Attachment is obsolete: true
Attachment #757601 - Attachment is obsolete: true
Comment on attachment 757612 [details] [diff] [review] rename RTCDataChannelInit dictionary items to match updated spec [Approval Request Comment] (for Aurora and Beta patches) Bug caused by (feature/regressing bug #): N/A (spec change) User impact if declined: Apps are likely to be forced to UA/version sniff, which is bad. We really don't want to cause them to do that Testing completed (on m-c, etc.): Waiting to land on inbound (closure). Patch tested locally on inbound, aurora and beta with custom test to verify the backwards-compatibility. Risk to taking this patch (and alternatives if risky): No risk other than changing the UUID String or IDL/UUID changes made by this patch: DataChannel UUID changes. We believe strongly no-one is using DataChannels from a binary extension, especially with the spec still in a flux - and if they are, they should track the release process as it's not in release yet. smaug is r+ other than the UUID issue
Attachment #757612 - Flags: approval-mozilla-aurora?
Attachment #757613 - Flags: approval-mozilla-beta?
Tracking, waiting for landing to central to approve for Aurora - Alex is running FF23 and so will make the call on uplifting the UUID change in beta but is there a reason to rush this to beta? If no one is using DataChannel yet couldn't this ride from 23?
Spoke with rjesup - we'll take this change (ba=akeybl) once it's on m-c given no known clients. We want this in FF22 since people will start targeting this interface in that version.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
https://hg.mozilla.org/releases/mozilla-beta/rev/0ed8a89f8484 Aurora is waiting on tree closure
Attachment #757612 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #757613 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Can you please provide some guidelines about how QA can manually verify this?
Verify that the new names work, and that the old names work (and on 24 throw a warning in the web console). Modifying the tests at mozilla.github.com/webrtc-landing/data_test.html is the easiest way.
You need to log in before you can comment on or make changes to this bug.