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

Fixed extra mousemove events on Windows caused by certain apps, not users #1068

Merged
merged 1 commit into from Apr 25, 2014

Conversation

Projects
None yet
4 participants
@download13
Contributor

download13 commented Mar 9, 2014

Some platforms produce frequent mousemove events whether or not the mouse has moved. This causes the UI to always be displayed since userActive_ is always true. By keeping track of the last mouse position and only firing if the new position is different, we can eleminate the event spam.

Prevent mousemove spam
Some platforms produce frequent mousemove events whether or not the mouse has moved. This causes the UI to always be displayed since userActive_ is always `true`. By keeping track of the last mouse position and only firing if the new position is different, we can eleminate the event spam.
@mmcc

This comment has been minimized.

Show comment
Hide comment
@mmcc

mmcc Mar 19, 2014

Member

Thanks for the PR! Would you mind telling us a little more about when you see this happen (like what platforms specifically)?

Member

mmcc commented Mar 19, 2014

Thanks for the PR! Would you mind telling us a little more about when you see this happen (like what platforms specifically)?

@download13

This comment has been minimized.

Show comment
Hide comment
@download13

download13 Mar 19, 2014

Contributor

I got this in node-webkit on Windows 7 x64. I remember running across the same problem in Chrome a while back (not involving video.js) but didn't think much of it. There's an SO question where someone suggested it might be caused by applications attempting to prevent a power-saving state.

Contributor

download13 commented Mar 19, 2014

I got this in node-webkit on Windows 7 x64. I remember running across the same problem in Chrome a while back (not involving video.js) but didn't think much of it. There's an SO question where someone suggested it might be caused by applications attempting to prevent a power-saving state.

@gkatsev

This comment has been minimized.

Show comment
Hide comment
@gkatsev

gkatsev Mar 19, 2014

Member

Another thing that will help is using requestAnimationFrame rather than setTimeout/setInterval for getting useractivity.

Member

gkatsev commented Mar 19, 2014

Another thing that will help is using requestAnimationFrame rather than setTimeout/setInterval for getting useractivity.

@heff

This comment has been minimized.

Show comment
Hide comment
@heff

heff Mar 24, 2014

Member

@gkatsev, requestAnimationFrame is IE10+, so I don't think we could use that?

Member

heff commented Mar 24, 2014

@gkatsev, requestAnimationFrame is IE10+, so I don't think we could use that?

@heff heff added the enhancement label Mar 24, 2014

@gkatsev

This comment has been minimized.

Show comment
Hide comment
@gkatsev

gkatsev Mar 24, 2014

Member

You'd want to use the polyfill which is basically requestAnimationFrame || webkitRequestAnimationFrame || mozRequestAnimationFrame || function(fun) { setTimeout(fun, 1000/60); }. http://www.paulirish.com/2011/requestanimationframe-for-smart-animating/
So, in browsers that support it, it'll use rAF, but otherwise continue using setTimeout.

Member

gkatsev commented Mar 24, 2014

You'd want to use the polyfill which is basically requestAnimationFrame || webkitRequestAnimationFrame || mozRequestAnimationFrame || function(fun) { setTimeout(fun, 1000/60); }. http://www.paulirish.com/2011/requestanimationframe-for-smart-animating/
So, in browsers that support it, it'll use rAF, but otherwise continue using setTimeout.

@heff

This comment has been minimized.

Show comment
Hide comment
@heff

heff Apr 5, 2014

Member

Have you had a chance to test this in IE8?

Member

heff commented Apr 5, 2014

Have you had a chance to test this in IE8?

@heff

This comment has been minimized.

Show comment
Hide comment
@heff

heff Apr 23, 2014

Member

I'm a little cautious about this one for a few reasons:

  1. This is clearly a bug in the platform where it's happening. There's no logical reason a mousemove event should be triggered when the mouse doesn't move. Which means there should be a bug report on the specific system. I found nwjs/nw.js#1014, which says this is a Parallels issue, and makes me want more confirmation that we should be solving this. @download13, were you using this in parallels?
  2. This is the first and only time this issue has been mentioned. There's nothing wrong with that in general, it's just easier to make a case for merging when at a least a few people are experiencing it.
  3. It adds extra processing to the mousemove event which happens very rapidly. Though it's a single conditional, so it's not too heavy.

It would be nice to get ahead of this issue if it's going to come up again, but I'm not yet convinced it will. @download13, would it be possible to set up an example case where others could see this happening?

Member

heff commented Apr 23, 2014

I'm a little cautious about this one for a few reasons:

  1. This is clearly a bug in the platform where it's happening. There's no logical reason a mousemove event should be triggered when the mouse doesn't move. Which means there should be a bug report on the specific system. I found nwjs/nw.js#1014, which says this is a Parallels issue, and makes me want more confirmation that we should be solving this. @download13, were you using this in parallels?
  2. This is the first and only time this issue has been mentioned. There's nothing wrong with that in general, it's just easier to make a case for merging when at a least a few people are experiencing it.
  3. It adds extra processing to the mousemove event which happens very rapidly. Though it's a single conditional, so it's not too heavy.

It would be nice to get ahead of this issue if it's going to come up again, but I'm not yet convinced it will. @download13, would it be possible to set up an example case where others could see this happening?

@download13

This comment has been minimized.

Show comment
Hide comment
@download13

download13 Apr 24, 2014

Contributor

The platform was node-webkit on Windows 7 64bit. No parallels or any other virtualization layer.

After more googling and testing, it turns out this is a bug that occurs when the task manager is open. Some other applications (iTunes and Windows Media Player) seem to cause it as well.

For an example, go to the videojs home page in Chrome and put the player into fullscreen mode. The controls should disappear after a short time. Now open task manager (assuming you're on Windows, haven't tested this with a mac but you could try opening iTunes) and again fullscreen the video. The controls should remain visible.

You can run this in the console to see the extra events:

addEventListener('mousemove', function(e) {
    console.log(e.type, e.x, e.y);
}, false);

From what I can tell, Firefox already filters out the extra events. Chrome and node-webkit fire the extra events. IE 11 does as well, but it doesn't seem to matter at the moment since fullscreen doesn't work there.

Contributor

download13 commented Apr 24, 2014

The platform was node-webkit on Windows 7 64bit. No parallels or any other virtualization layer.

After more googling and testing, it turns out this is a bug that occurs when the task manager is open. Some other applications (iTunes and Windows Media Player) seem to cause it as well.

For an example, go to the videojs home page in Chrome and put the player into fullscreen mode. The controls should disappear after a short time. Now open task manager (assuming you're on Windows, haven't tested this with a mac but you could try opening iTunes) and again fullscreen the video. The controls should remain visible.

You can run this in the console to see the extra events:

addEventListener('mousemove', function(e) {
    console.log(e.type, e.x, e.y);
}, false);

From what I can tell, Firefox already filters out the extra events. Chrome and node-webkit fire the extra events. IE 11 does as well, but it doesn't seem to matter at the moment since fullscreen doesn't work there.

@heff

This comment has been minimized.

Show comment
Hide comment
@heff

heff Apr 24, 2014

Member

I can confirm this happens with the task manager open. Windows Media Player didn't, but my VM has an audio issue so I wasn't able to see if something has to be playing.

So I think pulling this in is fine then, but I'd still like to be able to track a Chrome bug so we know if/when we can remove this. Would you mind submitting a bug to the chrome tracker about this?
I put together this jsbin you can use as an example. http://jsbin.com/ruzofamo/1/edit

Member

heff commented Apr 24, 2014

I can confirm this happens with the task manager open. Windows Media Player didn't, but my VM has an audio issue so I wasn't able to see if something has to be playing.

So I think pulling this in is fine then, but I'd still like to be able to track a Chrome bug so we know if/when we can remove this. Would you mind submitting a bug to the chrome tracker about this?
I put together this jsbin you can use as an example. http://jsbin.com/ruzofamo/1/edit

@download13

This comment has been minimized.

Show comment
Hide comment
@download13

download13 Apr 25, 2014

Contributor

Submitted to chrome tracker

Contributor

download13 commented Apr 25, 2014

Submitted to chrome tracker

@heff

This comment has been minimized.

Show comment
Hide comment
@heff

heff Apr 25, 2014

Member

Awesome, thanks. Pulling in.

Member

heff commented Apr 25, 2014

Awesome, thanks. Pulling in.

@heff heff changed the title from Prevent mousemove spam to Fixed extra mousemove events on Windows caused by certain apps, not users Apr 25, 2014

@heff heff merged commit b7cc2a8 into videojs:master Apr 25, 2014

1 check passed

default The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment