The default bug view has changed. See this FAQ.

PeerConnection ondatachannel callback receives channel in Firefox, but in Chrome an event is passed in

VERIFIED FIXED in mozilla22

Status

()

Core
WebRTC: Networking
--
minor
VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: ack, Assigned: jesup)

Tracking

({compat})

22 Branch
mozilla22
x86_64
Linux
compat
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [WebRTC][blocking-webrtc+][parity-chrome][spec-issue])

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

4 years ago
In Firefox:

peerconnection.ondatachannel = function(channel) {...}

In Chrome:

peerconnection.ondatachannel = function(event) { var channel = event.channel; ...}

This should be standardized.
Eric - Are we doing this right or is Chrome is doing right?
Flags: needinfo?(ekr)
Whiteboard: [WebRTC][parity-chrome]
Flags: needinfo?(rjesup)
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

4 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)
http://dev.w3.org/2011/webrtc/editor/webrtc.html#attributes-6
http://www.w3.org/html/wg/drafts/html/master/webappapis.html#eventhandler
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]
(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.
Assignee: nobody → rjesup
(Assignee)

Comment 7

4 years ago
Created attachment 729604 [details] [diff] [review]
Update DataChannel protocol to be declarative vs 3-way handshake (per IETF) (WIP)

Updated

4 years ago
Whiteboard: [WebRTC][blocking-webrtc+][parity-chrome] → [WebRTC][blocking-webrtc+][parity-chrome][spec-issue]
(Assignee)

Comment 8

4 years ago
Created attachment 730557 [details] [diff] [review]
Update DataChannel protocol to be declarative vs 3-way handshake (per IETF) (WIP)
(Assignee)

Updated

4 years ago
Attachment #729604 - Attachment is obsolete: true
(Assignee)

Comment 9

4 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

4 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

4 years ago
Created attachment 731032 [details] [diff] [review]
ondatachannel() should take an event not a channel
(Assignee)

Updated

4 years ago
Attachment #731032 - Flags: review?(bugs)
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

4 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/ea296ec9e471
Target Milestone: --- → mozilla22
https://hg.mozilla.org/mozilla-central/rev/ea296ec9e471
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED

Updated

4 years ago
Keywords: verifyme
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.