search

mozilla / popcorn-js

The HTML5 Media Framework. (Unmaintained. See https://github.com/menismu/popcorn-js for activity)
MIT License
2.14k stars 632 forks source link

Add .destroy() methods to Wrappers #434

Open jtomaszewski opened 9 years ago

jtomaszewski commented 9 years ago

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 #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 commented 9 years ago

I added .destroy() method to all media wrappers in https://github.com/jtomaszewski/popcorn-js/commit/3e44144bc34c5384162bd7935e85d85bb5f0cb49 .

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