From 440a7fe627f6c607fc29ff1fe9e1b92ef638d0d9 Mon Sep 17 00:00:00 2001
From: brandonocasey <brandonocasey@gmail.com>
Date: Wed, 21 Sep 2016 17:13:18 -0400
Subject: [PATCH 01/10] implement lifecycle hooks and trigger pre/post setup
 hooks

---
 src/js/video.js | 64 +++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 62 insertions(+), 2 deletions(-)

diff --git a/src/js/video.js b/src/js/video.js
index 28638cd153..8ce8eaa8b2 100644
--- a/src/js/video.js
+++ b/src/js/video.js
@@ -96,10 +96,70 @@ function videojs(id, options, ready) {
   }
 
   // Element may have a player attr referring to an already created player instance.
-  // If not, set up a new player and return the instance.
-  return tag.player || Player.players[tag.playerId] || new Player(tag, options, ready);
+  // If so return that otherwise set up a new player below
+  if (tag.player || Player.players[tag.playerId]) {
+    return tag.player || Player.players[tag.playerId];
+  }
+
+  videojs.hooks('presetup').forEach(function(hookFunction) {
+    options = videojs.mergeOptions(options, hookFunction(tag, options, ready));
+  });
+
+  // If not, set up a new player
+  const player = new Player(tag, options, ready);
+
+  videojs.hooks('postsetup').forEach((hookFunction) => hookFunction(player));
+
+  return player;
 }
 
+/**
+ * An Object that contains lifecycle hooks as keys which point to an array
+ * of functions that are run when a lifecycle event is triggered
+ */
+videojs.hooks_ = {};
+
+/**
+ * Get a list of hooks for a specific lifecycle event
+ *
+ * @param {String} type the lifecyle event to get hooks from
+ * @return {Array} an array of hooks, or an empty array if there are none
+ */
+videojs.hooks = function(type) {
+  return videojs.hooks_[type] || [];
+};
+
+/**
+ * Add a function hook to a specific videojs lifecycle event
+ *
+ * @param {String} type the lifecycle event to hook the function to
+ * @param {Function} fn the function to attach
+ */
+videojs.hook = function(type, fn) {
+  videojs.hooks_[type] = videojs.hooks_[type] || [];
+  videojs.hooks_[type].push(fn);
+};
+
+/**
+ * Remove a hook from a specific videojs lifecycle event
+ *
+ * @param {String} type the lifecycle event that the function hooked to
+ * @param {Function} fn the hooked function to remove
+ * @return {Function|undefined} the function that was removed or undef
+ */
+videojs.removeHook = function(type, fn) {
+  const hooks = videojs.hooks_[type] || [];
+  let i = hooks.length;
+
+  while (i--) {
+    const hook = hooks[i];
+
+    if (hook === fn) {
+      return hooks.splice(i, 1);
+    }
+  }
+};
+
 // Add default styles
 if (window.VIDEOJS_NO_DYNAMIC_STYLE !== true) {
   let style = Dom.$('.vjs-styles-defaults');

From a6aae258bfc1f596be22582f87835d21c3b14f86 Mon Sep 17 00:00:00 2001
From: brandonocasey <brandonocasey@gmail.com>
Date: Mon, 26 Sep 2016 16:36:54 -0400
Subject: [PATCH 02/10] dont pass ready to presetup

---
 src/js/video.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/js/video.js b/src/js/video.js
index 8ce8eaa8b2..1fedf56fac 100644
--- a/src/js/video.js
+++ b/src/js/video.js
@@ -102,7 +102,7 @@ function videojs(id, options, ready) {
   }
 
   videojs.hooks('presetup').forEach(function(hookFunction) {
-    options = videojs.mergeOptions(options, hookFunction(tag, options, ready));
+    options = videojs.mergeOptions(options, hookFunction(tag, options));
   });
 
   // If not, set up a new player

From f6c3194d08da2ff9eb1c660033cca21acc0721e9 Mon Sep 17 00:00:00 2001
From: brandonocasey <brandonocasey@gmail.com>
Date: Mon, 26 Sep 2016 17:07:19 -0400
Subject: [PATCH 03/10] make options default to object add a guide for hooks

---
 docs/guides/hooks.md | 86 ++++++++++++++++++++++++++++++++++++++++++++
 src/js/video.js      |  3 +-
 2 files changed, 88 insertions(+), 1 deletion(-)
 create mode 100644 docs/guides/hooks.md

diff --git a/docs/guides/hooks.md b/docs/guides/hooks.md
new file mode 100644
index 0000000000..1c626195a4
--- /dev/null
+++ b/docs/guides/hooks.md
@@ -0,0 +1,86 @@
+# Hooks
+Hooks exist so that users can latch on to certain Video.js lifecycle events
+
+
+## Current Hooks
+Currently the following lifecycle events are accept and use hooks
+
+### presetup
+Called just before the player is created, so that the user can modify the options passed to the player or modify the video element that will be used for the player
+An example of this hook:
+
+```js
+    var preSetup = function(videoEl, options) {
+        videoEl.className += ' some-super-class';
+
+        // never autoplay because it makes me mad!
+        options.autoplay = false
+
+        // options that are returned here will be merged with old options
+        // automatically
+        return options;
+    };
+```
+
+### postsetup
+Called just after the player is created, so that plugins or custom functionality can be initialized
+
+```js
+    var postSetup = function(player) {
+        // initialize the foo plugin
+        player.foo();
+    };
+```
+
+
+## Usage
+
+### Adding
+In order to setup hooks you must first include Video.js in the page or script that you are using. Then you add hooks before running the videojs function like this:
+
+```js
+    videojs.hook('presetup', function(videoEl, options) {
+        // videoEl will be the element with id=vid1
+        // options will contain {autoplay: false}
+    });
+    videojs.hook('presetup', function(player) {
+        player will be the player that would get returned by the videojs function
+    });
+    videojs('vid1', {autoplay: false});
+```
+
+After adding your hooks they will automatically be run at the correct time in the Video.js lifecycle
+
+### Getting
+To access the array of hooks that currently exists and will be run on the Video.js object you can use the `hooks` function:
+
+```js
+    var preSetupHooks = videojs.hooks('presetup');
+    var postSetupHooks = videojs.hooks('postsetup');
+```
+
+### Removing
+To remove hooks from being executed during lifecycle events you will use `removeHook` like so:
+
+```js
+    var preSetup = function(videoEl, options) {};
+
+    // add the hook
+    videojs.hook('presetup', preSetup);
+
+    // remove that same hook
+    videojs.removeHook('presetup', preSetup);
+```
+
+You can also use `hooks` in conjunction with `removeHook` but it may have unexpected results if used during an asynchronous callbacks as other plugins/functionality may have added hooks.
+
+```js
+    // add the hook
+    videojs.hook('presetup', function(videoEl, options) {});
+
+    var preSetupHooks = videojs.hooks('presetup');
+
+    // remove the hook you just added
+    videojs.removeHook('presetup', preSetupHooks[preSetupHooks.length - 1]);
+```
+
diff --git a/src/js/video.js b/src/js/video.js
index 1fedf56fac..f44a4fc200 100644
--- a/src/js/video.js
+++ b/src/js/video.js
@@ -56,6 +56,7 @@ if (typeof HTMLVideoElement === 'undefined') {
  */
 function videojs(id, options, ready) {
   let tag;
+  options = options || {};
 
   // Allow for element or ID to be passed in
   // String ID
@@ -102,7 +103,7 @@ function videojs(id, options, ready) {
   }
 
   videojs.hooks('presetup').forEach(function(hookFunction) {
-    options = videojs.mergeOptions(options, hookFunction(tag, options));
+    options = videojs.mergeOptions(options, hookFunction(tag, videojs.mergeOptions({}, options)));
   });
 
   // If not, set up a new player

From 1531b4e0f8b51151b28df48c2544bd1dff6b0fad Mon Sep 17 00:00:00 2001
From: brandonocasey <brandonocasey@gmail.com>
Date: Thu, 29 Sep 2016 12:52:02 -0400
Subject: [PATCH 04/10] renamed presetup to beforesetup, and postsetup to setup

---
 docs/guides/hooks.md | 31 +++++++++++++++----------------
 src/js/video.js      |  5 +++--
 2 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/docs/guides/hooks.md b/docs/guides/hooks.md
index 1c626195a4..f1e2e6aa3f 100644
--- a/docs/guides/hooks.md
+++ b/docs/guides/hooks.md
@@ -5,12 +5,12 @@ Hooks exist so that users can latch on to certain Video.js lifecycle events
 ## Current Hooks
 Currently the following lifecycle events are accept and use hooks
 
-### presetup
+### beforesetup
 Called just before the player is created, so that the user can modify the options passed to the player or modify the video element that will be used for the player
 An example of this hook:
 
 ```js
-    var preSetup = function(videoEl, options) {
+    var beforeSetup = function(videoEl, options) {
         videoEl.className += ' some-super-class';
 
         // never autoplay because it makes me mad!
@@ -22,29 +22,28 @@ An example of this hook:
     };
 ```
 
-### postsetup
+### setup
 Called just after the player is created, so that plugins or custom functionality can be initialized
 
 ```js
-    var postSetup = function(player) {
+    var setup = function(player) {
         // initialize the foo plugin
         player.foo();
     };
 ```
 
-
 ## Usage
 
 ### Adding
 In order to setup hooks you must first include Video.js in the page or script that you are using. Then you add hooks before running the videojs function like this:
 
 ```js
-    videojs.hook('presetup', function(videoEl, options) {
+    videojs.hook('beforesetup', function(videoEl, options) {
         // videoEl will be the element with id=vid1
         // options will contain {autoplay: false}
     });
-    videojs.hook('presetup', function(player) {
-        player will be the player that would get returned by the videojs function
+    videojs.hook('setup', function(player) {
+        // player will be the player that would get returned by the videojs() function
     });
     videojs('vid1', {autoplay: false});
 ```
@@ -55,32 +54,32 @@ After adding your hooks they will automatically be run at the correct time in th
 To access the array of hooks that currently exists and will be run on the Video.js object you can use the `hooks` function:
 
 ```js
-    var preSetupHooks = videojs.hooks('presetup');
-    var postSetupHooks = videojs.hooks('postsetup');
+    var beforeSetupHooks = videojs.hooks('beforesetup');
+    var setupHooks = videojs.hooks('setup');
 ```
 
 ### Removing
 To remove hooks from being executed during lifecycle events you will use `removeHook` like so:
 
 ```js
-    var preSetup = function(videoEl, options) {};
+    var beforeSetup = function(videoEl, options) {};
 
     // add the hook
-    videojs.hook('presetup', preSetup);
+    videojs.hook('beforesetup', beforeSetup);
 
     // remove that same hook
-    videojs.removeHook('presetup', preSetup);
+    videojs.removeHook('beforesetup', beforeSetup);
 ```
 
 You can also use `hooks` in conjunction with `removeHook` but it may have unexpected results if used during an asynchronous callbacks as other plugins/functionality may have added hooks.
 
 ```js
     // add the hook
-    videojs.hook('presetup', function(videoEl, options) {});
+    videojs.hook('setup', function(videoEl, options) {});
 
-    var preSetupHooks = videojs.hooks('presetup');
+    var setupHooks = videojs.hooks('setup');
 
     // remove the hook you just added
-    videojs.removeHook('presetup', preSetupHooks[preSetupHooks.length - 1]);
+    videojs.removeHook('setup', setupHooks[setupHooks.length - 1]);
 ```
 
diff --git a/src/js/video.js b/src/js/video.js
index f44a4fc200..5bd27ff836 100644
--- a/src/js/video.js
+++ b/src/js/video.js
@@ -56,6 +56,7 @@ if (typeof HTMLVideoElement === 'undefined') {
  */
 function videojs(id, options, ready) {
   let tag;
+
   options = options || {};
 
   // Allow for element or ID to be passed in
@@ -102,14 +103,14 @@ function videojs(id, options, ready) {
     return tag.player || Player.players[tag.playerId];
   }
 
-  videojs.hooks('presetup').forEach(function(hookFunction) {
+  videojs.hooks('beforesetup').forEach(function(hookFunction) {
     options = videojs.mergeOptions(options, hookFunction(tag, videojs.mergeOptions({}, options)));
   });
 
   // If not, set up a new player
   const player = new Player(tag, options, ready);
 
-  videojs.hooks('postsetup').forEach((hookFunction) => hookFunction(player));
+  videojs.hooks('setup').forEach((hookFunction) => hookFunction(player));
 
   return player;
 }

From a1d00eb7130173a1389ebaf2c816a5264d120065 Mon Sep 17 00:00:00 2001
From: brandonocasey <brandonocasey@gmail.com>
Date: Thu, 29 Sep 2016 17:21:38 -0400
Subject: [PATCH 05/10] code review changes

---
 docs/guides/hooks.md | 65 ++++++++++++++++++++++++++++++++++----------
 src/js/video.js      | 25 ++++++++---------
 2 files changed, 62 insertions(+), 28 deletions(-)

diff --git a/docs/guides/hooks.md b/docs/guides/hooks.md
index f1e2e6aa3f..82c8b41d50 100644
--- a/docs/guides/hooks.md
+++ b/docs/guides/hooks.md
@@ -1,66 +1,100 @@
 # Hooks
-Hooks exist so that users can latch on to certain Video.js lifecycle events
+Hooks exist so that users can "hook" on to certain video.js player lifecycle
 
 
 ## Current Hooks
-Currently the following lifecycle events are accept and use hooks
+Currently, the following hooks are avialable:
 
 ### beforesetup
-Called just before the player is created, so that the user can modify the options passed to the player or modify the video element that will be used for the player
-An example of this hook:
+`beforesetup` is called just before the player is created. This allows:
+* modification of the options passed to the video.js function (`videojs('some-id, options)`)
+* modification of the dom video element that will be used for the player
 
+`beforesetup` hook functions should:
+* take two arguments
+    1. videoEl: dom video element that video.js is going to use to create a player
+    2. options: options that video.js was intialized with and will later pass to the player during creation
+* return options that will merge and override options that video.js with intialized with
+
+Example: adding beforesetup hook
 ```js
     var beforeSetup = function(videoEl, options) {
+        // videoEl.id will be some-id here, since that is what video.js
+        // was created with
+
         videoEl.className += ' some-super-class';
 
-        // never autoplay because it makes me mad!
-        options.autoplay = false
+        // autoplay will be true here, since we passed in as such
+        (options.autoplay) {
+            options.autoplay = false
+        }
 
         // options that are returned here will be merged with old options
-        // automatically
+        // in this example options will now be
+        // {autoplay: false, controls: true}
         return options;
     };
+
+    videojs.hook('beforesetup', beforeSetup);
+    videojs('some-id', {autoplay: true, controls: true});
 ```
 
 ### setup
-Called just after the player is created, so that plugins or custom functionality can be initialized
+`setup` is called just after the player is created. This allows:
+* plugin or custom functionalify to intialize on the player
+* changes to the player object itself
+
+`setup` hook functions:
+* Take one argument
+    * player: the player that video.js created
+* Don't have to return anything
 
+Example: adding setup hook
 ```js
     var setup = function(player) {
         // initialize the foo plugin
         player.foo();
     };
+    var foo = function() {};
+
+    videojs.plugin('foo', foo);
+    videojs.hook('setup', setup);
+    var player = videojs('some-id', {autoplay: true, controls: true});
 ```
 
 ## Usage
 
 ### Adding
-In order to setup hooks you must first include Video.js in the page or script that you are using. Then you add hooks before running the videojs function like this:
+In order to use hooks you must first include video.js in the page or script that you are using. Then you add hooks using `videojs.hook(<name>, function)` before running the `videojs()` function.
 
+Example: adding hooks
 ```js
     videojs.hook('beforesetup', function(videoEl, options) {
         // videoEl will be the element with id=vid1
         // options will contain {autoplay: false}
     });
     videojs.hook('setup', function(player) {
-        // player will be the player that would get returned by the videojs() function
+        // player will be the same player that is defined below
+        // as `var player`
     });
-    videojs('vid1', {autoplay: false});
+    var player = videojs('vid1', {autoplay: false});
 ```
 
-After adding your hooks they will automatically be run at the correct time in the Video.js lifecycle
+After adding your hooks they will automatically be run at the correct time in the video.js lifecycle.
 
 ### Getting
-To access the array of hooks that currently exists and will be run on the Video.js object you can use the `hooks` function:
+To access the array of hooks that currently exists and will be run on the video.js object you can use the `videojs.hooks` function.
 
+Example: getting all hooks attached to video.js
 ```js
     var beforeSetupHooks = videojs.hooks('beforesetup');
     var setupHooks = videojs.hooks('setup');
 ```
 
 ### Removing
-To remove hooks from being executed during lifecycle events you will use `removeHook` like so:
+To stop hooks from being executed during the video.js lifecycle you will remove them using `videojs.removeHook`.
 
+Example: remove a hook that was defined by you
 ```js
     var beforeSetup = function(videoEl, options) {};
 
@@ -71,8 +105,9 @@ To remove hooks from being executed during lifecycle events you will use `remove
     videojs.removeHook('beforesetup', beforeSetup);
 ```
 
-You can also use `hooks` in conjunction with `removeHook` but it may have unexpected results if used during an asynchronous callbacks as other plugins/functionality may have added hooks.
+You can also use `videojs.hooks` in conjunction with `videojs.removeHook` but it may have unexpected results if used during an asynchronous callbacks as other plugins/functionality may have added hooks.
 
+Example: using `videojs.hooks` and `videojs.removeHook` to remove a hook
 ```js
     // add the hook
     videojs.hook('setup', function(videoEl, options) {});
diff --git a/src/js/video.js b/src/js/video.js
index 5bd27ff836..29e2775003 100644
--- a/src/js/video.js
+++ b/src/js/video.js
@@ -128,18 +128,20 @@ videojs.hooks_ = {};
  * @return {Array} an array of hooks, or an empty array if there are none
  */
 videojs.hooks = function(type) {
-  return videojs.hooks_[type] || [];
+  videojs.hooks_[type] = videojs.hooks_[type] || [];
+  return videojs.hooks_[type];
 };
 
 /**
  * Add a function hook to a specific videojs lifecycle event
  *
  * @param {String} type the lifecycle event to hook the function to
- * @param {Function} fn the function to attach
+ * @param {Function|Array} fn the function to attach
  */
 videojs.hook = function(type, fn) {
-  videojs.hooks_[type] = videojs.hooks_[type] || [];
-  videojs.hooks_[type].push(fn);
+  const hooks = videojs.hooks[type];
+
+  hooks[type].concat(fn);
 };
 
 /**
@@ -147,19 +149,16 @@ videojs.hook = function(type, fn) {
  *
  * @param {String} type the lifecycle event that the function hooked to
  * @param {Function} fn the hooked function to remove
- * @return {Function|undefined} the function that was removed or undef
+ * @return {Boolean} the function that was removed or undef
  */
 videojs.removeHook = function(type, fn) {
-  const hooks = videojs.hooks_[type] || [];
-  let i = hooks.length;
+  const hooks = videojs.hooks(type);
+  const index = hooks[type].indexOf(fn);
 
-  while (i--) {
-    const hook = hooks[i];
+  hooks[type] = hooks[type].slice();
+  hooks[type].splice(index, 1);
 
-    if (hook === fn) {
-      return hooks.splice(i, 1);
-    }
-  }
+  return index > -1;
 };
 
 // Add default styles

From 74b120f5b568bceee877038918b51d4b234b3e57 Mon Sep 17 00:00:00 2001
From: brandonocasey <brandonocasey@gmail.com>
Date: Mon, 3 Oct 2016 17:33:38 -0400
Subject: [PATCH 06/10] small logic change

---
 src/js/video.js | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/src/js/video.js b/src/js/video.js
index 29e2775003..4b742f0652 100644
--- a/src/js/video.js
+++ b/src/js/video.js
@@ -117,37 +117,41 @@ function videojs(id, options, ready) {
 
 /**
  * An Object that contains lifecycle hooks as keys which point to an array
- * of functions that are run when a lifecycle event is triggered
+ * of functions that are run when a lifecycle is triggered
  */
 videojs.hooks_ = {};
 
 /**
- * Get a list of hooks for a specific lifecycle event
+ * Get a list of hooks for a specific lifecycle
  *
- * @param {String} type the lifecyle event to get hooks from
+ * @param {String} type the lifecyle to get hooks from
+ * @param {Function=} optionally add a hook to the lifecycle that your are getting
  * @return {Array} an array of hooks, or an empty array if there are none
  */
-videojs.hooks = function(type) {
+videojs.hooks = function(type, fn) {
   videojs.hooks_[type] = videojs.hooks_[type] || [];
+  if (fn) {
+    videojs.hook(type, fn);
+  }
   return videojs.hooks_[type];
 };
 
 /**
- * Add a function hook to a specific videojs lifecycle event
+ * Add a function hook to a specific videojs lifecycle
  *
- * @param {String} type the lifecycle event to hook the function to
+ * @param {String} type the lifecycle to hook the function to
  * @param {Function|Array} fn the function to attach
  */
 videojs.hook = function(type, fn) {
-  const hooks = videojs.hooks[type];
+  const hooks = videojs.hooks(type);
 
-  hooks[type].concat(fn);
+  videojs.hooks_[type] = hooks[type].concat(fn);
 };
 
 /**
- * Remove a hook from a specific videojs lifecycle event
+ * Remove a hook from a specific videojs lifecycle
  *
- * @param {String} type the lifecycle event that the function hooked to
+ * @param {String} type the lifecycle that the function hooked to
  * @param {Function} fn the hooked function to remove
  * @return {Boolean} the function that was removed or undef
  */

From 52697fda9f40e801c5d494c27a4327cad1fc360d Mon Sep 17 00:00:00 2001
From: brandonocasey <brandonocasey@gmail.com>
Date: Tue, 4 Oct 2016 11:32:18 -0400
Subject: [PATCH 07/10] unit tests, and small code changes for them

---
 src/js/video.js         |  13 ++--
 test/unit/video.test.js | 133 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 138 insertions(+), 8 deletions(-)

diff --git a/src/js/video.js b/src/js/video.js
index 4b742f0652..2ae1ed2847 100644
--- a/src/js/video.js
+++ b/src/js/video.js
@@ -131,7 +131,7 @@ videojs.hooks_ = {};
 videojs.hooks = function(type, fn) {
   videojs.hooks_[type] = videojs.hooks_[type] || [];
   if (fn) {
-    videojs.hook(type, fn);
+    videojs.hooks_[type] = videojs.hooks_[type].concat(fn);
   }
   return videojs.hooks_[type];
 };
@@ -143,9 +143,7 @@ videojs.hooks = function(type, fn) {
  * @param {Function|Array} fn the function to attach
  */
 videojs.hook = function(type, fn) {
-  const hooks = videojs.hooks(type);
-
-  videojs.hooks_[type] = hooks[type].concat(fn);
+  videojs.hooks(type, fn);
 };
 
 /**
@@ -156,11 +154,10 @@ videojs.hook = function(type, fn) {
  * @return {Boolean} the function that was removed or undef
  */
 videojs.removeHook = function(type, fn) {
-  const hooks = videojs.hooks(type);
-  const index = hooks[type].indexOf(fn);
+  const index = videojs.hooks(type).indexOf(fn);
 
-  hooks[type] = hooks[type].slice();
-  hooks[type].splice(index, 1);
+  videojs.hooks_[type] = videojs.hooks_[type].slice();
+  videojs.hooks_[type].splice(index, 1);
 
   return index > -1;
 };
diff --git a/test/unit/video.test.js b/test/unit/video.test.js
index 0ba89af735..02349c0dec 100644
--- a/test/unit/video.test.js
+++ b/test/unit/video.test.js
@@ -166,3 +166,136 @@ QUnit.test('should expose DOM functions', function(assert) {
                       `videojs.${vjsName} is a reference to Dom.${domName}`);
   });
 });
+
+QUnit.module('video.js:hooks ', {
+  beforeEach() {
+    videojs.hooks_ = {};
+  }
+});
+
+QUnit.test('should be able to add a hook', function(assert) {
+  videojs.hook('foo', function() {});
+  assert.equal(Object.keys(videojs.hooks_).length, 1, 'should have 1 hook type');
+  assert.equal(videojs.hooks_.foo.length, 1, 'should have 1 foo hook');
+
+  videojs.hook('bar', function() {});
+  assert.equal(Object.keys(videojs.hooks_).length, 2, 'should have 2 hook types');
+  assert.equal(videojs.hooks_.bar.length, 1, 'should have 1 bar hook');
+  assert.equal(videojs.hooks_.foo.length, 1, 'should have 1 foo hook');
+
+  videojs.hook('bar', function() {});
+  assert.equal(videojs.hooks_.bar.length, 2, 'should have 2 bar hooks');
+  assert.equal(videojs.hooks_.foo.length, 1, 'should have 1 foo hook');
+
+  videojs.hook('foo', function() {});
+  videojs.hook('foo', function() {});
+  videojs.hook('foo', function() {});
+  assert.equal(videojs.hooks_.foo.length, 4, 'should have 4 foo hooks');
+  assert.equal(videojs.hooks_.bar.length, 2, 'should have 2 bar hooks');
+});
+
+QUnit.test('should be able to remove a hook', function(assert) {
+  const noop = function() {};
+
+  videojs.hook('foo', noop);
+  assert.equal(Object.keys(videojs.hooks_).length, 1, 'should have 1 hook types');
+  assert.equal(videojs.hooks_.foo.length, 1, 'should have 1 foo hook');
+
+  videojs.hook('bar', noop);
+  assert.equal(Object.keys(videojs.hooks_).length, 2, 'should have 2 hooks types');
+  assert.equal(videojs.hooks_.foo.length, 1, 'should have 1 foo hook');
+  assert.equal(videojs.hooks_.bar.length, 1, 'should have 1 bar hook');
+
+  const fooRetval = videojs.removeHook('foo', noop);
+
+  assert.equal(fooRetval, true, 'should return true');
+  assert.equal(Object.keys(videojs.hooks_).length, 2, 'should have 2 hooks types');
+  assert.equal(videojs.hooks_.foo.length, 0, 'should have 0 foo hook');
+  assert.equal(videojs.hooks_.bar.length, 1, 'should have 0 bar hook');
+
+  const barRetval = videojs.removeHook('bar', noop);
+
+  assert.equal(barRetval, true, 'should return true');
+  assert.equal(Object.keys(videojs.hooks_).length, 2, 'should have 2 hooks types');
+  assert.equal(videojs.hooks_.foo.length, 0, 'should have 0 foo hook');
+  assert.equal(videojs.hooks_.bar.length, 0, 'should have 0 bar hook');
+
+  const errRetval = videojs.removeHook('bar', noop);
+
+  assert.equal(errRetval, false, 'should return false');
+  assert.equal(Object.keys(videojs.hooks_).length, 2, 'should have 2 hooks types');
+  assert.equal(videojs.hooks_.foo.length, 0, 'should have 0 foo hook');
+  assert.equal(videojs.hooks_.bar.length, 0, 'should have 0 bar hook');
+});
+
+QUnit.test('should be get all hooks for a type', function(assert) {
+  const noop = function() {};
+
+  videojs.hook('foo', noop);
+  assert.equal(Object.keys(videojs.hooks_).length, 1, 'should have 1 hook types');
+  assert.equal(videojs.hooks_.foo.length, 1, 'should have 1 foo hook');
+
+  videojs.hook('bar', noop);
+  assert.equal(Object.keys(videojs.hooks_).length, 2, 'should have 2 hooks types');
+  assert.equal(videojs.hooks_.foo.length, 1, 'should have 1 foo hook');
+  assert.equal(videojs.hooks_.bar.length, 1, 'should have 1 bar hook');
+
+  const fooHooks = videojs.hooks('foo');
+  const barHooks = videojs.hooks('bar');
+
+  assert.deepEqual(videojs.hooks_.foo, fooHooks, 'should return the exact foo list from videojs.hooks_');
+  assert.deepEqual(videojs.hooks_.bar, barHooks, 'should return the exact bar list from videojs.hooks_');
+});
+
+QUnit.test('should be get all hooks for a type and add at the same time', function(assert) {
+  const noop = function() {};
+
+  videojs.hook('foo', noop);
+  assert.equal(Object.keys(videojs.hooks_).length, 1, 'should have 1 hook types');
+  assert.equal(videojs.hooks_.foo.length, 1, 'should have 1 foo hook');
+
+  videojs.hook('bar', noop);
+  assert.equal(Object.keys(videojs.hooks_).length, 2, 'should have 2 hooks types');
+  assert.equal(videojs.hooks_.foo.length, 1, 'should have 1 foo hook');
+  assert.equal(videojs.hooks_.bar.length, 1, 'should have 1 bar hook');
+
+  const fooHooks = videojs.hooks('foo', noop);
+  const barHooks = videojs.hooks('bar', noop);
+
+  assert.deepEqual(videojs.hooks_.foo.length, 2, 'foo should have two noop hooks');
+  assert.deepEqual(videojs.hooks_.bar.length, 2, 'bar should have two noop hooks');
+  assert.deepEqual(videojs.hooks_.foo, fooHooks, 'should return the exact foo list from videojs.hooks_');
+  assert.deepEqual(videojs.hooks_.bar, barHooks, 'should return the exact bar list from videojs.hooks_');
+});
+
+QUnit.test('should trigger beforesetup and setup during videojs setup', function(assert) {
+  const vjsOptions = {techOrder: ['techFaker']};
+  let setupCalled = false;
+  let beforeSetupCalled = false;
+  const beforeSetup = function(video, options) {
+    beforeSetupCalled = true;
+    assert.deepEqual(options, vjsOptions, 'options should be the same');
+    assert.equal(video.id, 'test_vid_id', 'video id should be correct');
+  };
+  const setup = function(player) {
+    setupCalled = true;
+    assert.ok(player, 'created player from tag');
+    assert.ok(player.id() === 'test_vid_id');
+    assert.ok(videojs.getPlayers().test_vid_id === player,
+              'added player to global reference');
+  };
+
+  const fixture = document.getElementById('qunit-fixture');
+
+  fixture.innerHTML += '<video id="test_vid_id"><source type="video/mp4"></video>';
+
+  const vid = document.getElementById('test_vid_id');
+
+  videojs.hook('beforesetup', beforeSetup);
+  videojs.hook('setup', setup);
+
+  videojs(vid, vjsOptions);
+
+  assert.equal(beforeSetupCalled, true, 'beforeSetup was called');
+  assert.equal(setupCalled, true, 'setup was called');
+});

From 2ea6a47cb216cae48662e934f501ab241a54620c Mon Sep 17 00:00:00 2001
From: brandonocasey <brandonocasey@gmail.com>
Date: Tue, 4 Oct 2016 17:20:39 -0400
Subject: [PATCH 08/10] handle bad beforesetup retrun values

---
 src/js/video.js         |  9 ++++++++-
 test/unit/video.test.js | 27 ++++++++++++++++++++++++++-
 2 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/src/js/video.js b/src/js/video.js
index 2ae1ed2847..24e1247923 100644
--- a/src/js/video.js
+++ b/src/js/video.js
@@ -104,7 +104,14 @@ function videojs(id, options, ready) {
   }
 
   videojs.hooks('beforesetup').forEach(function(hookFunction) {
-    options = videojs.mergeOptions(options, hookFunction(tag, videojs.mergeOptions({}, options)));
+    const opts = hookFunction(tag, videojs.mergeOptions({}, options));
+
+    if (!opts || typeof opts !== 'object' || Array.isArray(opts)) {
+      videojs.log.error('please return an object in beforesetup hooks');
+      return;
+    }
+
+    options = videojs.mergeOptions(options, opts);
   });
 
   // If not, set up a new player
diff --git a/test/unit/video.test.js b/test/unit/video.test.js
index 02349c0dec..f0180b1e2e 100644
--- a/test/unit/video.test.js
+++ b/test/unit/video.test.js
@@ -294,8 +294,33 @@ QUnit.test('should trigger beforesetup and setup during videojs setup', function
   videojs.hook('beforesetup', beforeSetup);
   videojs.hook('setup', setup);
 
-  videojs(vid, vjsOptions);
+  const player = videojs(vid, vjsOptions);
 
+  assert.ok(player.options_, 'returning null in beforesetup does not lose options');
   assert.equal(beforeSetupCalled, true, 'beforeSetup was called');
   assert.equal(setupCalled, true, 'setup was called');
 });
+
+QUnit.test('beforesetup returns dont break videojs options', function(assert) {
+  const vjsOptions = {techOrder: ['techFaker']};
+  const fixture = document.getElementById('qunit-fixture');
+
+  fixture.innerHTML += '<video id="test_vid_id"><source type="video/mp4"></video>';
+
+  const vid = document.getElementById('test_vid_id');
+
+  videojs.hook('beforesetup', function() {
+    return null;
+  });
+  videojs.hook('beforesetup', function() {
+    return '';
+  });
+  videojs.hook('beforesetup', function() {
+    return [];
+  });
+
+  const player = videojs(vid, vjsOptions);
+
+  assert.ok(player.options_, 'beforesetup should not destory options');
+  assert.equal(player.options_.techOrder, vjsOptions.techOrder, 'options set by user should exist');
+});

From 849b10a154f46f9cbffed2ea6fff2c36eac1fe6d Mon Sep 17 00:00:00 2001
From: brandonocasey <brandonocasey@gmail.com>
Date: Thu, 6 Oct 2016 11:50:42 -0400
Subject: [PATCH 09/10] small updates from code review

---
 src/js/video.js         | 6 +++++-
 test/unit/video.test.js | 5 ++++-
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/src/js/video.js b/src/js/video.js
index 24e1247923..200c06216c 100644
--- a/src/js/video.js
+++ b/src/js/video.js
@@ -163,10 +163,14 @@ videojs.hook = function(type, fn) {
 videojs.removeHook = function(type, fn) {
   const index = videojs.hooks(type).indexOf(fn);
 
+  if (index <= -1) {
+    return false;
+  }
+
   videojs.hooks_[type] = videojs.hooks_[type].slice();
   videojs.hooks_[type].splice(index, 1);
 
-  return index > -1;
+  return true;
 };
 
 // Add default styles
diff --git a/test/unit/video.test.js b/test/unit/video.test.js
index f0180b1e2e..5164fd379b 100644
--- a/test/unit/video.test.js
+++ b/test/unit/video.test.js
@@ -228,7 +228,7 @@ QUnit.test('should be able to remove a hook', function(assert) {
   assert.equal(videojs.hooks_.bar.length, 0, 'should have 0 bar hook');
 });
 
-QUnit.test('should be get all hooks for a type', function(assert) {
+QUnit.test('should be able get all hooks for a type', function(assert) {
   const noop = function() {};
 
   videojs.hook('foo', noop);
@@ -274,11 +274,14 @@ QUnit.test('should trigger beforesetup and setup during videojs setup', function
   let beforeSetupCalled = false;
   const beforeSetup = function(video, options) {
     beforeSetupCalled = true;
+    assert.equal(setupCalled, false, 'setup should be called after beforesetup');
     assert.deepEqual(options, vjsOptions, 'options should be the same');
     assert.equal(video.id, 'test_vid_id', 'video id should be correct');
   };
   const setup = function(player) {
     setupCalled = true;
+
+    assert.equal(beforeSetupCalled, true, 'beforesetup should have been called already');
     assert.ok(player, 'created player from tag');
     assert.ok(player.id() === 'test_vid_id');
     assert.ok(videojs.getPlayers().test_vid_id === player,

From cddd175a25ea0c1c6dfec6da7c499b5fd2dbe06b Mon Sep 17 00:00:00 2001
From: brandonocasey <brandonocasey@gmail.com>
Date: Thu, 6 Oct 2016 11:57:44 -0400
Subject: [PATCH 10/10] added a positive test case for beforesetup options

---
 test/unit/video.test.js | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/test/unit/video.test.js b/test/unit/video.test.js
index 5164fd379b..bc2402a688 100644
--- a/test/unit/video.test.js
+++ b/test/unit/video.test.js
@@ -327,3 +327,23 @@ QUnit.test('beforesetup returns dont break videojs options', function(assert) {
   assert.ok(player.options_, 'beforesetup should not destory options');
   assert.equal(player.options_.techOrder, vjsOptions.techOrder, 'options set by user should exist');
 });
+
+QUnit.test('beforesetup options override videojs options', function(assert) {
+  const vjsOptions = {techOrder: ['techFaker'], autoplay: false};
+  const fixture = document.getElementById('qunit-fixture');
+
+  fixture.innerHTML += '<video id="test_vid_id"><source type="video/mp4"></video>';
+
+  const vid = document.getElementById('test_vid_id');
+
+  videojs.hook('beforesetup', function(options) {
+    assert.equal(options.autoplay, false, 'false was passed to us');
+    return {autoplay: true};
+  });
+
+  const player = videojs(vid, vjsOptions);
+
+  assert.ok(player.options_, 'beforesetup should not destory options');
+  assert.equal(player.options_.techOrder, vjsOptions.techOrder, 'options set by user should exist');
+  assert.equal(player.options_.autoplay, true, 'autoplay should be set to true now');
+});