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

VERIFIED FIXED in mozilla22

Status

()

--
minor
VERIFIED FIXED
6 years ago
6 years ago

People

(Reporter: akligman, 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

6 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

6 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)
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

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

Updated

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

Comment 8

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

Updated

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

Comment 9

6 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

6 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

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

Updated

6 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+
https://hg.mozilla.org/mozilla-central/rev/ea296ec9e471
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED

Updated

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