Closed
Bug 846110
Opened 11 years ago
Closed 11 years ago
PeerConnection ondatachannel callback receives channel in Firefox, but in Chrome an event is passed in
Categories
(Core :: WebRTC: Networking, defect)
Tracking
()
VERIFIED
FIXED
mozilla22
People
(Reporter: akligman, Assigned: jesup)
Details
(Keywords: compat, Whiteboard: [WebRTC][blocking-webrtc+][parity-chrome][spec-issue])
Attachments
(1 file, 2 obsolete files)
951 bytes,
patch
|
smaug
:
review+
|
Details | Diff | Splinter Review |
In Firefox: peerconnection.ondatachannel = function(channel) {...} In Chrome: peerconnection.ondatachannel = function(event) { var channel = event.channel; ...} This should be standardized.
Comment 1•11 years ago
|
||
Eric - Are we doing this right or is Chrome is doing right?
Flags: needinfo?(ekr)
Whiteboard: [WebRTC][parity-chrome]
Updated•11 years ago
|
Flags: needinfo?(rjesup)
Comment 2•11 years ago
|
||
Potential compat issue. Might block, but let's see what Randell and Ekr say.
Whiteboard: [WebRTC][parity-chrome] → [WebRTC][blocking-webrtc?][parity-chrome]
Comment 3•11 years ago
|
||
I'm having trouble finding a detailed answer in the specification, but based on the example in the spec, it appears that the argument is supposed to be an event: // setup chat on incoming data channel pc.ondatachannel = function (evt) { channel = evt.channel; setupChat(); };
Flags: needinfo?(ekr)
Comment 4•11 years ago
|
||
http://dev.w3.org/2011/webrtc/editor/webrtc.html#attributes-6 http://www.w3.org/html/wg/drafts/html/master/webappapis.html#eventhandler
Comment 5•11 years ago
|
||
Sounds like this is a bug on our end then. I'm calling blocking on this one because this sounds like an obvious place for a cross-browser compatibility issue that developers will experience. And we should really try not to encourage ugly JS practices or worse, user agent sniffing to work around this problem.
Flags: needinfo?(rjesup)
Keywords: compat
Whiteboard: [WebRTC][blocking-webrtc?][parity-chrome] → [WebRTC][blocking-webrtc+][parity-chrome]
Comment 6•11 years ago
|
||
(In reply to Jason Smith [:jsmith] from comment #5) > Sounds like this is a bug on our end then. > > I'm calling blocking on this one because this sounds like an obvious place > for a cross-browser compatibility issue that developers will experience. And > we should really try not to encourage ugly JS practices or worse, user agent > sniffing to work around this problem. Yes, I think you're right. I believe this is an easy fix to make BUT it will break backwards compatibility. So we'll want to let developers know via Hacks and Developer Engagement a few days before we fix this bug and uplift the patch.
Updated•11 years ago
|
Assignee: nobody → rjesup
Assignee | ||
Comment 7•11 years ago
|
||
Updated•11 years ago
|
Whiteboard: [WebRTC][blocking-webrtc+][parity-chrome] → [WebRTC][blocking-webrtc+][parity-chrome][spec-issue]
Assignee | ||
Comment 8•11 years ago
|
||
Assignee | ||
Updated•11 years ago
|
Attachment #729604 -
Attachment is obsolete: true
Assignee | ||
Comment 9•11 years ago
|
||
Comment on attachment 730557 [details] [diff] [review] Update DataChannel protocol to be declarative vs 3-way handshake (per IETF) (WIP) Trying a review request to Michael Tuexen, since he knows the protocol and has written patches.
Attachment #730557 -
Flags: review?(tuexen)
Assignee | ||
Comment 10•11 years ago
|
||
Comment on attachment 730557 [details] [diff] [review] Update DataChannel protocol to be declarative vs 3-way handshake (per IETF) (WIP) Oops. Uploaded patch to the wrong bug... Real bug is bug 855623
Attachment #730557 -
Attachment is obsolete: true
Attachment #730557 -
Flags: review?(tuexen)
Assignee | ||
Comment 11•11 years ago
|
||
Assignee | ||
Updated•11 years ago
|
Attachment #731032 -
Flags: review?(bugs)
Comment 12•11 years ago
|
||
Comment on attachment 731032 [details] [diff] [review] ondatachannel() should take an event not a channel über-hack, but I guess ok until we have proper EventTarget.
Attachment #731032 -
Flags: review?(bugs) → review+
Assignee | ||
Comment 13•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/ea296ec9e471
Target Milestone: --- → mozilla22
Comment 14•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/ea296ec9e471
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Comment 15•11 years ago
|
||
Verified through a sanity test with ondatachannel - I'm getting an event object that I can grab a channel off of it.
Status: RESOLVED → VERIFIED
Keywords: verifyme
You need to log in
before you can comment on or make changes to this bug.
Description
•