From f9751b9256bcd68cd6ffea14b6a8a5b7d18728bf Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Thu, 5 Jul 2018 17:26:47 -0400 Subject: [PATCH] revert: "fix: Allow evented objects, such as components and plugins, to listen to the window object in addition to DOM objects. (#5255)" This reverts commit 7fd29b4f18320ee6c7daee6a9110c7afb72695c5. With this change, playing back HLS content via VHS caused an infinite event loop to be printed out. See issue #5281 --- src/js/mixins/evented.js | 19 +-- test/unit/mixins/evented.test.js | 235 ++----------------------------- 2 files changed, 17 insertions(+), 237 deletions(-) diff --git a/src/js/mixins/evented.js b/src/js/mixins/evented.js index f9cb563bb8..1df977a828 100644 --- 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 index c9ca6e955f..493e48ae9e 100644 --- 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();