revert: "fix: Allow evented objects, such as components and plugins, to listen to the window object in addition to DOM objects. (videojs#5255)" (videojs#5301)

gkatsev · web-flow · commit 361dc76e127f · 2018-07-06T13:51:22.000-04:00
This reverts commit 7fd29b4. With this change, playing back HLS content via VHS caused an infinite event loop to be printed out. See issue videojs#5281
diff --git a/src/js/mixins/evented.js b/src/js/mixins/evented.js
@@ -9,17 +9,6 @@ import * as Fn from '../utils/fn';
 import * as Obj from '../utils/obj';
 import EventTarget from '../event-target';
 
-/**
- * Returns whether or not an object supports native events.
- *
- * @param  {Object} object
- *         An object to test.
- *
- * @return {boolean}
- *         Whether or not an object supports native events.
- */
-const supportsNativeEvents = (object) => typeof object.addEventListener === 'function';
-
 /**
  * Returns whether or not an object has had the evented mixin applied.
  *
@@ -61,7 +50,7 @@ const isValidEventType = (type) =>
  *         The object to test.
  */
 const validateTarget = (target) => {
-  if (!supportsNativeEvents(target) && !isEvented(target)) {
+  if (!target.nodeName && !isEvented(target)) {
     throw new Error('Invalid target; must be a DOM node or evented object.');
   }
 };
@@ -165,7 +154,7 @@ const normalizeListenArgs = (self, args) => {
 const listen = (target, method, type, listener) => {
   validateTarget(target);
 
-  if (supportsNativeEvents(target)) {
+  if (target.nodeName) {
     Events[method](target, type, listener);
   } else {
     target[method](type, listener);
@@ -318,7 +307,7 @@ const EventedMixin = {
       // the same guid as the event listener in on().
       this.off('dispose', listener);
 
-      if (supportsNativeEvents(target)) {
+      if (target.nodeName) {
         Events.off(target, type, listener);
         Events.off(target, 'dispose', listener);
       } else if (isEvented(target)) {
@@ -357,7 +346,7 @@ const EventedMixin = {
  * @param  {String} [options.eventBusKey]
  *         By default, adds a `eventBusEl_` DOM element to the target object,
  *         which is used as an event bus. If the target object already has a
- *         DOM element which should be used, pass its key here.
+ *         DOM element that should be used, pass its key here.
  *
  * @return {Object}
  *         The target object.
diff --git a/test/unit/mixins/evented.test.js b/test/unit/mixins/evented.test.js
@@ -1,6 +1,4 @@
 /* eslint-env qunit */
-import document from 'global/document';
-import window from 'global/window';
 import sinon from 'sinon';
 import evented from '../../../src/js/mixins/evented';
 import * as Dom from '../../../src/js/utils/dom';
@@ -13,20 +11,6 @@ const errors = {
   target: new Error('Invalid target; must be a DOM node or evented object.')
 };
 
-// Cross-browser event creation.
-const createEvent = (type) => {
-  let event;
-
-  try {
-    event = new window.Event(type);
-  } catch (x) {
-    event = document.createEvent('Event');
-    event.initEvent(type, true, true);
-  }
-
-  return event;
-};
-
 const validateListenerCall = (call, thisValue, eventExpectation) => {
   const eventActual = call.args[0];
 
@@ -47,13 +31,7 @@ QUnit.module('mixins: evented', {
   },
 
   afterEach() {
-    Object.keys(this.targets).forEach(k => {
-      if (typeof this.targets[k].trigger === 'function') {
-        this.targets[k].trigger('dispose');
-      } else {
-        this.targets[k] = null;
-      }
-    });
+    Object.keys(this.targets).forEach(k => this.targets[k].trigger('dispose'));
   }
 });
 
@@ -83,7 +61,7 @@ QUnit.test('evented() with custom element', function(assert) {
   );
 });
 
-QUnit.test('on() and one() error states', function(assert) {
+QUnit.test('on() and one() errors', function(assert) {
   const target = this.targets.a = evented({});
 
   ['on', 'one'].forEach(method => {
@@ -96,7 +74,7 @@ QUnit.test('on() and one() error states', function(assert) {
   });
 });
 
-QUnit.test('off() error states', function(assert) {
+QUnit.test('off() errors', function(assert) {
   const target = this.targets.a = evented({});
 
   // An invalid event actually causes an invalid target error because it
@@ -108,7 +86,7 @@ QUnit.test('off() error states', function(assert) {
   assert.throws(() => target.off(evented({}), 'x', null), errors.listener, 'the expected error is thrown');
 });
 
-QUnit.test('on() can add a listener to one event type on itself', function(assert) {
+QUnit.test('on() can add a listener to one event type on this object', function(assert) {
   const a = this.targets.a = evented({});
   const spy = sinon.spy();
 
@@ -123,7 +101,7 @@ QUnit.test('on() can add a listener to one event type on itself', function(asser
   });
 });
 
-QUnit.test('on() can add a listener to an array of event types on itself', function(assert) {
+QUnit.test('on() can add a listener to an array of event types on this object', function(assert) {
   const a = this.targets.a = evented({});
   const spy = sinon.spy();
 
@@ -144,7 +122,7 @@ QUnit.test('on() can add a listener to an array of event types on itself', funct
   });
 });
 
-QUnit.test('one() can add a listener to one event type on itself', function(assert) {
+QUnit.test('one() can add a listener to one event type on this object', function(assert) {
   const a = this.targets.a = evented({});
   const spy = sinon.spy();
 
@@ -160,7 +138,7 @@ QUnit.test('one() can add a listener to one event type on itself', function(asse
   });
 });
 
-QUnit.test('one() can add a listener to an array of event types on itself', function(assert) {
+QUnit.test('one() can add a listener to an array of event types on this object', function(assert) {
   const a = this.targets.a = evented({});
   const spy = sinon.spy();
 
@@ -183,7 +161,7 @@ QUnit.test('one() can add a listener to an array of event types on itself', func
   });
 });
 
-QUnit.test('on() can add a listener to one event type on a different evented object', function(assert) {
+QUnit.test('on() can add a listener to one event type on a different target object', function(assert) {
   const a = this.targets.a = evented({});
   const b = this.targets.b = evented({});
   const spy = sinon.spy();
@@ -202,47 +180,7 @@ QUnit.test('on() can add a listener to one event type on a different evented obj
   });
 });
 
-QUnit.test('on() can add a listener to one event type on a node object', function(assert) {
-  const a = this.targets.a = evented({});
-  const b = this.targets.b = Dom.createEl('div');
-  const spy = sinon.spy();
-  const event = createEvent('x');
-
-  a.on(b, 'x', spy);
-  b.dispatchEvent(event);
-
-  // Make sure we aren't magically binding a listener to "a".
-  a.trigger('x');
-
-  assert.strictEqual(spy.callCount, 1, 'the listener was called the expected number of times');
-
-  validateListenerCall(spy.getCall(0), a, {
-    type: 'x',
-    target: b
-  });
-});
-
-QUnit.test('on() can add a listener to one event type on the window object', function(assert) {
-  const a = this.targets.a = evented({});
-  const b = this.targets.b = window;
-  const spy = sinon.spy();
-  const event = createEvent('x');
-
-  a.on(b, 'x', spy);
-  b.dispatchEvent(event);
-
-  // Make sure we aren't magically binding a listener to "a".
-  a.trigger('x');
-
-  assert.strictEqual(spy.callCount, 1, 'the listener was called the expected number of times');
-
-  validateListenerCall(spy.getCall(0), a, {
-    type: 'x',
-    target: b
-  });
-});
-
-QUnit.test('on() can add a listener to an array of event types on a different evented object', function(assert) {
+QUnit.test('on() can add a listener to an array of event types on a different target object', function(assert) {
   const a = this.targets.a = evented({});
   const b = this.targets.b = evented({});
   const spy = sinon.spy();
@@ -268,70 +206,13 @@ QUnit.test('on() can add a listener to an array of event types on a different ev
   });
 });
 
-QUnit.test('on() can add a listener to an array of event types on a node object', function(assert) {
-  const a = this.targets.a = evented({});
-  const b = this.targets.b = Dom.createEl('div');
-  const spy = sinon.spy();
-  const x = createEvent('x');
-  const y = createEvent('y');
-
-  a.on(b, ['x', 'y'], spy);
-  b.dispatchEvent(x);
-  b.dispatchEvent(y);
-
-  // Make sure we aren't magically binding a listener to "a".
-  a.trigger('x');
-  a.trigger('y');
-
-  assert.strictEqual(spy.callCount, 2, 'the listener was called the expected number of times');
-
-  validateListenerCall(spy.getCall(0), a, {
-    type: 'x',
-    target: b
-  });
-
-  validateListenerCall(spy.getCall(1), a, {
-    type: 'y',
-    target: b
-  });
-});
-
-QUnit.test('on() can add a listener to an array of event types on the window object', function(assert) {
-  const a = this.targets.a = evented({});
-  const b = this.targets.b = window;
-  const spy = sinon.spy();
-  const x = createEvent('x');
-  const y = createEvent('y');
-
-  a.on(b, ['x', 'y'], spy);
-  b.dispatchEvent(x);
-  b.dispatchEvent(y);
-
-  // Make sure we aren't magically binding a listener to "a".
-  a.trigger('x');
-  a.trigger('y');
-
-  assert.strictEqual(spy.callCount, 2, 'the listener was called the expected number of times');
-
-  validateListenerCall(spy.getCall(0), a, {
-    type: 'x',
-    target: b
-  });
-
-  validateListenerCall(spy.getCall(1), a, {
-    type: 'y',
-    target: b
-  });
-});
-
-QUnit.test('one() can add a listener to one event type on a different evented object', function(assert) {
+QUnit.test('one() can add a listener to one event type on a different target object', function(assert) {
   const a = this.targets.a = evented({});
   const b = this.targets.b = evented({});
   const spy = sinon.spy();
 
   a.one(b, 'x', spy);
   b.trigger('x');
-  b.trigger('x');
 
   // Make sure we aren't magically binding a listener to "a".
   a.trigger('x');
@@ -344,49 +225,9 @@ QUnit.test('one() can add a listener to one event type on a different evented ob
   });
 });
 
-QUnit.test('one() can add a listener to one event type on a node object', function(assert) {
-  const a = this.targets.a = evented({});
-  const b = this.targets.b = Dom.createEl('div');
-  const spy = sinon.spy();
-  const event = createEvent('x');
-
-  a.one(b, 'x', spy);
-  b.dispatchEvent(event);
-  b.dispatchEvent(event);
-
-  // Make sure we aren't magically binding a listener to "a".
-  a.trigger('x');
-
-  assert.strictEqual(spy.callCount, 1, 'the listener was called the expected number of times');
-
-  validateListenerCall(spy.getCall(0), a, {
-    type: 'x',
-    target: b
-  });
-});
-
-QUnit.test('one() can add a listener to one event type on the window object', function(assert) {
-  const a = this.targets.a = evented({});
-  const b = this.targets.b = window;
-  const spy = sinon.spy();
-  const event = createEvent('x');
-
-  a.one(b, 'x', spy);
-  b.dispatchEvent(event);
-  b.dispatchEvent(event);
-
-  // Make sure we aren't magically binding a listener to "a".
-  a.trigger('x');
-
-  assert.strictEqual(spy.callCount, 1, 'the listener was called the expected number of times');
-
-  validateListenerCall(spy.getCall(0), a, {
-    type: 'x',
-    target: b
-  });
-});
-
-QUnit.test('one() can add a listener to an array of event types on a different evented object', function(assert) {
+// The behavior here unfortunately differs from the identical case where "a"
+// listens to itself. This is something that should be resolved...
+QUnit.test('one() can add a listener to an array of event types on a different target object', function(assert) {
   const a = this.targets.a = evented({});
   const b = this.targets.b = evented({});
   const spy = sinon.spy();
@@ -409,56 +250,6 @@ QUnit.test('one() can add a listener to an array of event types on a different e
   });
 });
 
-QUnit.test('one() can add a listener to an array of event types on a node object', function(assert) {
-  const a = this.targets.a = evented({});
-  const b = this.targets.b = Dom.createEl('div');
-  const spy = sinon.spy();
-  const x = createEvent('x');
-  const y = createEvent('y');
-
-  a.one(b, ['x', 'y'], spy);
-  b.dispatchEvent(x);
-  b.dispatchEvent(y);
-  b.dispatchEvent(x);
-  b.dispatchEvent(y);
-
-  // Make sure we aren't magically binding a listener to "a".
-  a.trigger('x');
-  a.trigger('y');
-
-  assert.strictEqual(spy.callCount, 1, 'the listener was called the expected number of times');
-
-  validateListenerCall(spy.getCall(0), a, {
-    type: 'x',
-    target: b
-  });
-});
-
-QUnit.test('one() can add a listener to an array of event types on the window object', function(assert) {
-  const a = this.targets.a = evented({});
-  const b = this.targets.b = window;
-  const spy = sinon.spy();
-  const x = createEvent('x');
-  const y = createEvent('y');
-
-  a.one(b, ['x', 'y'], spy);
-  b.dispatchEvent(x);
-  b.dispatchEvent(y);
-  b.dispatchEvent(x);
-  b.dispatchEvent(y);
-
-  // Make sure we aren't magically binding a listener to "a".
-  a.trigger('x');
-  a.trigger('y');
-
-  assert.strictEqual(spy.callCount, 1, 'the listener was called the expected number of times');
-
-  validateListenerCall(spy.getCall(0), a, {
-    type: 'x',
-    target: b
-  });
-});
-
 QUnit.test('off() with no arguments will remove all listeners from all events on this object', function(assert) {
   const a = this.targets.a = evented({});
   const spyX = sinon.spy();
