Skip to content

Commit

Permalink
revert: "fix: Allow evented objects, such as components and plugins, …
Browse files Browse the repository at this point in the history 
…to listen to the window object in addition to DOM objects. (#5255)"

This reverts commit 7fd29b4.

With this change, playing back HLS content via VHS caused an infinite
event loop to be printed out. See issue #5281
  • Loading branch information
@gkatsev
gkatsev committed Jul 5, 2018
1 parent 8257a37 commit f9751b9
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 237 deletions.
19 changes: 4 additions & 15 deletions src/js/mixins/evented.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down Expand Up @@ -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.');
}
};
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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)) {
Expand Down Expand Up @@ -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.
Expand Down
235 changes: 13 additions & 222 deletions test/unit/mixins/evented.test.js
View file
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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];

Expand All @@ -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'));
}
});

Expand Down Expand Up @@ -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 => {
Expand All @@ -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
Expand All @@ -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();

Expand All @@ -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();

Expand All @@ -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();

Expand All @@ -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();

Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -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');
Expand All @@ -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();
Expand All @@ -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();
Expand Down

0 comments on commit f9751b9

Please sign in to comment.