Temasys / AdapterJS

AdapterJS Javascript Polyfill and Tools for WebRTC - Skylink WebRTC
http://skylink.io/web
Other
428 stars 100 forks source link

Add bind() polyfill to plugin functions #291

Open jeffstephens opened 6 years ago

jeffstephens commented 6 years ago

The Temasys plugin returns Javascript functions that reflect a much older version of Javascript and do not support the bind() functionality which causes failures in our code and some plugins like rtcpeerconnection. This recursively polyfills all functions on the plugin object so they support this relatively recent Javascript functionality.

johache commented 6 years ago

Will need to test, but in theory, it sounds great to me :)

jeffstephens commented 6 years ago

Sweet! I do believe I'm seeing infinite recursion in IE11 due to the different event listener code (attachEvent)... I'll fix that up.

Also, would it be better if I squashed these commits into one?

johache commented 6 years ago

You don't need to squash the commits.

I will be waiting for an update on the infinite recursion :)

jeffstephens commented 6 years ago

Ok, that should do it! Let me know if you'd like me to make any changes.

johache commented 6 years ago

Hi, I'm afraid this is not working as intended.

Safari

Take for example this sample : https://plugin.temasys.com.sg/demo/src/content/peerconnection/pc1/ You can modify it to use your AJS.

Try the following code :

var rs = pc2.getRemoteStreams;
rs.bind(pc1)(); // returns pc2.getRemoteStreams(), instead of pc1.getRemoteStreams()

This is probably due to the nature of NPAPI/ActiveX plugin. When you call pc.createOffer, it's not calling the createOffer function on the pc1 instance. It's calling the createOffer function of this specific instance of PC. pc2.createOffer is a difference instance of the function. I know it's a bit weird, but I'm pretty sure that's what's happening.

IE

IE is flat out not working for me. I tried to bind pc.createOffer and I am getting an immediate error when I try to call a function after binding it.

jeffstephens commented 6 years ago

Whoops - thanks for the info, that is helpful. The plugin objects are a bit strange. I'll work on getting this fixed!

johache commented 6 years ago

Feel free to work on it as much as you want, but very honestly, I don't think that there is a clean way of fixing this.

jeffstephens commented 6 years ago

Thanks for the help! I think you are probably right and I'm going to start looking at a different approach than modifying AdapterJS.

jeffstephens commented 6 years ago

Hi @johache - my colleague @dwilson6 figured out a way to make this work with the plugin objects. I'm now able to call bind() on the functions as in Chrome/Firefox. Would you mind taking another look?

johache commented 6 years ago

Sure, no prob. I'll either get to it today/tomorrow, or it will have a to wait for a couple of weeks.

jeffstephens commented 6 years ago

Hey @johache, do you have time to look at this? Thanks for your attention!

johache commented 6 years ago

I'm sorry, but it still doesn't seem to work.

var rs = pc2.getRemoteStreams;
rs.bind(pc2)(); // same error for rs.bind(pc2)();

Gives me this error TypeError: rs.bind(pc2) is not a function. (In 'rs.bind(pc2)()', 'rs.bind(pc2)' is an instance of Array)

and trying to preset actually calls the function :

var co = pc1.createOffer.bind(onCreateOfferSuccess, onCreateSessionDescriptionError,
      offerOptions);
ee11131 commented 6 years ago

In my case, when I merge this pull request in adapter, using the latest simplewebrtc I get the error "Calling method on NPObject" on "return fToBind.apply(this instanceof fNOP". Note that SimpleWebRTC uses bind() so that is why it's so important to me to get this pull request to work.

This is when I do a Safari - Chrome video call. This error only appears on Safari's side.