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
5 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

5 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

5 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.