961778 - youtube wrapper ended test is timing out

Status: VERIFIED FIXED

(Webmaker Graveyard :: popcorn.js, defect)

Description

[Scott [:thecount] Downe]

Looks like bug 894943 broke this.

It just landed, so we either need to fix fast or back out. I am hoping for a fast fix.

Comment 1

[Scott [:thecount] Downe]

Attachment: https://github.com/mozilla/popcorn-js/pull/372

Currently not able to get that last test passing yet.

Scratching my head on it right now.

Based on things that have changed, and how they were, it *should* work. I am changing something, that shouldn't make a difference but is.

Not sure if we should split it or not, but backing out the internal event ticket DOES fix it... so, maybe we should consider back it out if I cannot fix it.

Comment 2

[Matthew Schranz [:mjschranz]]

Comment on attachment 8362691 [details] [review]
https://github.com/mozilla/popcorn-js/pull/372

Missed semi-colon.

Also sounds like you want this to bake a little while longer based on in person convo.

Comment 3

[Scott [:thecount] Downe]

Comment on attachment 8362691 [details] [review]
https://github.com/mozilla/popcorn-js/pull/372

Got it.

Because of the new event system, callbacks and events are outside of the player's readyState function, which means if they are not removed on destruction of the player when the source is changed, we start calling them unexpectedly.

Also changed the order of things inside the firstPlay and firstPause to just be cleaner. It didn't have a bug, but leaving events around when they are not needed anymore only to remove them later when we can sooner, we might as well remove them right away.

Comment 4

[Scott [:thecount] Downe]

Staged: https://github.com/mozilla/popcorn-js/commit/40061b0cc687e7099fff46d0a5e5e7d6bbcb4967

Needs verification.