Inherit tech 'features' from 'MediaTechController'

[gkatsev]

Use vjs.obj.create to inherit 'features' from
'MediaTechController.prototype.features'. Because we are inheriting, if
the defaults change (prototype.features), we can receive these updates
in our tech if we had not changed properties of features manually.

This fixes #1407.

[heff]

Can you test to make sure this works with the different uses of features?

[heff]

i.e. I think the html5 tech usage needs some modification

[jnwng]

ugh, sorry, i had actually stumbled upon this many months ago but wasn't sure about how to get a good use case for assigning this.features since my use case was switching techs

coursera@e90a154

i think @gkatsev's change does the same thing in a much less fragile way

[gkatsev]

@heff I think it works correctly. I had a flash and an html player on the page and both work perfectly now.
Anything with the html5 tech I should check out specifically?

[heff]

The use of features in the HTML5 tech before MediaTechController is called.

video.js/src/js/media/html5.js

Line 16 in 7f4e6eb

this.features['volumeControl'] = vjs.Html5.canControlVolume();

[gkatsev]

Alright, I'll take a look later tonight or tomorrow.

[gkatsev]

@heff is there anything keeping those things happening before calling MediaTechController? A quick test to move it down seems to work fine.

[gkatsev]

I just did a more thorough test with some android and ios devices as well and moving these 4 lines down to below the MediaTechController call doesn't break anything and works as expected.
@heff let me know if you can think off any problems that moving those lines might cause.

[heff]

Yeah, I can't think of any problems with that. I wish those weren't in the init in the first place actually, but not sure if there's an easy way to do that now.

[gkatsev]

Hm... never mind, moving those lines breaks things.

[gkatsev]

Yeah, it doesn't work to move the setting below the MediaTechController call.
Things like movingMediaElementInDOM end up not being available.

[heff]

It looks like it's probably because some of those settings need to be set before the createEl function that gets run in the Component init. It looks like a higher level refactor of features is needed.

[gkatsev]

One way of fixing it is by passing the overrides in as an option to MediaTechController, which would then apply those this.features after doing the create call. @heff thoughts about that?

[heff]

I think we'd still run into some complication with the defaults on MediaTech with the different this contexts. Or at least we'd have to refer to those directly (vjs.MediaTech...) in order to merge with whatever comes through options.

Another option would be to promote these to direct prototype properties in some way. I think the object is only used for organization currently.
this.featuresVolumeControl = true;
There might be a prettier way to do that.

[gkatsev]

Do you mean moving them to the prototype of MediaTechController? If so, we might still run into the same problem because after you set the property, when the object gets created during the .call, the prototype will overwrite the this value.

[heff]

Do you mean moving them to the prototype of MediaTechController?

Yeah. If we do that we can rely on standard prototype inheritance. If everything's added to MediaTech.prototype and Html5.prototype, not added in the init(), then we don't have to worry about .call.

[gkatsev]

Ok, I'll try it out.

[gkatsev]

Looks like promoting it to the prototype works well.

[gkatsev]

I kept these as strings for now, but I can change it.

[gkatsev]

Oops, forgot to update tests.

[gkatsev]

The rename was needed because otherwise playbackRate conflicted with the Html5 tech's playbackRate method.

[heff]

Yeah, I was actually thinking we'd leave 'features' or something like it in the property name. features playbackRate or supportsPlaybackRate. tech.volumeControl, for example, isn't the best name for what the property actually is used for.

[heff]

Could you update this so it can merge with @dmlap's change?

[heff]

@gkatsev working on a release. Can you try to finish this up?

[gkatsev]

This PR is now obsolete. See #1470.