Closed
Bug 878945
Opened 12 years ago
Closed 12 years ago
Update DataChannel WebIDL to match spec changes (renaming dictionary values)
Categories
(Core :: WebRTC, defect)
Core
WebRTC
Tracking
()
RESOLVED
FIXED
mozilla24
People
(Reporter: jesup, Assigned: jesup)
References
Details
(Keywords: compat, Whiteboard: [webrtc][blocking-webrtc-][spec-issue])
Attachments
(2 files, 4 obsolete files)
6.30 KB,
patch
|
akeybl
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
3.14 KB,
patch
|
akeybl
:
approval-mozilla-beta+
|
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.
Assignee | ||
Comment 1•12 years ago
|
||
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-]
Comment 2•12 years ago
|
||
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.
status-firefox22:
--- → affected
status-firefox23:
--- → affected
status-firefox24:
--- → affected
tracking-firefox23:
--- → ?
tracking-firefox24:
--- → ?
Keywords: compat
Assignee | ||
Updated•12 years ago
|
Whiteboard: [WebRTC] [blocking-webrtc-] → [WebRTC] [blocking-webrtc-][spec-issue]
Assignee | ||
Comment 3•12 years ago
|
||
tested this with webrtc-landing/data_test.html; warnings appear on the web console
Comment 4•12 years ago
|
||
It might be useful to mention the replacements in the warnings.
Assignee | ||
Comment 5•12 years ago
|
||
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 6•12 years ago
|
||
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+
Assignee | ||
Comment 7•12 years ago
|
||
beta version of the patch (no webidl, warning function isn't available
Assignee | ||
Comment 8•12 years ago
|
||
Noticed we expose 'stream' as an attribute; now should be named 'id'. Tested (on m-c patch and beta patch)
Assignee | ||
Updated•12 years ago
|
Attachment #757582 -
Attachment is obsolete: true
Assignee | ||
Comment 9•12 years ago
|
||
Assignee | ||
Updated•12 years ago
|
Attachment #757553 -
Attachment is obsolete: true
Assignee | ||
Updated•12 years ago
|
Attachment #757601 -
Flags: review?(bugs)
Assignee | ||
Updated•12 years ago
|
Attachment #757603 -
Flags: review?(bugs)
Comment 10•12 years ago
|
||
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 11•12 years ago
|
||
Comment on attachment 757603 [details] [diff] [review]
rename RTCDataChannelInit dictionary items to match updated spec
update uuid
Attachment #757603 -
Flags: review?(bugs) → review+
Assignee | ||
Comment 12•12 years ago
|
||
updated UUID
Assignee | ||
Updated•12 years ago
|
Attachment #757603 -
Attachment is obsolete: true
Assignee | ||
Comment 13•12 years ago
|
||
Assignee | ||
Updated•12 years ago
|
Attachment #757601 -
Attachment is obsolete: true
Assignee | ||
Comment 14•12 years ago
|
||
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?
Assignee | ||
Updated•12 years ago
|
Attachment #757613 -
Flags: approval-mozilla-beta?
Comment 15•12 years ago
|
||
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?
Updated•12 years ago
|
Comment 16•12 years ago
|
||
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.
Updated•12 years ago
|
Assignee | ||
Comment 17•12 years ago
|
||
Comment 18•12 years ago
|
||
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
Assignee | ||
Comment 19•12 years ago
|
||
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]
Updated•12 years ago
|
Attachment #757612 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Updated•12 years ago
|
Attachment #757613 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Assignee | ||
Comment 20•12 years ago
|
||
Whiteboard: [webrtc][blocking-webrtc-][spec-issue][webrtc-uplift] → [webrtc][blocking-webrtc-][spec-issue]
Comment 21•12 years ago
|
||
Can you please provide some guidelines about how QA can manually verify this?
Assignee | ||
Comment 22•12 years ago
|
||
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.
Description
•