Correcting NativeControls + Taps.

[darrennolan]

Skipping browser bugfix for changing cursor as that stops tap events
from propagating to native controls. Also enabled option that disables
converting events to 'tap'. The 'tap' event does not cause native
controls to appear on devices like android.

Description

When using nativeControlsForTouch - taps appear to be stalled and don't go through to the native video element, therefore the controls never appear again when tapping as would be expected. On Android you have to tap and hold to bring up the controls, which is not as users would typically expect.

Additionally, added option for disableEmitTapEvents which when enabled, doesn't attempt to convert / check taps and allows the browser to natively interpret taps as clicks instead.

Specific Changes proposed

Added option disableEmitTapEvents to stop converting taps to manually.
Don't call stopPropagation and preventDefault on mousemove when TOUCH_ENABLED is true and we're using nativeControls.

This particular change has been in production for us for a while now as we rely on native controls for touch users.

[gkatsev]

So, the issue is that on mobile if you're using native controls, our tap emitter causes issues when applied to the "tech" itself. Maybe the disableEmitTapEvents isn't necessary but instead we just want to only turn on these tap events if we are not in nativeControlsForTouch? I.e., wrap https://github.com/videojs/video.js/blob/master/src/js/tech/tech.js#L143-L144 with an if that checks to see whether native controls are being used?

This way, we can make this PR a more precise and minimal change targeting the issue at hand. If something like disableEmitTapEvents is necessary in the future, we can add it at a later date. What do you think, @darrennolan?

[darrennolan]

That could possibly solve our issue with native controls not showing on taps. Let me do some playing and I'll get back to this PR.

Thanks for your input! :D

I did originally want to wrap in an option in case people were previously expecting this behaviour.

[darrennolan]

Updated the branch, now simply skipping emitTapEvents if nativeControlsForTouch is enabled.

Tested with our current usage of VideoJS, desktop-chrome, iOS Safari and Android Chrome. All working nicely.

Thanks for the suggestion for the simpler solution.

[gkatsev]

Oh, cool. This is even simpler. Is the other piece no longer necessary?

[darrennolan]

It's not required for showing controls on tap, no. That may have had something to do with scrolling past the video player.

However for the purposes of this PR, I'm going to ignore it and open a separate PR if that mousemove capture/stop causes issues around the scroll.

[gkatsev]

Sounds good.
We're going to try and make a minor release tomorrow some time, this fix is going to go in.

[misteroneill]

Sounds good to me.