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)

22 Branch
x86_64
Linux
defect
Not set
minor

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)

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]
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
Whiteboard: [WebRTC][blocking-webrtc+][parity-chrome] → [WebRTC][blocking-webrtc+][parity-chrome][spec-issue]
Attachment #729604 - Attachment is obsolete: true
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)
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)
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
Closed: 11 years ago
Resolution: --- → FIXED
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.

Attachment

General

Created:
Updated:
Size: