[#963724] Ensure other youtube scripts don't clobber our ready events. #373

Open
wants to merge 2 commits into
from

Conversation

Projects
None yet
2 participants
Contributor

ScottDowne commented Jan 24, 2014

No description provided.

+ protocol = window.location.protocol === "file:" ? "http:" : "";
+ // Wait for the script to be loaded, then check if it's ready.
+ tag.addEventListener( "load", onYouTubeIframeAPIReady, false);
+ tag.src = protocol + "//www.youtube.com/iframe_api";
@jbuck

jbuck Jan 29, 2014

Owner

Instead of having it be http: or https:, can you just force it to https: ?

@ScottDowne

ScottDowne Jan 29, 2014

Contributor

Why?

I think there was a time where it needed to work in file:// too...

@jbuck

jbuck Jan 31, 2014

Owner

If you set it to https: then we don't need to worry about loading it from file:// ever.

It also means I can just close https://bugzilla.mozilla.org/show_bug.cgi?id=965045

+ // Wait for the script to be loaded, then check if it's ready.
+ tag.addEventListener( "load", onYouTubeIframeAPIReady, false);
+ tag.src = protocol + "//www.youtube.com/iframe_api";
+ firstScriptTag = document.getElementsByTagName( "script" )[ 0 ];
@jbuck

jbuck Jan 29, 2014

Owner

You could probably simplify what you're doing here with document.head.appendChild( tag )

@ScottDowne

ScottDowne Jan 29, 2014

Contributor

The docs have it as injecting it in the start. I hope there is no reason for that, but I would rather not muck with it if it is not broken.

@ScottDowne

ScottDowne Jan 29, 2014

Contributor

I just don't want to anger what I do not understand :P

@jbuck

jbuck Jan 31, 2014

Owner

There is no reason for it, but it's loooonger than just saying document.head. A nice-to-have?

+ if ( YT.loaded ) {
+ ytReady = true;
+ while( ytCallbacks.length ) {
+ ytCallbacks[ 0 ]();
@jbuck

jbuck Jan 31, 2014

Owner

You could pop this off the queue with .shift(), then execute it there.

This might be slightly better, as if the callback fails to execute for some reason, it won't leave the callback in the queue. Not sure if that matters though...

+ callback();
+ }
+ } else {
+ setTimeout( onYouTubeIframeAPIReady, 1000 );
@jbuck

jbuck Feb 14, 2014

Owner

The script file loads in about 68ms from the office, so I think it might make sense to lower this? maybe to 250ms?

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