Closed Bug 878945 Opened 6 years ago Closed 6 years ago

Update DataChannel WebIDL to match spec changes (renaming dictionary values)

Categories

(Core :: WebRTC, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla24
Tracking Status
firefox22 + fixed
firefox23 + fixed
firefox24 + fixed

People

(Reporter: jesup, Assigned: jesup)

References

Details

(Keywords: compat, Whiteboard: [webrtc][blocking-webrtc-][spec-issue])

Attachments

(2 files, 4 obsolete files)

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.
https://hg.mozilla.org/mozilla-central/rev/d784336f86e9
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
Whiteboard: [WebRTC] [blocking-webrtc-][spec-issue] → [webrtc][blocking-webrtc-][spec-issue][webrtc-uplift]
Keywords: verifyme
Attachment #757612 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #757613 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
https://hg.mozilla.org/releases/mozilla-aurora/rev/e67923f89166
Whiteboard: [webrtc][blocking-webrtc-][spec-issue][webrtc-uplift] → [webrtc][blocking-webrtc-][spec-issue]
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.
Keywords: verifyme
Blocks: 892441
You need to log in before you can comment on or make changes to this bug.