[#1317] Fixed issue where trackevents were not being removed properly wh... #192

Open
wants to merge 1 commit into
from

2 participants

@dseif
Mozilla member

...en calling removePlugin

@ScottDowne ScottDowne commented on the diff Aug 30, 2012
popcorn.js
// update for loop if something removed, but keep checking
- idx--; sl--;
- if ( obj.data.trackEvents.startIndex <= idx ) {
+ if ( obj.data.trackEvents.startIndex > sl ) {
@ScottDowne
ScottDowne added a line comment Aug 30, 2012

Are you sure this should be reversed?

I worry this was here to make sure if the currently indexed track event is removed, we are not forced to scrub forward and back to be able to get the proper index.

@ScottDowne
ScottDowne added a line comment Aug 31, 2012

So yes, I am pretty sure you want to keep the old code, being <=

You also probably want to make sure it never goes below 0. If it currently is 0, and you're removing the first three track events, you want to keep it at 0, otherwise, you want to go back one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@rwaldron

Need some tests to prove it :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment