Skip to content
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

Add .destroy() methods to Wrappers #40

Open
menismu opened this issue Apr 4, 2018 · 0 comments
Open

Add .destroy() methods to Wrappers #40

menismu opened this issue Apr 4, 2018 · 0 comments

Comments

@menismu
Copy link
Owner

menismu commented Apr 4, 2018

Issue reported: mozilla#434
Reported by: @jtomaszewski

@jtomaszewski
jtomaszewski commented on 5 Mar 2015
Currently we have function destroyPlayer() { ... } methods in every wrapper like soundcloud/youtube, but they are hidden in private scope and can't be called manually. Thus, if I want to f.e. destroy my soundcloud wrapper and to create youtube wrapper, I will receive Uncaught TypeError: Cannot read property 'postMessage' of null errors, because callbacks haven't been removed (similar to mozilla#427).

Simple adding of self.destroy = destroyPlayer; line to popcorn.HTMLYouTubeVideoElement.js and popcorn.HTMLSoundCloudAudioElement.js solves the problem.

I can make a pull request for that, but I'd want to be sure, that it's the best way to do it?

jtomaszewski added a commit to jtomaszewski/popcorn-js that referenced this issue on 11 Jun 2015
@jtomaszewski
Add .destroy() method to media wrappers (mozilla#434)
3e44144
@jtomaszewski
jtomaszewski commented on 11 Jun 2015
I added .destroy() method to all media wrappers in jtomaszewski/popcorn-js@3e44144 .

IMHO it's okay to merge. Optionally, we could add a common test case for all wrappers.

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

No branches or pull requests

1 participant