Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Custom Controls - User is unable to scroll/zoom page if touching the video #895

Closed
flucivja opened this issue Dec 13, 2013 · 22 comments
Closed

Comments

@flucivja
Copy link

Hi Guys,

I just have found one issue which is serious for my project. If I have enabled custom controls on mobile then I am not able to scroll/zoom the page if I am touching the video (tested on iOS and Android).

I found that issue is probably preventDefault which is called at touchstart event here:
https://github.com/videojs/video.js/blob/master/src/js/media/media.js#L84.

Could this be fixed or could you help me how can I workaround this?

Thanks,
Fero

@heff
Copy link
Member

heff commented Dec 17, 2013

Does it fix the issue if you remove preventDefault? It's in there to mimic typical touch reactions, e.g. if you touch and hold, it shouldn't act the same as if you tap. But it sounds like we may be missing something. Can you set up an example online to try out?

@gkatsev, any thoughts on this?

@gkatsev
Copy link
Member

gkatsev commented Dec 17, 2013

I tried to check it out on my phone but looks like customControlsOnMobile might still be broken per #635.
But yeah, I don't really know what's going on exactly.
It's possible that we might need to move the preventDefaults a bit later in the chain after we're sure that a pinch operation isn't what's happening.

@flucivja
Copy link
Author

I replaced preventDefault with following code:

this.one('touchend', function(e) {
  e.stopPropagation();
  e.preventDefault();
});

Explanation:
You cant call preventDefault immediately on touchstart because it will not continue to catch touchmove and touchend which are needed for scrolling/zooming. So I attached only one touchend event to current one in touchstart which will call preventDefault if touch is only tap.

Not sure if this is good fix, but it works on all our tested devices.

Also I noticed same issue with poster which is button object:
https://github.com/videojs/video.js/blob/master/src/js/button.js#L21

@heff
Copy link
Member

heff commented Mar 4, 2014

We made some modifications to touch handling so that events are still allowed to bubble.
#962

I believe this should be fixed as of 4.4, so I'm going to close this, but please comment and reopen if you find otherwise.

@heff heff closed this as completed Mar 4, 2014
@smohadjer
Copy link

This is still not fixed. On iPad iOS7 touching videos with custom controls (nativeControlsForTouch: false) does not allow scrolling the page. You can test here:
http://sandbox.saeidmohadjer.com/video_vjs.html

@timomayer
Copy link

i can confirm this bug is still open and not fixed! can you pls fix it? broken page scroll over all video elements is a huge blocker in my oppinion.

@mmcc mmcc reopened this May 5, 2014
@mmcc
Copy link
Member

mmcc commented May 5, 2014

Reopening so we can revisit this one.

@zang
Copy link

zang commented Jun 19, 2014

Hi Matt,

Any updates on this one? :)

thanks,
Lucy

@heff
Copy link
Member

heff commented Jun 23, 2014

The previous example didn't show controls for me so I created a new jsbin for nativeControlsForTouch:
http://jsbin.com/xorag/

@heff
Copy link
Member

heff commented Jun 23, 2014

I can confirm this is still an issue. The line of that needs to be updated is:
https://github.com/videojs/video.js/blob/v4.6.2/src/js/media/media.js#L80

I'm not immediately clear what the side effects will be of removing preventDefault. I don't think we want to remove it completely and let mouse events happen because we still want a touch and hold to not reveal the controls again on touch devices. It could be that we need to either add a new touchend event listener, or it could be that we can use the emitTapEvents code here.

@gkatsev
Copy link
Member

gkatsev commented Jun 23, 2014

We might want to move the preventDefault to touchend instead. We are mostly interested in preventing the default click events from happening in addition to our tap events.

@heff
Copy link
Member

heff commented Jun 23, 2014

Yeah, the complication behind touch + controls is:

when controls are showing
  touchmove should keep controls showing
when controls are hidden
  touchstart/touchmove should not show controls, only a tap should

And I think you're right. Just moving the preventDefault to touchend should fix this. Pull requests welcome if anyone wants to get this one.

@zang
Copy link

zang commented Jul 2, 2014

Anyone kindly fix this one? Only myself not so good at js. ;)

thanks,
Lucy

@heff heff added the unclaimed label Jul 3, 2014
@heff
Copy link
Member

heff commented Jul 3, 2014

It's been labeled as unclaimed and easy, so hopefully someone will pick it up and finish it soon.

@jackunion
Copy link
Contributor

@heff @zang here you go.

@zang
Copy link

zang commented Jul 29, 2014

You are a star. Thanks.

Huff, is this fix included in the next release?

@heff
Copy link
Member

heff commented Jul 29, 2014

Yeah, we'll get it in for the next release.

@heff heff closed this as completed in a0d8db9 Aug 4, 2014
@dovp
Copy link

dovp commented Sep 16, 2014

Hello.
I think there is some regression. this bug exists now (tested on Ipad with IOS7, with the latest example from video.js site, which I added the <"nativeControlsForTouch": false > of course.

@mmcc
Copy link
Member

mmcc commented Sep 16, 2014

@dovp How are you adding the nativeControlsForTouch option? If you're just adding it to the DOM element it's not going to do anything since the player's already been initialized by that point. Can you create a reduced test case showing the problem?

@dovp
Copy link

dovp commented Sep 17, 2014

Sure.
http://jsbin.com/kiqaqe/2/edit

@mmcc
Copy link
Member

mmcc commented Sep 17, 2014

Added a bunch of text to the example so we could scroll, and scrolling directly on top of the of video element works fine. Zooming also worked as expected. This was on an iPad running iOS 7 and an iPhone running iOS 8.

@dovp
Copy link

dovp commented Sep 17, 2014

Thanks.
Maybe what I see is not exactly belongs to this specific bug.

When I open a page with a video, normally I can do zoom in with multi-touch and with the rightest button on the control bar.

By Zoom in I mean switching from normal playing mode to full screen mode.
By Multi touch I mean tapping (or more accurate dragging) two fingers from some point over the video, one finger goes up and the other down, quickly afterwards the video opens on full screen.

Right now I can switch to full screen only if I press on the rightest button but I can't do multi-touch over the video.
Though I can switch back to normal screen with multi-touch (the other direction - make my fingers closer and closer).

I hope I was more clear this time.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

9 participants