Skip to content

Add bind() polyfill to plugin functions #291

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

jeffstephens
Copy link

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
Copy link
Contributor

johache commented Oct 2, 2017

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

@jeffstephens
Copy link
Author

jeffstephens commented Oct 2, 2017

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
Copy link
Contributor

johache commented Oct 3, 2017

You don't need to squash the commits.

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

@jeffstephens
Copy link
Author

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

@johache
Copy link
Contributor

johache commented Oct 4, 2017

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
Copy link
Author

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

@johache
Copy link
Contributor

johache commented Oct 5, 2017

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
Copy link
Author

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
Copy link
Author

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
Copy link
Contributor

johache commented Oct 12, 2017

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

@jeffstephens
Copy link
Author

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

@johache
Copy link
Contributor

johache commented Nov 14, 2017

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
Copy link

ee11131 commented Nov 15, 2017

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants