ManualTimeUpdatesOff was not de-registering events #1793

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
4 participants
@gkatsev
Member

gkatsev commented Jan 9, 2015

ManualTimeUpdatesOff was trying to de-register trackCurrentTime and
stopTrackingCurrentTime against itself rather than the player, which is
where these events were registered against.

This manifests itself if you have flash-based tech that is loaded initially and then try to unload it and switch to an html5-based tech.

ManualTimeUpdatesOff was not de-registering events
ManualTimeUpdatesOff was trying to de-register trackCurrentTime and
stopTrackingCurrentTime against itself rather than the player, which is
where these events were registered against.
@mmcc

This comment has been minimized.

Show comment
Hide comment
@mmcc

mmcc Jan 9, 2015

Member

lgtm

Member

mmcc commented Jan 9, 2015

lgtm

@heff

This comment has been minimized.

Show comment
Hide comment
@heff

heff Jan 16, 2015

Member

@mmcc, want to attempt merging this in with contrib?

Member

heff commented Jan 16, 2015

@mmcc, want to attempt merging this in with contrib?

@mmcc

This comment has been minimized.

Show comment
Hide comment
@mmcc

mmcc Jan 16, 2015

Member

yep

Member

mmcc commented Jan 16, 2015

yep

mmcc added a commit to mmcc/video.js that referenced this pull request Jan 16, 2015

mmcc added a commit to mmcc/video.js that referenced this pull request Jan 16, 2015

@gkatsev gkatsev closed this in 1c17a67 Jan 16, 2015

@mmcc

This comment has been minimized.

Show comment
Hide comment
@mmcc

mmcc Jan 16, 2015

Member

^^ thanks, auth.

Member

mmcc commented Jan 16, 2015

^^ thanks, auth.

@gkatsev gkatsev deleted the gkatsev:hotfix/timeupdatesoff branch Jan 16, 2015

@giakas

This comment has been minimized.

Show comment
Hide comment
@giakas

giakas Jan 23, 2015

this same issue exists for 'controlsenabled' and 'controlsdisabled' event.

we never have a deregister for these events. so if we load another tech as part of src(value) and then dispose, we are getting exception. we should deregister these events as well.

giakas commented Jan 23, 2015

this same issue exists for 'controlsenabled' and 'controlsdisabled' event.

we never have a deregister for these events. so if we load another tech as part of src(value) and then dispose, we are getting exception. we should deregister these events as well.

@gkatsev

This comment has been minimized.

Show comment
Hide comment
@gkatsev

gkatsev Jan 24, 2015

Member

@giakas yes we should. Would you be up for submitting a PR for that change?

Member

gkatsev commented Jan 24, 2015

@giakas yes we should. Would you be up for submitting a PR for that change?

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