connortechnology / ZoneMinder

ZoneMinder is a free, open source Closed-circuit television software application developed for Linux which supports IP, USB and Analog cameras.
http://www.zoneminder.com/
GNU General Public License v2.0
9 stars 9 forks source link

progressBar click event offset is always the last value #31

Closed digital-gnome closed 6 years ago

digital-gnome commented 6 years ago

When clicking on the progress bar to skip around in the stream the video always jumps to the last block from the end. The offset for every click event is set to the last offset. It seems to be a scope problem because using let instead of var fixes it and there was a recent change from forEach to for. Let is only supported on IE 11+. I don't know what kind of compatibility ZoneMinder wants.

var offset = parseInt( (index*eventData.Length)/cells_length ); to let offset = parseInt( (index*eventData.Length)/cells_length );

connortechnology commented 6 years ago

We do not give the slightest shit about IE any version. Only recently have I done anything about supporting anything other than Firefox for more than 4 cameras.
If changing the var to a let fixes it.. then then is something larger going on. Think harder. Take a step back....this code is a mess, it might not be as simple a fix as we would like.

digital-gnome commented 6 years ago

We do not give the slightest shit about IE

I've never heard a more beautiful thing.

I think it has to do with scoping differences between forEach and a standard for loop. It's the major change between the version that's working and the one that isn't. I tried following the mootools chain but I don't entirely understand it yet and never could see where offset was actually passed to it. I'll think on it some more.

Somewhat unrelated but should the progressbar resize when you use the dropdown menu scale change? I know it's not coded for that, but should it be?

digital-gnome commented 6 years ago

It seems that with the change from forEach to for it's now passing a reference to offset instead of a copy. forEach added a closure because it used an extra layer of function. Changing it to let is the simplest change to fix. Otherwise we can encapsulate it in a function but I don't see the point. Let me know which way you want to go.

Tracking this down did let me discover that cell.removeEvent( 'click' ); needs to be Events. After navigating a few times there were multiple click events assigned to the cell that would all fire sequentially.