951392 - Sequencer doesn't properly teardown DOM elements in all cases

Status: RESOLVED INVALID

(Webmaker Graveyard :: Popcorn Maker, defect)

Description

[Matthew Schranz [:mjschranz]]

This occurs in the following situations:

1) The trackevent is removed before it finishes loading.
2) JWPlayer fails to properly load the video, leaving a black div in place over everything.

May be more, will investigate.

Comment 1

[Matthew Schranz [:mjschranz]]

Attachment: https://github.com/mozilla/popcorn.webmaker.org/pull/393

Seems to do the trick effectively. I don't think it has negative side effects.

Comment 2

[Scott [:thecount] Downe]

Comment on attachment 8349502 [details] [review]
https://github.com/mozilla/popcorn.webmaker.org/pull/393

So, I may be seeing something different.

I'm seeing a jwplayer being setup with empty string fallback sources. This means an error event is never going to be hit, and without a timeout, it'll never load, or stop loading.

Can you try changing the sourceToArray function to this:

      options.sourceToArray = function( object, type ) {
        // If our src is not an array, create an array of one.
        object[ type ] = typeof object[ type ] === "string" ? [ object[ type ] ] : object[ type ];
        object[ type ] = object[ type ].filter( function( e ){
          return e;
        });
      };

Doing that, I got proper loads, and if I deleted it before it loaded, everything looked normal to me.

Comment 3

[Matthew Schranz [:mjschranz]]

Comment on attachment 8349502 [details] [review]
https://github.com/mozilla/popcorn.webmaker.org/pull/393

Seems to do the trick well!

Comment 4

[Scott [:thecount] Downe]

Comment on attachment 8349502 [details] [review]
https://github.com/mozilla/popcorn.webmaker.org/pull/393

I'm still not able to reproduce any of the above problems. Although, jwplayer clips do load now.

If I remove a sequencer clip before it loads, it gets torn down completely.

Comment 5

[Matthew Schranz [:mjschranz]]

Scott, what is your opinion on where this should go? Sounds like it isn't causing any problems.

Comment 6

[Scott [:thecount] Downe]

I never could reproduce what you see with DOM elements still being there after a teardown, so I'm hesitant to add the timeout.

I want the source to array fix, but I need help reproducing the dom issues.

Even then, I would next try to find a solution that did not use a timeout. Either a ready event is going to eventually fire, or an error event. Once we get one or the other, we can ensure a teardown happens without a timeout.

If either a ready event or an error event is not fired, it is either still loading, and we should wait, or it has another bug which causes an error or ready event to fail, in which case we should attempt to solve that, or at the least understand what's wrong, and if after understanding it, we cannot find another solution, a timeout would be the next best solution.

Comment 7

[Matthew Schranz [:mjschranz]]

This was occuring before but I am not having any luck reproducing the issues I found.

Closing for now.

Comment 8

[Scott [:thecount] Downe]

I wonder though, the filter fix here: https://github.com/mozilla/popcorn.webmaker.org/pull/393/files#diff-8f546a8bfc7999fb36e0248e07050e7aR177

Wouldn't we need that still?