benwiley4000 / cassette

📼 A flexible media player component library for React that requires no up-front config
https://benwiley4000.github.io/cassette/styleguide
MIT License
185 stars 28 forks source link

Empty playlist has an empty source #357

Closed danielr18 closed 5 years ago

danielr18 commented 5 years ago

image

This triggers onTrackPlaybackFailure. This happens because when playlist is invalid, const blankSources = [{ src: '' }]; is returned.

I'm happy to make a PR to fix this, what do you think is the best way to handle it?

benwiley4000 commented 5 years ago

Thanks for catching this!

The "most correct" solution might be to make blankSources an empty array, but then we'd have to make a bunch of changes in the rest of the code, so I think the most straightforward solution is to have this check in the loop that adds source elements:

if (!source.src) {
  continue;
}
benwiley4000 commented 5 years ago

On the other hand maybe we do want to emit an error if someone accidentally adds a track without a source? So not sure if what I said above is ideal. What do you think?

danielr18 commented 5 years ago

We could check for isPlaylistValid before calling setMediaElementSources or do something like this in setMediaElementSources.

if (sources === blankSources) {
  return
}
benwiley4000 commented 5 years ago

We could check for isPlaylistValid before calling setMediaElementSources

I like where you're going with this - that makes sense to me.

I think we still want to empty the previous sources and call media.load() every time, so maybe we should:

  1. Wrap the for (const source of sources) loop in a check for isPlaylistValid
  2. While we're at it, remove the sources argument to setMediaElementSources and just look it up after the isPlaylistValid check for the loop.
benwiley4000 commented 5 years ago

Closed in #359.