silvermine / videojs-quality-selector

MIT License
183 stars 54 forks source link

selected is boolean attributes and should be treated like one #39

Closed gauda closed 4 years ago

gauda commented 4 years ago

In general selected is not permitted as attribute for the source tag (per https://html.spec.whatwg.org/#the-source-element) Although when using it you should at least check for selected in addition to true (as in https://html.spec.whatwg.org/#boolean-attributes).

https://github.com/silvermine/videojs-quality-selector/blob/7fdacdcb32d072288c491441b1866ee0dcff2f3a/src/js/middleware/SourceInterceptor.js#L30

Perhaps it would be better to use data-selected="true"?

jthomerson commented 4 years ago

@gauda I'm not clear what you're wanting. We know that selected isn't a typical attribute on the <source> tag, but we needed to support it for some reason; unfortunately, I can't remember the exact reason. I think that in earlier VideoJS tutorials, it was one of the methods shown for selecting a source, but I can't remember for sure.

So, removing it now would be a breaking change, which we're not going to do unless there's a major reason to do so.

It seems like you're just reporting an issue where we have a feature that you don't agree with. Is that all it is? Seems like it's a feature that won't impact you if you don't use it. I can reopen the issue if you comment back with anything that requires follow-up. Thanks.

gauda commented 4 years ago

Hallo @jthomerson, I'm using this in a Ruby on Rails app, and when using their TagHelper to create the source tags, all provided selected attributes are set to selected="selected" (since that's the proper way to declare the attribute). This behaviour breaks this library so right now I have to search and replace all generated tags. Perhaps you can change the line to return source.selected === true || source.selected === 'true' || source.selected === 'selected'; since that would eliminate my problem.

jthomerson commented 4 years ago

@gauda makes sense. Pull request welcome. Please be sure to observe our coding standards to ensure the greatest likelihood of success.